Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Parse conversation roles directly from 'conversation.create' event [FS-1659] #14916

Merged
merged 3 commits into from
Mar 27, 2023

Conversation

atomrc
Copy link
Contributor

@atomrc atomrc commented Mar 27, 2023

BugFS-1659 [Web] Inconsistent display of conversation admin

Currently the flow for creating a conversation locally when it was create by someone else if:

  • we parse the content of the conversation.create event
  • we map it to a conversation entity (without the roles)
  • we re-fetch the entire conversation metadata
  • we extract the roles for that fresh fetch

But, the conversation.create event actually contains everything we need to also compute the roles directly.

This PR removes the extra fetch request and computes the roles directly from the payload the conversation.create event

Comment on lines -2397 to -2404
const data = await this.conversationService.getConversationById(conversationEntity);
const allMembers = [...data.members.others, data.members.self];
const conversationRoles = allMembers.reduce<Record<string, string>>((roles, member) => {
roles[member.id] = member.conversation_role;
return roles;
}, {});
conversationEntity.roles(conversationRoles);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we fetched the conversation for a second time and then mapped the roles (this is now done in the ConversationMapper)

@@ -89,10 +87,6 @@ export class ConversationRoleRepository {
}
};

readonly setConversationRoles = (conversation: Conversation, newRoles: ConversationRole[]): void => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was never used

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #14916 (ae3d84e) into dev (46f5b50) will increase coverage by 0.02%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##              dev   #14916      +/-   ##
==========================================
+ Coverage   42.94%   42.96%   +0.02%     
==========================================
  Files         623      623              
  Lines       21297    21294       -3     
  Branches     4891     4892       +1     
==========================================
+ Hits         9145     9148       +3     
+ Misses      10993    10986       -7     
- Partials     1159     1160       +1     

@atomrc atomrc changed the title fix: Parse conversation roles directly from 'conversation.create' event fix: Parse conversation roles directly from 'conversation.create' event [FS-1659] Mar 27, 2023
@atomrc atomrc merged commit 89b74ef into dev Mar 27, 2023
@atomrc atomrc deleted the fix/conv-create-flow branch March 27, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants