Page MenuHomePhabricator

Make editing project images redirect to "Manage" more consistently
ClosedPublic

Authored by epriestley on May 19 2017, 8:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 16, 1:46 AM
Unknown Object (File)
Feb 5 2024, 8:48 PM
Unknown Object (File)
Dec 27 2023, 11:08 AM
Unknown Object (File)
Dec 21 2023, 6:45 PM
Unknown Object (File)
Nov 30 2023, 2:46 AM
Unknown Object (File)
Nov 16 2023, 5:11 AM
Unknown Object (File)
Nov 11 2023, 3:12 PM
Unknown Object (File)
Oct 25 2023, 6:17 AM
Subscribers
None

Details

Summary

Ref T12732. Currently, different ways of setting a profile image can leave you in different places.

Instead, always send the user back to the "Manage" page.

Test Plan

Used "Current Picture", "use picture", "Build picture" and "upload picture", always ended up in the same spot.

Diff Detail

Repository
rP Phabricator
Branch
tweak2
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 17122
Build 22908: Run Core Tests
Build 22907: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/applications/project/controller/PhabricatorProjectEditPictureController.php
27

I guess this variable was unused?

This revision is now accepted and ready to land.May 19 2017, 8:55 PM

Yeah, +F hits nothing.

We could probably detect this in lint, it just doesn't come up too terribly often.

Oh, I think we'd get false positives right now because the linter doesn't do a great job of "{$variable}" in strings until T8049, maybe?

Yeah, +F hits nothing.

We could probably detect this in lint, it just doesn't come up too terribly often.

I'd love a lint rule for this. I bet it would prevent typos more than anything else, but in PHP that seems extremely useful.

We can detect this version of things ("use of undeclared variable"), which I think gets most of the typos:

Error  (XHP5) Use of Undeclared Variable
 Declare variables prior to use (even if you are passing them as reference
 parameters). You may have misspelled this variable name.

           25 
           26     $manage_uri = $this->getApplicationURI('manage/'.$project->getID().'/');
           27 
 >>>       28     do_a_thing($maneg_yoori);
           29 
           30     $supported_formats = PhabricatorFile::getTransformableImageFormats();
           31     $e_file = true;

We just can't currently detect the opposite, "declared but unused variable".

epriestley marked an inline comment as done.

Filed as T12734.

(Also just playing around with the new sidebar thing.)

This revision was automatically updated to reflect the committed changes.