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

Send the sequenceNumber when deleting a report comment #3734

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

Dal-Papa
Copy link

Details

When making a call to the API to delete a report comment, we were not sending the sequenceNumber. On the API front, if no sequenceNumber is provided, we default to -1. The Pusher event that would be sent when a comment gets deleted on one device would have a sequenceNumber of -1, hence the comment could not be found or deleted.

Fixed Issues

Fixes #3524

Tests

  1. Login into account A on 2 devices. (or tabs/browsers that don't share local storage)
  2. Login into account B on another devices or incognito window.
  3. Send a message from account A to account B.
  4. You will see it on both second screen for account A and screen for account B.
  5. Delete the message you've just sent as account A.
  6. The message will still be on both second screen for account A and screen for account B.

QA Steps

Same as tests

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web & Desktop

Web (Safari) on the left, Desktop on the right
screencast 2021-06-23 18-46-36_small

@Dal-Papa Dal-Papa requested review from tgolen and a team June 23, 2021 17:02
@Dal-Papa Dal-Papa self-assigned this Jun 23, 2021
@MelvinBot MelvinBot requested review from robertjchen and removed request for a team June 23, 2021 17:02
Copy link
Contributor

@robertjchen robertjchen left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@robertjchen robertjchen merged commit 8f8d835 into main Jun 23, 2021
@robertjchen robertjchen deleted the clem-fix-sequence-number branch June 23, 2021 18:22
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.73-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.74-0🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

Delete comment - Deleted comment is not synchronized between other clients
4 participants