Build a rough transaction-level view of Feed
ClosedPublic
Actions

Authored by epriestley on May 20 2019, 9:23 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13294: Provide a transaction-level view of feed

Commits

rP642113708a83: Build a rough transaction-level view of Feed

Summary

Ref T13294. An install is interested in a way to easily answer audit-focused questions like "what edits were made to any Herald rule in Q1 2019?".

We can answer this kind of question with a more granular version of feed that focuses on being exhaustive rather than being human-readable.

This starts a rough version of it and deals with the two major tricky pieces: transactions are in a lot of different tables; and paging across them is not trivial.

To solve "lots of tables", we just query every table. There's a little bit of sleight-of-hand to get this working, but nothing too awful.

To solve "paging is hard", we order by "<dateCreated, phid>". The "phid" part of this order doesn't have much meaning, but it lets us put every transaction in a single, stable, global order and identify a place in that ordering given only one transaction PHID.

Test Plan

Screen Shot 2019-05-20 at 2.02.27 PM.png (1×1 px, 446 KB)

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.May 20 2019, 9:23 PM

Harbormaster completed remote builds in B22835: Diff 48959.May 20 2019, 9:24 PM

epriestley requested review of this revision.May 20 2019, 9:24 PM

epriestley added a child revision: D20533: Add support for querying feed transactions by author and date range.May 21 2019, 12:25 PM

amckinley accepted this revision.May 21 2019, 6:37 PM

amckinley added inline comments.

src/applications/feed/controller/PhabricatorFeedController.php
11–13	Weird that we instantiate this object and immediately drop a reference to it. Also, I think the important part of this got moved faithfully to `PhabricatorFeedListController`, but just wanted to make sure it was intentional to lose this `SideNavFilterView` part.

This revision is now accepted and ready to land.May 21 2019, 6:37 PM

epriestley added inline comments.May 21 2019, 7:27 PM

src/applications/feed/controller/PhabricatorFeedController.php
11–13	The new behavior should be faithful to the old behavior, just with a simpler implementation. The old behavior, with `delegateToController()`, required more boilerplate and this weird instantiation just to call `addNavigationItems()`, which is counterintuitively "take the navigation items from THIS object and add them to some other menu". The new behavior, with `SearchEngine->buildResponse()`, achieves the same effect with a simpler API, and changes to the "Hamburger" menu connected to T13247 mean that we don't need to do `buildApplicationMenu()` gymnastics anymore to get a proper mobile menu. Possible I missed something since this code is pretty old and this upgrade isn't a routine upgrade that I'm doing regularly, but I'm nearly sure the new code is the old code but better. Stuff I tested: `/feed/` has the same menu. `/feed/` has the right hamburger menu on mobile. No other page in Feed has any menus (same behavior before/after).