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(orgs): split creation flows + add new kind of roles based organization #1429

Merged
merged 106 commits into from
Dec 5, 2024

Conversation

MikaelVallenet
Copy link
Member

@MikaelVallenet MikaelVallenet commented Nov 29, 2024

Integration of Roles Manager into Organizations Flow

The roles manager allow to manage roles & permissions on a user base.
This PR integrates it into our DAO gno contracts

Exhaustive list of additions:

  • Add a new Module called RolesModule within DAO Core with its own interface see gno/p/dao_interfaces
  • add a new gno pure package dao_roles_group that implements the interface IRolesModule
  • Add these functions: IsRoleExist, CountRoles, GetUserRoles(user) into role manager pure pkg
  • Refactor Organization flow to split the logic (except the first step that is shared)
  • Remove token based logic (was not working for now anyway)
  • Reset form data on structure type change & on tx cancel
  • Reset unlocked step on structure type change
  • Add a roles setting step for roles based struct with unlimited list of roles with color
  • Add an input to set roles to members in roles based dao
  • Adapt the generated gno contract to work roles based dao
  • Refactor project structure for e2e gno tests (out of scope, but since it's a small adjustement i don't think i should open an atomir PR just for this, waste of time imo)
  • Add of register the dao profile (displayName, description, ImageURI) => i will move this to an atomic PR
  • move getDuration && getPercent into utils/gnodao/helpers.ts
  • Remove and replace all stylesheet.Create in organizations/**

Membership based and Roles dao based share the same contracts, just the membership are roles dao based organization without any roles

MikaelVallenet and others added 30 commits October 10, 2024 16:52
@@ -37,3 +37,17 @@ type IProposalModule interface {
}

type ProposalModuleFactory func(core IDAOCore) IProposalModule

type IRolesModule interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMHO, it could be nice and more practice if we handle AddressOrName instead of only Address

Copy link
Collaborator

Choose a reason for hiding this comment

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

I disagree, if we do we have to add a way to resolve users by passing a resolver in the role module, increasing gas cost, dependencies and complexity
then if at some point usernames can change, we get a lot of headache
IMO it's better to always operate on addresses where possible

@clegirar
Copy link
Collaborator

clegirar commented Dec 4, 2024

Screenshot 2024-12-04 at 21 57 53
A little confusing fontSize here

Screenshot 2024-12-04 at 22 19 10
I have 2 things to say here, thrash icons are too big imo, and maybe you can fit the thrash icon with the end of the container ? Like a full width
In term of components behavior, on the configuring voting tab, all <TextInputCustom<ConfigureVotingFormType> are not clickable and just scrollable (out of scope i think btw), i think the best is to have both possibilities

Screenshot 2024-12-04 at 22 21 19
In the recap at the end of the form, just before the creation maybe we can have the colors for each role ?

Screenshot 2024-12-04 at 22 36 49
At the end of the creation, after the validated animation, the middle circle stay here like that ? Maybe this behaviour was already here before (I'll check, but found that weird, and in the same time we have a call-to-action component, so idk maybe i'm tickling)

Screenshot 2024-12-04 at 22 40 02
Maybe create a role tab for a role based DAO ?

@MikaelVallenet
Copy link
Member Author

MikaelVallenet commented Dec 5, 2024

Screenshot 2024-12-04 at 21 57 53 A little confusing fontSize here

Screenshot 2024-12-04 at 22 19 10 I have 2 things to say here, thrash icons are too big imo, and maybe you can fit the thrash icon with the end of the container ? Like a full width In term of components behavior, on the configuring voting tab, all <TextInputCustom<ConfigureVotingFormType> are not clickable and just scrollable (out of scope i think btw), i think the best is to have both possibilities

Screenshot 2024-12-04 at 22 21 19 In the recap at the end of the form, just before the creation maybe we can have the colors for each role ?

Screenshot 2024-12-04 at 22 36 49 At the end of the creation, after the validated animation, the middle circle stay here like that ? Maybe this behaviour was already here before (I'll check, but found that weird, and in the same time we have a call-to-action component, so idk maybe i'm tickling)

Screenshot 2024-12-04 at 22 40 02 Maybe create a role tab for a role based DAO ?

Thanks you a lot for the review 🙏
For now since you did not see any crash / problem in the logic, i would prefer to focus on continue building instead of invest time on it. For the role tab it's plan in the next PRs
I propose i open issues about what you said after this PR merge so i can do atomic PR later ❤️
wdyt (i would like to iterate fast to find the good approach of managing roles, then polish it for production)

@n0izn0iz n0izn0iz changed the title feat: refactor & split each flow + add new kind of roles based organization feat(orgs): refactor & split each creation flow + add new kind of roles based organization Dec 5, 2024
@n0izn0iz n0izn0iz changed the title feat(orgs): refactor & split each creation flow + add new kind of roles based organization feat(orgs): split creation flows + add new kind of roles based organization Dec 5, 2024
@n0izn0iz n0izn0iz merged commit b843b86 into main Dec 5, 2024
25 checks passed
@n0izn0iz n0izn0iz deleted the dev/mikaelvallenet/feat/push-role-manager-into-dao branch December 5, 2024 11:20
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.

4 participants