Ref T4712. This adds pagination. Future diffs will need to deploy buildTransactionTimeline everywhere and massage this stuff as necessary if we hit any special cases.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T4712: Paginate transactions on objects once there are a huge number of them
- Commits
- Restricted Diffusion Commit
rPd6341cfffec9: Transactions - add pagination to application transactions
Set page size to "5" to make it need to paginate often. Verified proper transactions loaded in and the javascript actions worked.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
This looks good so far.
Since we don't actually care about the individual nodes in the response, I think you could just send back a giant block of HTML (e.g., use hsprintf('%s', $events) to produce one) and then dump it in with something like:
JX.DOM.replace(swap, JX.$H(r.timeline).getFragment());
That might make it a little easier?
Comment Actions
We can probably clean all that reverse paging stuff up once everything moves over to this, I think it was just a lazy way for me to not have to array_reverse() a dozen times in every application.
| src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php | ||
|---|---|---|
| 7–17 | Oh, you could do the slightly more modern handleRequest(AphrontRequest $request) + $request->getURIData('phid') thing here if you want. | |
| 28 | Maybe 404 if the object doesn't implement PhabricatorApplicationTransactionInterface? | |
Comment Actions
This isn't quite ready yet. I forgot about the case where you have some transaction ID in the URI and that transaction ID is in the middle such that you have some "show newer" UI at the bottom. This theoretically breaks without this so I am going to get this part right before checkin. I also want to restore the highlight feature which I currently broke with my if (pager) new_js else old_js code.
Comment Actions
- changes as suggested
- handle the #transactionID case with some javascript wizardry. We'll keep loading prior transactions until we find what we're looking for
Comment Actions
- make sure to specify "with hiding" as false in new ajax endpoint. "with hiding" as true may hide a bunch of transactions based on assumptions about showing the author when they commented most recently... this should be off for this end point.
(this is really, really, ready now...! :D )
Comment Actions
I've been holding this because family is in town and I want to support it aggressively if it breaks anything. I'll probably land it this coming Sunday evening or Monday morning.
Also, @epriestley - if you want to take a look again given the javascript I added since last firm review please do. :D
Comment Actions
Yeah, my family stuff is turning out to take like 100% of my time. I'll try to give this another look, but may not really be back in action until late Monday.