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

feat: check if call epoch change event was fired by parent conversation #14788

Merged
merged 5 commits into from
Mar 7, 2023

Conversation

PatrykBuniX
Copy link
Contributor

This will make parent conversation membership change affect subconversation.

@@ -104,11 +104,24 @@ export const subscribeToEpochUpdates = async (

const forwardNewEpoch = async ({groupId, epoch}: {groupId: string; epoch: number}) => {
if (groupId !== subconversationGroupId) {
return;
const parentConversationId = await mlsService.findConversationIdByGroupId?.(groupId);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this is not too heavy as forwardNewEpoch will get called every time any of the group will introduce new epoch... findConversationIdByGroupId does a .find() on the list of stored conversations. Other approach would be to store groupId -> conversationId map.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine for now. Adding a new stored value could lead of sync problems later on.
Epoch changes doesn't happen on a ms frequency basis. It's fine to do a find on everyone of them 👍

@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #14788 (77913f1) into dev (0267539) will increase coverage by 0.00%.
The diff coverage is 42.85%.

❗ Current head 77913f1 differs from pull request most recent head e0d7b4c. Consider uploading reports for the commit e0d7b4c to get more accurate results

@@           Coverage Diff           @@
##              dev   #14788   +/-   ##
=======================================
  Coverage   42.85%   42.86%           
=======================================
  Files         621      621           
  Lines       21212    21215    +3     
  Branches     4868     4869    +1     
=======================================
+ Hits         9090     9093    +3     
  Misses      10965    10965           
  Partials     1157     1157           

@PatrykBuniX PatrykBuniX marked this pull request as ready for review March 7, 2023 10:10
@PatrykBuniX PatrykBuniX requested review from a team and otto-the-bot as code owners March 7, 2023 10:10
Copy link
Contributor

@atomrc atomrc left a comment

Choose a reason for hiding this comment

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

LGTM

@PatrykBuniX PatrykBuniX merged commit 9814a3d into dev Mar 7, 2023
@PatrykBuniX PatrykBuniX deleted the feat/remove-user-from-call-on-parent-drop branch March 7, 2023 15: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

3 participants