Page MenuHomePhabricator

[server] Move condition to unset TRUNCATED to where TRUNCATED is set
ClosedPublic

Authored by ashoat on Mar 11 2022, 11:39 AM.
Tags
None
Referenced Files
F3300182: D3412.diff
Sun, Nov 17, 6:34 PM
Unknown Object (File)
Wed, Nov 13, 12:41 PM
Unknown Object (File)
Wed, Nov 13, 12:41 PM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 2:04 AM
Unknown Object (File)
Tue, Oct 22, 12:26 AM

Details

Summary

Instead of setting it in one place, and then conditionally unsetting it later... we should probably just check the condition in the first place.

Test Plan
  1. I tested different values of defaultMaxMessageAge such that some threads would return fewer than defaultNumberPerThread, and some threads would return 0
  2. I tested threads that needed a fetchMostRecentMessages, and threads that needed multiple fetchMessagesBeforeCursor to get to the start
  3. I made sure all threads loaded all the way to the start of a thread if I kept scrolling

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added inline comments.
server/src/fetchers/message-fetchers.js
316 ↗(On Diff #10313)

Note that we may unset TRUNCATED below.

This is probably no longer the case

This revision is now accepted and ready to land.Mar 14 2022, 6:21 AM
server/src/fetchers/message-fetchers.js
316 ↗(On Diff #10313)

I considered removing this, but then I realized that the code on line 345 technically can "unset" TRUNCATED. Not sure it's worth keeping this comment, though... it can "unset" any value, not just TRUNCATED. I'll consider removing it before landing

This revision was landed with ongoing or failed builds.Mar 14 2022, 12:47 PM
This revision was automatically updated to reflect the committed changes.