Page MenuHomePhabricator

[lib] Fix pinned messages not working
ClosedPublic

Authored by rohan on Dec 4 2023, 1:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 1:18 PM
Unknown Object (File)
Sun, Nov 24, 11:53 AM
Unknown Object (File)
Sun, Nov 24, 9:33 AM
Unknown Object (File)
Thu, Nov 21, 4:49 AM
Unknown Object (File)
Thu, Nov 21, 4:49 AM
Unknown Object (File)
Thu, Nov 21, 4:49 AM
Unknown Object (File)
Thu, Nov 21, 4:49 AM
Unknown Object (File)
Sat, Nov 2, 2:24 AM
Subscribers

Details

Summary

There is an issue with the recently introduced canBeRenderedIndependently code in message specs where I didn't properly account for message specs that do not define this property since its optional.

Right now:

  • if canBeRenderedIndependently is true, then isUnableToBeRenderedIndependently returns false (0 cases)
  • if canBeRenderedIndependently is false, then isUnableToBeRenderedIndependently returns true (2 cases --> edit-message-spec and reaction-message-spec)
  • if canBeRenderedIndependently is undefined, then isUnableToBeRenderedIndependently returns true` (all other cases, i.e. text-message-spec)

This causes a problem since we skip rendering TEXT messages and MULTIMEDIA messages in the pinned messages modal

This diff changes the check so that now:

  • if canBeRenderedIndependently is true, then isUnableToBeRenderedIndependently returns false (0 cases)
  • if canBeRenderedIndependently is false, then isUnableToBeRenderedIndependently returns true (2 cases --> edit-message-spec and reaction-message-spec)
  • if canBeRenderedIndependently is undefined, then isUnableToBeRenderedIndependently returns false` (all other cases, i.e. text-message-spec)

We now only class a message as being unable to rendered independently if its message spec explicitly defines it as false

Test Plan

Tested across web and native:

  1. Pinned a message
  2. Sent enough messages so the message is not automatically loaded when the chat is opened and the user would have to scroll up to load it
  3. Closed the app and re-opened it
  4. Made sure I could see the pinned message in the pinned messages modal/screen

Besides this, I ran yarn workspace lib test to make sure my existing unit tests for this code didn't break. I can alternatively add more unit tests in message-utils.test.js if needed for other message types

Also made sure to do this testing on the current state of master to repro the issue

Addresses ENG-6009

Diff Detail

Repository
rCOMM Comm
Branch
ENG-6009
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 4 2023, 4:16 PM
Harbormaster failed remote builds in B24751: Diff 34214!
rohan requested review of this revision.Dec 4 2023, 9:30 PM
This revision is now accepted and ready to land.Dec 4 2023, 9:49 PM
This revision was automatically updated to reflect the committed changes.