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

Bug: Fix view&post token-permitted channel input not locked #5023

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

kounkou
Copy link
Contributor

@kounkou kounkou commented Apr 8, 2024

Description

In previous version of status-go, we failed to compute the permission for communities with token permitted channels. This caused the UI to display the wrong UI when displaying the channel UI. Indeed, the user was enable to view the channel and couldn't send messages.
In this PR, we are now able to lock consistently the channel if we do not have permission to view&post. This PR fixes #14117
The change consists in adding a retrieval of the Revealed accounts for cases when the addresses are not shared, and where the purpose isn't to have all accounts considered for the calculation of permissions on channel.

Testing

Multiple tests already cover the functions CheckAllChannelsPermissions and CheckChannelPermissions, making sure all tests are passed. If any strong opinion on the testing side, I can allocate a bit more time.

  • community_test.go
  • manager_test.go

I have also checked the flows :

-[x] Restore write when channel is unlocked
-[x] Set the channel permissions at boot
-[x] Detect changes when user
-[x] edit permissions
-[x] create permissions
-[x] delete permissions

Demo

Screencast.from.2024-04-08.12.18.14.AM.webm

@status-im-auto
Copy link
Member

status-im-auto commented Apr 8, 2024

Jenkins Builds

Click to see older builds (20)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 67d89f4 #1 2024-04-08 08:59:27 ~3 min linux 📦zip
✔️ 67d89f4 #1 2024-04-08 09:00:51 ~4 min ios 📦zip
✔️ 67d89f4 #1 2024-04-08 09:01:37 ~5 min android 📦aar
✔️ 67d89f4 #1 2024-04-08 09:37:45 ~41 min tests 📄log
✔️ 3c52df5 #2 2024-04-08 15:29:44 ~3 min ios 📦zip
✔️ 3c52df5 #2 2024-04-08 15:29:50 ~3 min linux 📦zip
✔️ 3c52df5 #2 2024-04-08 15:31:45 ~5 min android 📦aar
✔️ 3c52df5 #2 2024-04-08 16:08:35 ~42 min tests 📄log
✔️ caf512a #3 2024-04-08 15:55:16 ~1 min linux 📦zip
✔️ caf512a #3 2024-04-08 15:55:44 ~2 min android 📦aar
✔️ caf512a #3 2024-04-08 15:57:07 ~3 min ios 📦zip
✔️ caf512a #3 2024-04-08 16:48:55 ~40 min tests 📄log
✔️ c126eb3 #4 2024-04-08 22:18:32 ~1 min linux 📦zip
✔️ c126eb3 #4 2024-04-08 22:18:48 ~1 min android 📦aar
✔️ c126eb3 #4 2024-04-08 22:19:59 ~3 min ios 📦zip
✔️ c126eb3 #4 2024-04-08 22:56:54 ~39 min tests 📄log
✔️ ce29cce #5 2024-04-09 18:51:50 ~1 min linux 📦zip
✔️ ce29cce #5 2024-04-09 18:52:09 ~1 min android 📦aar
✔️ ce29cce #5 2024-04-09 18:53:30 ~2 min ios 📦zip
✖️ ce29cce #5 2024-04-09 19:24:30 ~33 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ e532b61 #6 2024-04-09 18:53:20 ~1 min linux 📦zip
✔️ e532b61 #6 2024-04-09 18:53:46 ~1 min android 📦aar
✔️ e532b61 #6 2024-04-09 18:56:14 ~2 min ios 📦zip
✖️ e532b61 #6 2024-04-09 19:58:24 ~33 min tests 📄log
✖️ e532b61 #7 2024-04-09 20:47:47 ~33 min tests 📄log
✔️ 21cccbe #7 2024-04-09 21:30:22 ~1 min linux 📦zip
✔️ 21cccbe #7 2024-04-09 21:30:28 ~1 min android 📦aar
✔️ 21cccbe #7 2024-04-09 21:32:04 ~3 min ios 📦zip
✔️ 21cccbe #8 2024-04-09 22:08:04 ~39 min tests 📄log

@kounkou kounkou requested a review from jrainville April 8, 2024 09:10
Copy link
Member

@jrainville jrainville left a comment

Choose a reason for hiding this comment

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

Does the Edit shared addresses flow still work with this?
Let's say you had all accounts selected at first, go in the edit popup and unselect the address with ETH, does the popup show correctly that we do not have ETH?

protocol/messenger_communities.go Outdated Show resolved Hide resolved
protocol/messenger_communities.go Outdated Show resolved Hide resolved
@kounkou
Copy link
Contributor Author

kounkou commented Apr 8, 2024

Does the Edit shared addresses flow still work with this? Let's say you had all accounts selected at first, go in the edit popup and unselect the address with ETH, does the popup show correctly that we do not have ETH?

Yes, here is a video recording

Screencast.from.2024-04-08.07.59.35.AM.webm

@jrainville
Copy link
Member

Yes, here is a video recording

In the video, we can see that it didn't work as intended. The Account with 0.15 ETH was unselected, and still, all the channel were marked a Can Post.
What should have happened is that when unselecting the account, the permissioned channels should have been marked as Can't post in the popup.

@kounkou
Copy link
Contributor Author

kounkou commented Apr 8, 2024

Yes, here is a video recording

In the video, we can see that it didn't work as intended. The Account with 0.15 ETH was unselected, and still, all the channel were marked a Can Post. What should have happened is that when unselecting the account, the permissioned channels should have been marked as Can't post in the popup.

Oh right, I misread your sentence. I think while preparing the code for testing I inadvertently changed something, let me double check. This flow should already work.
Here it is...

Screencast.from.2024-04-08.02.28.18.PM.webm

@kounkou kounkou force-pushed the fix-view-post-token-permitted-channel branch from caf512a to c126eb3 Compare April 8, 2024 22:16
Copy link
Member

@jrainville jrainville left a comment

Choose a reason for hiding this comment

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

Ok, seeing the new video, the behaviour seems correct now. The fact that the chat content in the background updates at the same time is known and will be addressed in another issue

We are now able to lock consistently the channel if we do not have
permission to view&post.
This PR fixes status-im/status-desktop#14117
@kounkou kounkou force-pushed the fix-view-post-token-permitted-channel branch from e532b61 to 21cccbe Compare April 9, 2024 21:28
@kounkou kounkou merged commit 3ec73cd into release/0.177.x Apr 9, 2024
6 checks passed
@kounkou kounkou deleted the fix-view-post-token-permitted-channel branch April 9, 2024 22:08
kounkou added a commit that referenced this pull request Apr 10, 2024
We are now able to lock consistently the channel if we do not have
permission to view&post.
This PR fixes status-im/status-desktop#14117
kounkou added a commit that referenced this pull request Apr 10, 2024
We are now able to lock consistently the channel if we do not have
permission to view&post.
This PR fixes status-im/status-desktop#14117
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.

Community: View&Post token-permitted channel input is not locked when requirements do not match
4 participants