Page MenuHomePhabricator

[keyserver/lib] Make shimUnsupportedMessageInfo and shimUnsupportedRawMessageInfos async functions
AbandonedPublic

Authored by rohan on Jul 18 2023, 3:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 29, 12:59 PM
Unknown Object (File)
Sat, Nov 23, 12:51 PM
Unknown Object (File)
Thu, Nov 7, 7:59 PM
Unknown Object (File)
Oct 16 2024, 9:17 PM
Unknown Object (File)
Oct 16 2024, 9:17 PM
Unknown Object (File)
Oct 16 2024, 9:13 PM
Unknown Object (File)
Oct 13 2024, 12:20 AM
Unknown Object (File)
Oct 12 2024, 11:37 PM
Subscribers

Details

Reviewers
ashoat
Summary

In preparation to revise D8502 in order to call getEntityTextToRawString and do ENS resolution, there were three solutions presented as the best path forward:

Worth noting that there are some nuances here for converting from`EntityText` to a string since it requires ENS resolution. Some ideas:

  1. Call entityTextToRawString and accept that ENS resolution would not occur
  2. Use "some members" or "a member" instead of affectedUsers for this case, so that ENS resolution doesn't matter. Then you can call entityTextToRawString without worrying
  3. (Best but hardest) call getEntityTextAsString and do ENS resolution, but first you'll need to convert shimUnsupportedMessageInfo and shimUnsupportedRawMessageInfos into async functions

I chose to attempt #3, but if it ends up the case where maybe my understanding of this task was different than what was expected / a lot of iteration is required on this diff, the best thing to do would just to go with option #2 since that will be easiest and not block further progress on the creating roles feature.

This diff required three main changes:

  • Make shimUnsupportedMessageInfo async (required changing all of the individual message specs)
  • Make shimUnsupportedRawMessageInfos async
  • Update the callsites for shimUnsupportedRawMessageInfos to await the Promise, this involved making one additional function async and return a Promise (namely getMessageFetchResultFromRedisMessages in `message-fetchers.js)

Depends on D8478

Test Plan

Verified that the message specs generate robotext as expected still and ran flow in each directory

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan requested review of this revision.Jul 18 2023, 3:27 PM
rohan removed reviewers: atul, ginsu. rohan added 1 blocking reviewer(s): ashoat.Jul 25 2023, 9:23 AM

I'm going to switch to option #2 for now to reduce complexity in D8502 (i.e. resolve the members to just 'some members' / 'a member'). Abandoning this diff but can always come back to it if needed since it'll exist here