Page MenuHomePhabricator

[native] Unify thread settings accessibility
ClosedPublic

Authored by michal on Sep 23 2022, 5:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 21, 12:45 PM
Unknown Object (File)
Sun, Apr 21, 12:45 PM
Unknown Object (File)
Sun, Apr 21, 12:45 PM
Unknown Object (File)
Sun, Apr 21, 12:45 PM
Unknown Object (File)
Sun, Apr 21, 12:45 PM
Unknown Object (File)
Sun, Apr 21, 12:44 PM
Unknown Object (File)
Sun, Apr 21, 12:43 PM
Unknown Object (File)
Mon, Apr 8, 9:55 AM
Subscribers

Details

Summary

ENG-1827
Makes sure that both ios and android use the same logic for determining whether the thread settings should be accessible.

Test Plan

Test on both android and ios

  1. Start creating a new thread, the settings should be unavailable
  2. Add new users, send a message. Settings should be available when the thread stops being pending.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 23 2022, 5:54 AM
Harbormaster failed remote builds in B12401: Diff 17020!
michal edited the summary of this revision. (Show Details)

update

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 23 2022, 6:11 AM
Harbormaster failed remote builds in B12403: Diff 17022!
michal edited the summary of this revision. (Show Details)
atul added a reviewer: ashoat. atul added 1 blocking reviewer(s): tomek.Sep 27 2022, 6:42 AM

Adding @tomek as blocking since he initially introduced the logic: https://phab.comm.dev/D545
Adding @ashoat as blocking since he pointed out the "discrepancy" between platforms: https://phab.comm.dev/D5082#150392

atul added inline comments.
native/chat/chat.react.js
202 ↗(On Diff #17097)

Outside the scope of this diff, but might be easier to comprehend if it was more explicit that the one member is the viewer instead of member list sans ashoat having a length of 1

Regarding the test plan, have you checked if the behavior is the same on both platforms?

Yes, I've checked. I will amend the test plan right now.

This revision is now accepted and ready to land.Sep 29 2022, 5:30 AM