-
Notifications
You must be signed in to change notification settings - Fork 67
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
[SYNPY-1348] Add User profile and Permission models #1058
Conversation
Hello @BryanFauble! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-01-30 23:14:52 UTC |
docs/scripts/object_orientated_programming_poc/oop_poc_user_and_permission.py
Fixed
Show fixed
Hide fixed
docs/scripts/object_orientated_programming_poc/oop_poc_user_and_permission.py
Fixed
Show fixed
Hide fixed
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.
looks great. I really like the mixin implementation. Only thing is permission issues in the demo script. I'll come back to approve once I can run it to completion
docs/scripts/object_orientated_programming_poc/oop_poc_user_and_permission.py
Show resolved
Hide resolved
Co-authored-by: Brad Macdonald <52762200+BWMac@users.noreply.github.com>
…networks/synapsePythonClient into SYNPY-1348-user-profile
docs/scripts/object_orientated_programming_poc/oop_poc_user_and_permission.py
Dismissed
Show dismissed
Hide dismissed
docs/scripts/object_orientated_programming_poc/oop_poc_user_and_permission.py
Dismissed
Show dismissed
Hide dismissed
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.
@BryanFauble It works now, thanks for updating the permissions.
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.
Just a note, Evaluation objects in Synapse have a different access control list.
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.
Good to know. When that gets created we may need to make a mixin for that specifically than
"""Should messages that are emailed to the user be marked as | ||
READ in Synapse? Default false.""" | ||
|
||
preferences: Optional[List[UserPreference]] = field(default_factory=list) |
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.
Nit: Not related, but I don't understand this preferences
parameter. I took a look at the API and it doesn't tell you much about it either...
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 was messing around with this as well since it isn't explained anywhere. It let's you set arbitrary booleans on your account. For example if you get the userprofile for my account i set an example value. bfauble
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.
🔥 LGTM!
|
Problem:
Solution:
Testing: