Page MenuHomePhabricator
Create Task
Maniphest T13303

There is an extra or missing "array_reverse()" somewhere after recent transaction strength sorting changes
Closed, ResolvedPublic
Actions

Description

Symptoms:

  • Maniphest email is now showing transactions in reverse order (not sure this was up to date, though).
  • When a revision is accepted, the notification popup bubble now says "X is ready to land", not "alice accepted X".

Probable cause:

  • The "stablize sort in PHP 7" stuff probably missed an extra array_reverse() somewhere, since the old sort order was flipped? Or the new sort order is flipped?

Revisions and Commits

Restricted Differential Revision
Restricted Differential RevisionRestricted Diffusion Commit
Restricted Differential RevisionRestricted Diffusion Commit
Restricted Differential RevisionRestricted Diffusion Commit
Restricted Differential RevisionRestricted Diffusion Commit
Restricted Differential RevisionRestricted Diffusion Commit
Restricted Differential RevisionRestricted Diffusion Commit
Restricted Differential RevisionRestricted Diffusion Commit

Event Timeline

epriestley triaged this task as Low priority.May 30 2019, 7:09 PM
epriestley created this task.
Herald added a subscriber: amckinley. · View Herald TranscriptMay 30 2019, 7:09 PM
epriestley renamed this task from There is an extra or missing "array_reverse()" somewhere after recent transaction strength sorting changs to There is an extra or missing "array_reverse()" somewhere after recent transaction strength sorting changes.May 30 2019, 7:09 PM
epriestley added a revision: Restricted Differential Revision.May 30 2019, 10:37 PM
epriestley added a comment.May 30 2019, 11:15 PM

D20563 probably fixes mail. I don't think (?) it will fix notifications but haven't hunted that down yet.

epriestley added a commit: Restricted Diffusion Commit.May 31 2019, 12:15 AM
epriestley added a comment.Jun 17 2019, 5:23 PM

This is doing somewhat better now, but I've still seen:

  • Notifications for accepted commits are saying "Dxxx is ready to land", not "alice accepted X".
  • Task creation email sent out with [ Changed Policy ] or something instead of [ Created ].

Pretty sure these were both after rP81134d7e made it to secure so there's at least some more stuff to hunt down.

epriestley added a comment.Jun 17 2019, 5:33 PM

uhoh looks like I'm a dum dum

epriestley added a revision: Restricted Differential Revision.Jun 17 2019, 5:36 PM
epriestley added a revision: Restricted Differential Revision.Jun 17 2019, 5:40 PM
epriestley added a commit: Restricted Diffusion Commit.Jun 17 2019, 8:00 PM
epriestley added a commit: Restricted Diffusion Commit.Jun 17 2019, 8:07 PM
epriestley added a revision: Restricted Differential Revision.Jun 17 2019, 8:11 PM
epriestley added a commit: Restricted Diffusion Commit.Jun 17 2019, 8:20 PM
epriestley added a comment.Jun 17 2019, 9:25 PM

I deployed that last round of things to secure. Not totally confident I got everything, but hopefully we're in better shape now.

epriestley closed this task as Resolved.Jun 19 2019, 5:29 PM
epriestley claimed this task.
epriestley added a revision: Restricted Differential Revision.
epriestley removed a subscriber: epriestley.
epriestley added a subscriber: epriestley.

I think that might be everything? Not entirely sure, but haven't seen any more since the last deploy.

epriestley added a revision: Restricted Differential Revision.Jun 20 2019, 9:41 PM
epriestley added a commit: Restricted Diffusion Commit.Jun 20 2019, 11:07 PM
epriestley added a revision: Restricted Differential Revision.Jun 22 2019, 4:03 PM
epriestley added a commit: Restricted Diffusion Commit.Jun 22 2019, 4:05 PM
epriestley added a commit: Restricted Diffusion Commit.
Phabricator · Built by Phacility