Skip to content

refactor: make memberlist update logic easier to follow #6449

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

Merged
merged 1 commit into from
Jan 18, 2025

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Jan 18, 2025

No description provided.

@link2xt link2xt force-pushed the link2xt/refactor-memberlist branch from ae759d7 to e5400c6 Compare January 18, 2025 17:48
@link2xt
Copy link
Collaborator Author

link2xt commented Jan 18, 2025

This does not change anything, just some refactoring I did while debugging #6445

I think it makes it easier to verify that in case there is no Chat-Group-Member-Timestamps header, for non-DC MUA missing members are added from the to field and for DC only explicit changes are allowed and the to field is not used at all.

@link2xt link2xt requested review from Hocuri and iequidoo January 18, 2025 17:50
Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

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

BTW, another readability improvement would be to extract the computation of the group changes messages:

        removed_ids.remove(&removed_id);
    }
    let group_changes_msgs =
        implicit_group_changes_msgs(context, added_ids, removed_ids, chat_id).await?;

    if let Some(avatar_action) = &mime_parser.group_avatar {

and

async fn implicit_group_changes_msgs(
    context: &Context,
    added_ids: HashSet<ContactId>,
    removed_ids: HashSet<ContactId>,
    chat_id: ChatId,
) -> Result<Vec<String>> {
    let mut group_changes_msgs = Vec::new();
    if !added_ids.is_empty() {
        warn!(
            context,
            "Implicit addition of {added_ids:?} to chat {chat_id}."
        );
    }
    if !removed_ids.is_empty() {
        warn!(
            context,
            "Implicit removal of {removed_ids:?} from chat {chat_id}."
        );
    }
    group_changes_msgs.reserve(added_ids.len() + removed_ids.len());
    for contact_id in added_ids {
        let contact = Contact::get_by_id(context, contact_id).await?;
        group_changes_msgs.push(
            stock_str::msg_add_member_local(context, contact.get_addr(), ContactId::UNDEFINED)
                .await,
        );
    }
    for contact_id in removed_ids {
        let contact = Contact::get_by_id(context, contact_id).await?;
        group_changes_msgs.push(
            stock_str::msg_del_member_local(context, contact.get_addr(), ContactId::UNDEFINED)
                .await,
        );
    }

    Ok(group_changes_msgs)
}

@link2xt link2xt force-pushed the link2xt/refactor-memberlist branch from 4cc03d8 to d2dca14 Compare January 18, 2025 21:14
@link2xt link2xt enabled auto-merge (rebase) January 18, 2025 21:15
@link2xt link2xt disabled auto-merge January 18, 2025 21:36
@link2xt link2xt force-pushed the link2xt/refactor-memberlist branch from d2dca14 to ed74997 Compare January 18, 2025 21:36
@link2xt link2xt merged commit 0b9746b into main Jan 18, 2025
37 checks passed
@link2xt link2xt deleted the link2xt/refactor-memberlist branch January 18, 2025 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants