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

Group refactor #50

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Group refactor #50

wants to merge 3 commits into from

Conversation

scaspin
Copy link
Collaborator

@scaspin scaspin commented Dec 5, 2022

Opening a bit to get feedback on existing modifications.

Still need to do (not in order of importance):

  • write some tests, specifically unit test groups, permissions
  • make permissions more general (right now tied directly to admin/writer/reader but can expand to be more general)
  • make sure group sets go through higher lib and inform other devices in group of changes
  • maybe: refactor prefix + key parameters in higher and in applications
  • refactor period app to work with new sharing scheme (now we add permissions per key, not to a group)
  • abstract some of the linked device / contacts info
  • make sure group info is shown to a developer (for example, you can imagine a context where you want to see what groups your book is shared with) and not removed on a get call

@scaspin scaspin marked this pull request as draft December 5, 2022 21:44
@nataliepopescu
Copy link
Collaborator

nataliepopescu commented Dec 6, 2022

my thoughts on how these todos should be prioritized (numbers refer to the bullet order):
3, 5, 7: definitely (7 should be pretty quick)
4: yes if it impacts/cleans up the API
2, 6: these are more nice-to-haves, can discuss further

@scaspin
Copy link
Collaborator Author

scaspin commented Dec 6, 2022

@nataliepopescu , agree that was very much a brain dump, the most important things imo for refining the api are:

  • developers should have more access to groups/permissions (add functions like "attachPermissions, assignGroupToReaders, seeMembers(groupId), etc...)
  • do not need to "shed" group or permissions information when returning values on getData (I don't think we do this now, but I can totally see how a developer would want to overwrite permissions by assigning a different group to the field, andt there isn't really a need to go through multiple function calls when the permissions field is treated like any other field of an object)

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.

2 participants