Page MenuHomePhabricator

On portals, make the "selected" / "default" logic more straightforward

Authored by epriestley on Mar 30 2019, 5:31 PM.



Depends on D20349. Ref T13275. Currently, a default item is selected as a side effect of generating the full list of items, for absolutely no reason.

The logic to pick the currently selected item can also be separated out pretty easily.

(And fix a bug in with a weird edge case in projects.)

This doesn't really change anything, but it will probably make T12949 a bit easier to fix.

Test Plan

Viewed Home / projects / portals, clicked various links, got same default/selection behavior as before.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

amckinley added inline comments.

Is this going to result in a stable choice of default? Presumably items is an ordered list.


So I understand the cast to int is because everything comes out of the MySQL layer as a string, but why cast the target ID initially instead of just using the == operator?

This revision is now accepted and ready to land.Apr 1 2019, 6:27 PM

Yeah, the stored items have an explicit order (like <someOrderColumn, id>).


If the ID is some kind of junk value like "quack", it will match 0 with ==.

There's probably no way we can reach this code and actually get a bad comparison, this is more just a general PHP hygiene thing where code that compares if ("quack" == 0) { ... } and enters the condition is never desirable even in cases where it doesn't cause any actual problems.

This revision was automatically updated to reflect the committed changes.