Page MenuHomePhabricator

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

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

Details

Summary

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

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Sat, Mar 30, 5:31 PM
epriestley requested review of this revision.Sat, Mar 30, 5:32 PM
amckinley accepted this revision.Mon, Apr 1, 6:27 PM
amckinley added inline comments.
src/applications/search/engine/PhabricatorProfileMenuEngine.php
1336

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

1347

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.Mon, Apr 1, 6:27 PM
epriestley added inline comments.Mon, Apr 1, 6:37 PM
src/applications/search/engine/PhabricatorProfileMenuEngine.php
1336

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

1347

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.