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

Sync Community on SetMuted #2414

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

Samyoul
Copy link
Member

@Samyoul Samyoul commented Oct 29, 2021

Community muted state is sync on initial sync, but muted changes are not subsequently synced.

  • Add go tests
  • Check e2e tests with QA

Notes for QA

This functionality syncs the mute state of communities, we need to test that after a community has initially been synced further changes to the muted option sync between devices.

@status-github-bot
Copy link

Pull Request Checklist

  • Have you updated the documentation, if impacted (e.g. docs.status.im)?
  • Have you tested changes with mobile?
  • Have you tested changes with desktop?

@Samyoul Samyoul self-assigned this Oct 29, 2021
@status-im-auto
Copy link
Member

status-im-auto commented Oct 29, 2021

Jenkins Builds

Click to see older builds (8)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b8f4a71 #1 2021-10-29 13:46:47 ~3 min linux 📦zip
✔️ b8f4a71 #1 2021-10-29 13:47:34 ~3 min ios 📦zip
✔️ b8f4a71 #1 2021-10-29 13:49:51 ~6 min android 📦aar
✔️ 3c0dee2 #2 2021-11-03 16:37:00 ~2 min ios 📦zip
✔️ 3c0dee2 #2 2021-11-03 16:37:06 ~2 min linux 📦zip
✔️ 3c0dee2 #2 2021-11-03 16:40:43 ~6 min android 📦aar
✔️ aa64193 #3 2021-11-03 16:48:48 ~4 min ios 📦zip
✔️ aa64193 #3 2021-11-03 16:49:21 ~4 min android 📦aar
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ af09615 #4 2021-11-03 16:49:00 ~3 min linux 📦zip
✔️ af09615 #4 2021-11-03 16:50:53 ~2 min ios 📦zip
✔️ af09615 #4 2021-11-03 16:53:28 ~4 min android 📦aar
✔️ 853d1b3 #5 2022-05-27 10:08:35 ~2 min linux 📦zip
✔️ 853d1b3 #5 2022-05-27 10:10:19 ~4 min ios 📦zip
✔️ 853d1b3 #5 2022-05-27 10:10:43 ~4 min android 📦aar

@Samyoul
Copy link
Member Author

Samyoul commented Nov 4, 2021

Looks like mute is a desktop only feature, see status-im/status-mobile#12788 (comment)

@Samyoul Samyoul marked this pull request as ready for review November 4, 2021 10:54
Copy link
Contributor

@cammellos cammellos left a comment

Choose a reason for hiding this comment

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

Missing version bump

@churik
Copy link
Member

churik commented May 12, 2022

@Samyoul is PR still relevant?

@Samyoul
Copy link
Member Author

Samyoul commented May 13, 2022

Hey @churik yes it is relevant, but it is something that only impacts desktop. Community mute setting doesn't seem to be active on mobile.

@churik
Copy link
Member

churik commented May 26, 2022

@Samyoul would you mind to rebase it?

@Samyoul Samyoul force-pushed the feature/sync-communitiy-muted branch from af09615 to 853d1b3 Compare May 27, 2022 10:05
@pavloburykh pavloburykh added the Tested: mobile checked for regression on mobile client label May 30, 2022
@anastasiyaig anastasiyaig self-requested a review June 2, 2022 15:55
@anastasiyaig anastasiyaig added the Tested: desktop checked for regression on desktop client label Jun 3, 2022
@Samyoul
Copy link
Member Author

Samyoul commented Jun 8, 2022

Hey @anastasiyaig I've moved the PR back into client testing. In tests this functionality works, and devices update the muted state of communities between devices once a user syncs their devices.

@anastasiyaig anastasiyaig removed the Tested: desktop checked for regression on desktop client label Jun 8, 2022
@anastasiyaig
Copy link
Contributor

anastasiyaig commented Jun 8, 2022

i've tested this functionality and looks like it works the opposite I do expect. so when i have 2 instances of desktop, same community with same recovered account , both items are synced and then i switch the toggle OFF on one of the devices -> sync devices -> then both instances have the toggle ON (i was expecting it to be OFF)

@Samyoul what do u think?

Screen.Recording.2022-06-08.at.17.49.15.mov

@anastasiyaig
Copy link
Contributor

@Samyoul shall that be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop: tested-issues Tested: mobile checked for regression on mobile client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants