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(llc): reply being considered deleted if a new message was typed inside its thread #1866

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

AndreHaueisen
Copy link
Contributor

Submit a pull request

CLA

  • I have signed the Stream CLA (required).
  • The code changes follow best practices
  • Code changes are tested (add some information if not applicable)

Description of the pull request

Problem Description

We encountered a bug in our messaging feature where quoted messages were inadvertently marked as deleted when replying within a thread. This behavior was linked to the updateMessage method's logic, which inaccurately managed the state of quoted messages during updates, particularly in thread reply scenarios.

Root Cause Analysis

The core of the issue was found in the original implementation of the updateMessage method within the Channel class. The method indiscriminately updated quoted messages whenever an updating message's ID matched that of a quoted message, leading to quoted messages being marked as deleted by mistake, simply due to their reference in updates or replies.

Solution Implemented

The fix involved a strategic refactor of the updateMessage method to ensure updates to messages, especially regarding quoted messages, are handled with precision, maintaining the integrity of quoted messages unless explicitly deleted. The following key improvements were made:

  1. Preserving Quoted Messages: Enhanced the method to verify if an updating message references a quoted message without intent to delete, preserving the quoted message's original state in such instances.

  2. Refined Deletion Logic: Refined the approach to marking a quoted message as deleted, ensuring this only occurs with clear indications of the quoted message's deletion (e.g., when the message's type is 'deleted').

  3. Code Readability and Maintainability: Restructured the method for improved readability, segregating concerns clearly and documenting with comments.

  4. Separate Handling for Pinned Messages: Segregated the pinned messages handling logic into its own method, simplifying the updateMessage method.

How it was

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2024-02-28.at.17.43.49.mp4

How it is now

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2024-02-28.at.17.26.29.mp4

@AndreHaueisen AndreHaueisen force-pushed the fix_channel_message_update branch from a9ab9c2 to 59518ea Compare February 29, 2024 11:29
@AndreHaueisen AndreHaueisen changed the title Fixes a reply being considered deleted if a new message was typed inside its thread fix: reply being considered deleted if a new message was typed inside its thread Feb 29, 2024
@AndreHaueisen AndreHaueisen changed the title fix: reply being considered deleted if a new message was typed inside its thread fix(core): reply being considered deleted if a new message was typed inside its thread Feb 29, 2024
@AndreHaueisen AndreHaueisen changed the title fix(core): reply being considered deleted if a new message was typed inside its thread fix(chore): reply being considered deleted if a new message was typed inside its thread Feb 29, 2024
@AndreHaueisen AndreHaueisen changed the title fix(chore): reply being considered deleted if a new message was typed inside its thread fix(llc): reply being considered deleted if a new message was typed inside its thread Feb 29, 2024
Copy link
Contributor

@esarbanis esarbanis left a comment

Choose a reason for hiding this comment

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

LGTM

@esarbanis esarbanis merged commit c3265b5 into GetStream:master Mar 5, 2024
13 checks passed
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