Page MenuHomePhabricator

Increase Conpherence notification panel transaction fetch
ClosedPublic

Authored by chad on Oct 12 2016, 4:09 PM.
Tags
None
Referenced Files
F15436898: D16695.id40191.diff
Tue, Mar 25, 4:45 PM
F15428292: D16695.diff
Sun, Mar 23, 6:58 PM
F15392504: D16695.id40191.diff
Sat, Mar 15, 3:37 PM
F15386078: D16695.id40190.diff
Sat, Mar 15, 12:00 AM
F15379163: D16695.id40191.diff
Thu, Mar 13, 7:04 PM
F15353847: D16695.id.diff
Tue, Mar 11, 12:34 AM
F15333931: D16695.id40191.diff
Sat, Mar 8, 5:42 AM
Unknown Object (File)
Mon, Mar 3, 5:17 PM
Subscribers

Details

Summary

We currently fetch 15 transactions for 5 rooms, which leads to some room subtitles in the notification panel to being blank since nothing was fetched. I don't think this is a great fix, but moves the bar much further. Maybe there is a more accurate fix that isn't 5 SQL queries?

Test Plan

Review notification panel in sandbox, ensure all threads have some additional information.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Increase Conpherence notification panel transaction fetch.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
epriestley edited edge metadata.

Yeah, this is garbage but fixing it isn't trivial. Best would probably be something like this:

  • Put a nullable lastInterestingTransactionPHID (or whatever, that name is kind of wordy) on ConpherenceThread.
  • When writing a transaction, update that field if it's an "interesting" transaction (I personally think only chat is interesting, but there's room for product debate on that).
  • Get rid of needTransactions(), replace it with needLastInterestingTransactions().
  • Do one query to load + attach those specific transactions by PHID.
This revision is now accepted and ready to land.Oct 12 2016, 4:13 PM
This revision was automatically updated to reflect the committed changes.