Changing a callsign throws a 404
Open, Needs TriagePublic



phabricator f9a58fafba0d15e043f20e410bbe782357130183 (Tue, Jun 14)
arcanist c13e5a629535f460ca1a16d0bfe6d95f43b70b78 (Thu, Jun 9)
phutil fb1e159d36402cc5f6cdb64726599acf784283b6 (Mon, Jun 13)

How to reproduce

  1. Create a repo with a callsign, for example rABC
  2. Now change the callsign, the URL is diffusion/ABC/manage/basics/ in that case

Expected result

  • New Callsign got saved
  • You are now again at the basics page

Actual result

  • New Callsign got saved
  • You get a 404, because (for example you change rABC to rXYZ), phabricator still redirects you to diffusion/ABC/manage/basics/ instead of diffusion/XYZ/manage/basics/, so you get a 404, because the old callsign is not present.
avivey edited projects, added Diffusion; removed Differential.Jun 17 2016, 3:40 AM
chad added a subscriber: chad.Jun 22 2016, 5:56 PM

I dinked around on this but for the life of me, can't find where it's getting the URI from. I did learn how paging works in EditEngine though.

Yeah, this is a little obscure since I was trying not to build more than we needed. The URI comes from DiffusionRepositoryManagementPanel->newEditEnginePage(), which does this:

$key = $this->getManagementPanelKey();
$label = $this->getManagementPanelLabel();
$panel_uri = $this->getPanelURI();

return id(new PhabricatorEditPage())

getPanelURI() has an implementation above:

final public function getPanelURI() {
  $repository = $this->getRepository();
  $key = $this->getManagementPanelKey();
  return $repository->getPathURI("manage/{$key}/");

The problem is that this URI is being generated before you do your edit, and the URI contains the (old) callsign.

A hacky approach would probably be to override getEffectiveObjectEditDoneURI($object) in DiffusionRepositoryEditEngine, and do something like this:

$page = $this->getSelectedPage();
if ($page) {
  $key = $page->getKey();
  return $object->getURI("manage/{$key}/");
return parent::getEffectiveObjectEditDoneURI($object);

I think that will fix it? I'm really not happy with how these methods are working right now and plan to clean them up a bit -- I probably made the behavior too simple in Diffusion, and then kind of weird in Settings.

aubort added a subscriber: aubort.Sep 26 2016, 1:34 PM

+1 it's very confusing

Heh, just ran into this today

epriestley moved this task from Backlog to v3 on the Diffusion board.Jan 18 2017, 7:00 PM
epriestley edited projects, added Diffusion (v3); removed Diffusion.