Page MenuHomePhorge

Update Diff: fix dead-end when not yours
ClosedPublic

Authored by valerio.bozzolan on Jul 6 2023, 20:58.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 25, 22:30
Unknown Object (File)
Thu, May 23, 11:10
Unknown Object (File)
Mon, May 20, 06:50
Unknown Object (File)
Sun, May 12, 22:18
Unknown Object (File)
Sun, May 12, 12:45
Unknown Object (File)
Fri, May 10, 00:13
Unknown Object (File)
Fri, May 10, 00:13
Unknown Object (File)
Wed, May 8, 07:58

Details

Summary

Fix "Update Patch" for generic Differential revisions.

You can now help a coworker to update their patch, from the web.

So, if you visit Diff 111 (not yours) and you click "Update Diff", and you paste a valid diff,
you arrive here:

https://we.phorge.it/differential/diff/222/?revisionID=111

That page as default was suggesting some Diffs that are yours. Plus, now it shows Diff 111.

Omitting the Diff from which the workflow was started made no sense and it's certainly an oversight.

Any follow-up change is welcome to modernize the selector using AphrontFormTokenizerControl.

Closes T15538

Test Plan
  • create a Diff on Differential with user A
  • click on Update Diff from user B (pasting a valid diff) and Continue

Now that patch is selected (and selectable). No dead-end anymore.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/differential/controller/DifferentialDiffViewController.php
181

This is kept intentionally.

So in short this just assures that the current item is shown.

This is coherent with the above comment:

If a specific revision is selected (for example, because the user is following the "Update Diff" workflow), but not present in the dropdown, try to add it to the dropdown even if it is closed

valerio.bozzolan retitled this revision from Differential Update Patch: when not yours, fix dead-end to Differential Update Patch: fix dead-end when not yours.Jul 25 2023, 08:53
valerio.bozzolan edited the summary of this revision. (Show Details)
valerio.bozzolan edited the test plan for this revision. (Show Details)
valerio.bozzolan retitled this revision from Differential Update Patch: fix dead-end when not yours to Update Diff: fix dead-end when not yours.Jul 25 2023, 08:59
valerio.bozzolan edited the summary of this revision. (Show Details)
valerio.bozzolan edited the test plan for this revision. (Show Details)

I don't remember how to explain things to humans

I hope the commit message is understandable

avivey subscribed.

I was finally able to read the code and understand what's going on here.
The original code is strange, and the withAuthors() part in it is definitely wrong in context.

This revision is now accepted and ready to land.Jul 25 2023, 09:30

ah, the original intention for the second query was "even if the diff is closed", circa 2015. Back then, it was probably not possible to update a diff owned by someone else, so it made sense to add the withAuthor..