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.

Sign up for GitHub

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

Jump to bottom

Chat content updates in the background while playing with the Edit Shared Popup addresses #14376

Closed
jrainville opened this issue Apr 9, 2024 · 5 comments
Closed

Chat content updates in the background while playing with the Edit Shared Popup addresses #14376

jrainville opened this issue Apr 9, 2024 · 5 comments
Assignees
@MishkaRogachev
Labels
backend-team bug Something isn't working E:Desktop Comm Perms and Minting MVP Misc tasks about Community permissions that are not part of another Epic, due for the MVP E:Desktop Community Shared Addresses Selection Implementation of the Shared Addresses feature for joining communities and also for edits
Milestone
2.29.0 Beta

Comments

@jrainville
Copy link
Member

Bug Report

Description

You can see it happening in the video from this comment: status-im/status-go#5023 (comment)

Basically, if you have a community with permissioned channels and have different accounts that have different tokens, if you go in a channel that has permissions, then go in the Edit Shared Addresses popup, when you select/unselect accounts, the permission check result will not only affect the popup, but also the chat in the background.

One solution would be to add a UUID to the permission check, so that when we send the result from the service, only the module that asked for that check handles the result. In this case, only the communities module should react to that event.

Steps to reproduce

  1. Have a community with some channels containing permissions to view/post (let's say 0.01 ETH)
  2. Have another Status account with 2 wallet accounts, one with 0.01 ETH and the other has nothing
  3. With that account, go see the channel that has a permission -> you should have access
  4. Go in the Edit Shared Addresses popup
  5. Unselect the account that has 0.01 ETH

Result: the popup edit correctly showing that you no longer have access to the channel, but in the background, the channel also loses access, even if we didn't send yet.

Expected behavior

Only the popup updates until the addresses are actually sent

Actual behavior

The chat_section module reacts to the check permission event even if it wasn't meant for it, resulting in the chat content to lock up before the addresses are even sent.

Additional Information

  • Status desktop version: master (277dda7533c8602f2fe0a4c72615be05ff9df118)
@jrainville jrainville added bug Something isn't working backend-team E:Desktop Community Shared Addresses Selection Implementation of the Shared Addresses feature for joining communities and also for edits E:Desktop Comm Perms and Minting MVP Misc tasks about Community permissions that are not part of another Epic, due for the MVP labels Apr 9, 2024
@jrainville jrainville added this to the 2.29.0 Beta milestone Apr 9, 2024
@caybro
Copy link
Member

caybro commented Apr 9, 2024

Ah, I think we had a similar bug with multiple communities, and then editing the shared addresses would get the permissions from another community. Can't find it now, @alexjba ?

@alexjba
Copy link
Contributor

alexjba commented Apr 9, 2024

Duplicate of #14200.

@jrainville When the shared addresses popup is opened we have 2 permissions models for the same community. When changing the shared addresses we request new permissions check with the selected address and the update is propagated in both models. This bug is a side effect of the wrong model being updated with the new permissions check.

@jrainville
Copy link
Member Author

I'd say they are related, but it's not exactly the same issue

In this case, the problem is that the chat section module reacts to the event if it was not meant for it.

I think doing the UUID solution is needed anyway, because even with your change to move the permisisonsModel to the chat section module, we still need to differentiate a real call to check permissions vs one from the Edit Shared addresses popup

@alexjba
Copy link
Contributor

alexjba commented Apr 9, 2024

I'd say they are related, but it's not exactly the same issue

In this case, the problem is that the chat section module reacts to the event if it was not meant for it.

I think doing the UUID solution is needed anyway, because even with your change to move the permisisonsModel to the chat section module, we still need to differentiate a real call to check permissions vs one from the Edit Shared addresses popup

It's true and the refactoring should consider this broken flow as well.

@MishkaRogachev MishkaRogachev linked a pull request Apr 11, 2024 that will close this issue
fix(Communities): Introduce request id for checking community permissions #14403
Closed
2 tasks
@MishkaRogachev MishkaRogachev mentioned this issue Apr 12, 2024
Closed
2 tasks
@MishkaRogachev
Copy link
Contributor

Works in the current master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MishkaRogachev MishkaRogachev

Labels
backend-team bug Something isn't working E:Desktop Comm Perms and Minting MVP Misc tasks about Community permissions that are not part of another Epic, due for the MVP E:Desktop Community Shared Addresses Selection Implementation of the Shared Addresses feature for joining communities and also for edits
Projects
Status Desktop/Mobile Board
Archived in project
Milestone
2.29.0 Beta
Development

Successfully merging a pull request may close this issue.

fix(Communities): Introduce request id for checking community permissions status-im/status-desktop
4 participants
@MishkaRogachev @caybro @jrainville @alexjba