Page MenuHomePhorge

Update Patch: it's dead-end if not yours
Closed, ResolvedPublic

Description

This drives my colleagues crazy :D

Steps to reproduce:

  1. Visit a Diff like D25328 and Update Patch
  2. Paste a valid git diff and Continue

At this point you are on a similar URL:

https://we.phorge.it/differential/diff/9876/?revisionID=25328

But, you cannot select the Diff D25328:

Differential patch cannot select.png (723×779 px, 87 KB)

It seems the selectbox is limited to your diffs, but, indeed the server allows you to do update Diff D25328 from command line, but also, from the web interface: if you hack with the HTML and you put D25328 in the <select> box, you successfully update the Diff, and that is good.

So this seems just an UX limitation.

In short, the selection box probably should include the specified ?revisionID=.

Background

The current behavior probably comes from an unrelated thing.

https://secure.phabricator.com/T7602

Also the "only current viewer limitation" seems originated from a copy-paste on a part that needed the author indeed.

https://secure.phabricator.com/D12167

Proposed change

Among your changes, pick also the Diff that is specified in the current URL, whenever the DIff is yours or not (as soon as you can view+edit it of course).

So probably the fix is to just omit this specific "only current viewer" limitation in this specific line:

https://we.phorge.it/source/phorge/browse/master/src/applications/differential/controller/DifferentialDiffViewController.php;7bebfa289aa18e3ff043ea3a5b00d178ec6756e1$200

Event Timeline

valerio.bozzolan created this task.
valerio.bozzolan created this object in space S1 Public.

consider just replacing the selector with a modern AphrontFormTokenizerControl - that smart search features.

Uhm. Nice!

Can I just propose a veeery small fix for this (it's one line, after all), and then create another one to modernize the workflow with that component? I'm not sure I will be able to do not destroy everything if I immediately try the modernization ihih

Here the follow-up:

T15539: Update Patch: modernize "Diff selector"

consider just replacing the selector with a modern AphrontFormTokenizerControl - that smart search features.

I suspect just listing all open revisions would make using this page even worse - users will have to wade through a giant list.

I agree and that is why the general idea is: still showing your Diffs, but, showing the selected one (even if it's not yours). So it's just a +1 entry

ok, now I understand.

hint:
your "proposed fix" section is too low-level: it talks about code (literally a diff), but it should be written in terms of business/user flows ("also list explicitly mentioned revision").