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

feat: configurator team, label, permission&member #388

Merged
merged 48 commits into from
Dec 4, 2023

Conversation

Tguntenaar
Copy link
Collaborator

@Tguntenaar Tguntenaar commented Nov 1, 2023

Resolves #315

PREVIEW
comment down below if I need to add you to the moderators

Acceptance Criteria:

  • An "Admin" link is visible in the header navigation, visible and accessible only to DevHub admins (not community admins!)

  • When a DevHub admin clicks the Admin button, they see a settings page with the existing section for configuring the featured communities + a new section for configuring the label permissions

  • Make the UI more intuitive for label permissions so that it is clear for a moderator how they can perform the common tasks

In my last call with ori about issue #129 we discussed that I should rethink the formally known Teams page into an intuitive UI. I have made two major improvents and some minor:

  1. We decided to simplify the task by only allowing 1 label per team. This made possible that the user can create a label and team at the same time instead of seperate and having to link them together afterwards.

  2. The next improvement I made is removing most of the buttons for every action. Before we had a create, an edit and delete button/toggle for every members, labels, teams, permissions. Now this all abstracted away by simply creating, editing and deleting a team in one configurator.

  3. We decided to make a clear distinction between the moderators group and other groups.

Additionally:
The moderators description (in the devhub.near contract) should be changed to:
The moderator group has permissions to edit any posts, and apply all labels including restricted ones.

@Tguntenaar Tguntenaar self-assigned this Nov 1, 2023
@Tguntenaar Tguntenaar added the enhancement New feature or request label Nov 1, 2023
@Tguntenaar Tguntenaar marked this pull request as ready for review November 17, 2023 17:50
src/devhub/page/admin.jsx Outdated Show resolved Hide resolved
src/devhub/page/admin.jsx Outdated Show resolved Hide resolved
@ailisp
Copy link
Collaborator

ailisp commented Nov 23, 2023

The UI looks awesome! However, I have trouble add another moderator, get this banner popup:

 Do you add the newest member or clear the field? 
image

For editing zk team, the issue is if not change team name, it says " This team name already exists ". This should be a valid operation: change something of this team but not team name

@Tguntenaar
Copy link
Collaborator Author

Tguntenaar commented Nov 23, 2023

Hi @ailisp! Thank you for reviewing. As I mentioned in the issue here. I still had to update the preview with the latest changes which I'm planning to do today/tomorrow.

@Tguntenaar
Copy link
Collaborator Author

Hi @ailisp! It is ready for review, I have updated the tests and the preview.

@ailisp
Copy link
Collaborator

ailisp commented Nov 27, 2023

@Tguntenaar I tested, UI and the functionalities to edit label permissions look great! There're some bugs:

  • After add/remove a featured community, redirect back from wallet, it's succeed but not reflected on the UI. The changes only visible after refresh.
  • When I try adding boatnear.near as moderator, I got this error:
{"index":1,"kind":{"ExecutionError":"Smart contract panicked: panicked at 'Member does not exist', src/access_control/members.rs:201:52"}}

@Tguntenaar
Copy link
Collaborator Author

@ailisp Hmm that's strange I did push a fix for that exact error. I will look into it and fix it.

@Tguntenaar
Copy link
Collaborator Author

@ailisp I did some testing and can reproduce your error. It seems that after a transaction is made, the getAllCommunitiesMetadata function is called 4 times total. First time it returns null, than an array with the length of 4 (old data) and than the updated array with a length of 5 (after the transaction data). However somehow the useState hook trips and the visible data on the page shows the array of 4. I've not been able to resolve this issue, any ideas?

@ailisp
Copy link
Collaborator

ailisp commented Nov 29, 2023

This might be an unsolved problem. I checked other useState usage in DevHub and they face the same problem, for example edit community page, add an addon and save.
useState is like initState + set up a function for State.update, then initState is hooked by VM to execute only once. We need a mechanism that when redirect back from wallet, call the update state. Since Peter is working on avoid confirmation screen issue, a fix to this useState problem can be out of date after Peter's work. So I recommend leave this as is and revisit after Peter's work

@ailisp
Copy link
Collaborator

ailisp commented Nov 30, 2023

@Tguntenaar I still get

{"index":0,"kind":{"ExecutionError":"Smart contract panicked: panicked at 'Member does not exist', src/access_control/members.rs:201:52"}}

error when trying to add boatnear.near as a moderator on your preview

@Tguntenaar
Copy link
Collaborator Author

@ailisp Hmm, I might have overwritten the preview with another version. I deployed again and it worked for me. Could you try it once more?

Screenshot 2023-11-30 at 19 15 27

@ailisp
Copy link
Collaborator

ailisp commented Dec 1, 2023

@Tguntenaar I have trouble to test it as currently wallet seems having trouble to connect to RPC nodes (certainly not related to your PR)
image

I'll try again later

@ailisp
Copy link
Collaborator

ailisp commented Dec 4, 2023

Today wallet fails due to network congestion is resolved. This problem also doesn't exist now:

@ailisp I did some testing and can reproduce your error. It seems that after a transaction is made, the getAllCommunitiesMetadata function is called 4 times total. First time it returns null, than an array with the length of 4 (old data) and than the updated array with a length of 5 (after the transaction data). However somehow the useState hook trips and the visible data on the page shows the array of 4. I've not been able to resolve this issue, any ideas?

Maybe it's also due to network congestion or there was fix on the gateway side

@ailisp ailisp merged commit 91d3bec into NEAR-DevHub:main Dec 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Admin] Give DevHub moderators the ability to manage label permissions on the admin page
2 participants