[lib] Don't bump thin thread timestamp for membership robotext
AcceptedPublic
Actions

Authored by ashoat on Tue, Nov 12, 12:47 PM.

Details

Reviewers

tomek

Summary

This addresses ENG-9558 for membership robotext messages. The implementation is broadly similar to D13829.

Depends on D13918

Test Plan

Tested in combination with the rest of the stack. See video in D13918

Diff Detail

Repository

rCOMM Comm

Branch

ashoat/globalsearch

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Tue, Nov 12, 12:47 PM

Rebase

Harbormaster failed remote builds in B32630: Diff 45766!Tue, Nov 12, 1:17 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Tue, Nov 12, 1:29 PM

Harbormaster failed remote builds in B32636: Diff 45772!

Rebase

Harbormaster completed remote builds in B32641: Diff 45779.Tue, Nov 12, 3:33 PM

ashoat requested review of this revision.Tue, Nov 12, 3:33 PM

tomek accepted this revision.Wed, Nov 13, 4:57 AM

tomek added inline comments.

lib/shared/messages/add-members-message-spec.js
238	Don't bump thick thread timestamp for membership robotext Shouldn't the condition be inversed? When we return `null` here, the timestamp isn't bumped, so we should return `null` for thick threads. Is it correct?

This revision is now accepted and ready to land.Wed, Nov 13, 4:57 AM

ashoat added inline comments.Wed, Nov 13, 11:13 AM

lib/shared/messages/add-members-message-spec.js
238	Yes, you're absolutely right. I probably should've tested this better...

ashoat added inline comments.Wed, Nov 13, 11:15 AM

lib/shared/messages/add-members-message-spec.js
238	Wait no, this behavior is actually correct. We want to match behavior with `showInMessagePreview`, which hides for thin threads but shows for thick threads. Similarly, here we want to skip bumping for thin threads, but continue bumping for thick threads.

tomek added inline comments.Thu, Nov 14, 1:04 AM

lib/shared/messages/add-members-message-spec.js
238	Ok, that makes sense! But in that case, the diff's title should be updated.

ashoat retitled this revision from [lib] Don't bump thick thread timestamp for membership robotext to [lib] Don't bump thin thread timestamp for membership robotext.Thu, Nov 14, 4:53 AM

ashoat added inline comments.