Wire only meeting participant invites on schedule modal [WPB-25056]#21643
Wire only meeting participant invites on schedule modal [WPB-25056]#21643thisisamir98 wants to merge 4 commits into
Conversation
Use contact search with remote lookup and showAllProvidedUsers, drop edit prefill email placeholders, baseline invitation diffs on resolved emails only, and surface named ParticipantMissingEmailError messages via i18n.
thisisamir98
requested review from
arjita-mitra,
e-maad,
otto-the-bot,
screendriver and
zskhan
as code owners
thisisamir98
changed the title
thisisamir98
changed the title
M1 adds and removes schedule-meeting participants via ConversationRepository on the meeting's qualified_conversation, with metadata-only meetings API calls and no webapp invitation endpoint usage.
|
| const conversation = await conversationRepository.getConversationById( | ||
| createResult.value.qualified_conversation, | ||
| ); | ||
| await conversationRepository.addUsers(conversation, formState.selectedUsers); |
There was a problem hiding this comment.
Can we confirm that conversationRepository.addUsers() actually rejects for the participant-add failures we want to show as addParticipantsFailed here? From the repository implementation it looks like backend errors are caught and handled internally for several labels instead of being rethrown. In that case this try/catch would not run and the schedule submit flow could report success even though participants were not added. Maybe this flow needs an explicit result/failed-users signal instead of relying on thrown errors?
| return result.err(meetingSubmitErrors.updateFailed); | ||
| } | ||
|
|
||
| const conversation = await conversationRepository.getConversationById(qualifiedConversation.value); |
There was a problem hiding this comment.
Should this be inside a try/catch as well? If getConversationById() fails here tryUpdateMeeting() rejects before returning a MeetingSubmitErrors result. useScheduleMeetingSubmit() only displays the schedule-meeting error modal when performMeetingSubmit() resolves with result.isErr, so this path could fail without the translated update error. Since the meeting metadata may already have been updated at this point, we probably also want the same fetchMeetings() recovery behavior as in the add/remove branches.
| const formState = mapMeetingToScheduleFormState(meeting, availableUsers); | ||
| async (meeting: Meeting) => { | ||
| const conversationRepository = container.resolve(ConversationRepository); | ||
| const conversation = await conversationRepository.getConversationById(meeting.qualified_conversation); |
There was a problem hiding this comment.
Since opening the edit modal now depends on this async fetch, could we add an error path here? If the conversation cannot be loaded, openEdit() is never called and the caller discards the promise with void editMeeting(meeting) (fire-and-forget invoker?), so clicking "Edit meeting" can silently do nothing. Even a small catch with logging and a user-facing error would make this safer.
| onEdit: () => { | ||
| if (canEditMeeting(meeting, selfUser, wallClock.currentTimestampInMilliseconds)) { | ||
| editMeeting(meeting); | ||
| void editMeeting(meeting); |
There was a problem hiding this comment.
It would be better to use our fire-and-forget invoker here
2 participants
Use contact search with remote lookup and showAllProvidedUsers, drop edit prefill email placeholders, baseline invitation diffs on resolved emails only, and surface named ParticipantMissingEmailError messages via i18n.