-
Notifications
You must be signed in to change notification settings - Fork 440
feat(boards2): change realm to use gno.land/p/gnoland/boards
#4939
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
base: master
Are you sure you want to change the base?
feat(boards2): change realm to use gno.land/p/gnoland/boards
#4939
Conversation
This is required to be able to use `gno.land/p/gnoland/boards` package. Render code can eventually be improved potentially by introducing a render pattern to make rendering flexible.
This refactor removes basic types from the realm in favor of using types from `gno.land/p/gnoland/boards` package.
Some of the functions were not using `WithPermission` for simplicity, but using it is necessary to give flexibility to boards when using custom permissions implementations. This way it's possible to better customize independent boards behavior.
Order changed to better group arguments by type and save horizontal space.
Freezing replies makes no sense.
This package contains a custom CommonDAO member storage that makes it easier and cheaper to boards2 cuatom permissions implementation to integrate with CommonDAO. The custom storage allows using groups to organize roles. It allows saving board members into groups and at the same time automatically add/remove them to a "parent" storage to simplify and improve the basic permissions implementation.
This allows devs to directly modify dao members without going though the basic permissions type to update users.
This allows any member to leave a board.
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
gno.land/p/gnoland/boardsgno.land/p/gnoland/boards
| // NewMemberStorage creates a new CommonDAO member storage with grouping support. | ||
| // | ||
| // This is a custom storage that automatically adds or removes members that are added | ||
| // or removed from any of the member groups. This allows for quick and inexpensive | ||
| // checks for the number of total unique storage users, including users added to groups, | ||
| // and also to iterate all of them without needing to iterate individual groups. | ||
| func NewMemberStorage() commondao.MemberStorage { | ||
| // Create a new broker to allow storages to publish and subscribe to messages | ||
| // It is used to add/remove users from the storage each time one or more groups change. | ||
| broker := message.NewBroker() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I though of replacing the default commondao member storage implementation with this one, but I didn't because I think this one is opinionated, it uses a message broker to update the list of members in the storage when a member is added or removed from any storage group. A member storage can have multiple groups that can be used for example for tiers or roles.
This custom member storage automatically updates members from group members to make it easier to iterate all members that exists within the groups, in a cheaper way that doesn't require to potentially traversing multiple AVL trees, one within each group, but instead just traverse a single storage AVL tree. It also simplifies listing all the groups a member belongs to without traversing multiple trees, or getting the total number of members.
Finally, this custom member storage is being used within Boards2 realm, in the BasicPermissions implementation. It uses grouping to group board members by role.
| gLastReservedID BoardID | ||
| gBoardsByID avl.Tree // string(id) -> *Board | ||
| gListedBoardsByID avl.Tree // string(id) -> *Board | ||
| gBoardsByName avl.Tree // string(lowercase name) -> *Board | ||
| gListedBoardsByID avl.Tree // string(id) -> *boards.Board |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these global vars now use abstractions defined in gno.land/p/gnoland/boards. Vars gBoardsByID and gBoardsByName are removed in favor of using boards storage:
gBoards = boards.NewStorage()Var gLastReservedID has been replaced by an identifier generator:
gBoardsSequence = boards.NewIdentifierGenerator()| // This type is a default implementation to handle users, roles and permissions. | ||
| func NewBasicPermissions(dao *commondao.CommonDAO) *BasicPermissions { | ||
| if dao == nil { | ||
| panic("basic permissions require a DAO") | ||
| } | ||
|
|
||
| // It uses an underlying DAO to manage users and roles. | ||
| func NewBasicPermissions() *BasicPermissions { | ||
| s := storage.NewMemberStorage() | ||
| return &BasicPermissions{ | ||
| dao: dao, | ||
| roles: avl.NewTree(), | ||
| users: avl.NewTree(), | ||
| validators: avl.NewTree(), | ||
| public: avl.NewTree(), | ||
| dao: commondao.New( | ||
| // Use a custom boards member storage | ||
| commondao.WithMemberStorage(s), | ||
| ), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BasicPermissions changed to always create an underlying DAO which now is used to track users as members and roles as groups, this removes redundancy and potential inconsistencies between permissions' users and roles vs DAO members if members are modified outside the permissions instance
| type Flaggable interface { | ||
| // Flag adds a new flag to an item. | ||
| // It returns false when the user flagging already flagged the item. | ||
| Flag(user address, reason string) bool | ||
|
|
||
| // FlagsCount returns number of times item was flagged. | ||
| FlagsCount() int | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a Flaggable interface doesn't make sense, is not needed
|
PR modifies many files but most of them are filetests that are now referencing the boards package intsead of the realm. Changes are usually simple replacements. |
PR changes Boards2 realm to use
gno.land/p/gnoland/boardspackage which makes the realm leaner.It also improves
BasicPermissionsimplementation by using a custom members storage implementation that makes users iteration cheaper.