-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add permission manager #158
base: master
Are you sure you want to change the base?
Conversation
First part of permission manager system. Add page to view current office permission and current user permissions. Also shows current offices with no permissions and current users with no permissions. TODO: have each entry link to an edit page for that user
Clicking the name of the office or user will lead to a page to edit permissions. TODO: implement edit page.
import sqlalchemy | ||
from ruddock.resources import Permissions | ||
|
||
def fetch_office_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.
Why do we need the function to do this? Can't we just have it return the permissions of all offices, and also the is_active column?
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 mean I suppose lol, does it matter?
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.
ahh, i tried it out with more thorough test data, and now i see it. you wanna show every office that's got permissions, even if they're not active, in case you need to remove them. likewise, you want to show all active offices, even if they have no permissions, in case you want to manage them.
same with users.
if that's the case, lemme suggest the following (you gotta have the extra any_permissions column because there's a bug where you can't sort on expressions involving aliases: https://bugs.mysql.com/bug.php?id=80802)
SELECT GROUP_CONCAT(permission_id) AS permissions,
permission_id IS NOT NULL AS any_permissions,
office_name, office_id, office_order, is_active
FROM offices
NATURAL LEFT JOIN office_permissions
GROUP BY office_id
HAVING is_active OR any_permissions
ORDER BY
any_permissions DESC, -- puts nulls after non-nulls
permissions,
office_order
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.
Hmm, it worked fine on my machine (using MariaDB, maybe they fixed it?). I checked and the Ruddock website is using MariaDB also so this might not be needed?
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'm also using mariadb; maybe I'm just on an older version. In any case, I think that approach makes it much clearer what you're intending to fetch there.
|
||
def delete_office_permission(office_id, perm_id): | ||
"""Deletes the specified permission from the office.""" | ||
if type(perm_id) is str or type(perm_id) is unicode: |
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.
man, py3 makes this much nicer; maybe we should consider that someday
far in the future
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.
later later
return flask.g.db.execute(query, o_id=office_id, p_id=perm_id) | ||
|
||
|
||
def fetch_user_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.
Same as before, why does this return a table and also a subset of it?
+ str(type(string))) | ||
return [get_perm_name(int(x)) for x in string.split(sep)] | ||
|
||
def decode_perm_string_with_id(string, sep=","): |
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.
👍
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! 😄
UX suggestion: having a "back to permission manager" button might be good
Also, I managed to remove admin powers from everyone and every office. Since that can always be fixed by hand in the DB, IDK if we care that much, but worth bringing up.
Which reminds me, someday we should implement logging on all these actions. Might be a good project for a newbie, so they can get acquainted with the whole site.
import sqlalchemy | ||
from ruddock.resources import Permissions | ||
|
||
def fetch_office_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.
ahh, i tried it out with more thorough test data, and now i see it. you wanna show every office that's got permissions, even if they're not active, in case you need to remove them. likewise, you want to show all active offices, even if they have no permissions, in case you want to manage them.
same with users.
if that's the case, lemme suggest the following (you gotta have the extra any_permissions column because there's a bug where you can't sort on expressions involving aliases: https://bugs.mysql.com/bug.php?id=80802)
SELECT GROUP_CONCAT(permission_id) AS permissions,
permission_id IS NOT NULL AS any_permissions,
office_name, office_id, office_order, is_active
FROM offices
NATURAL LEFT JOIN office_permissions
GROUP BY office_id
HAVING is_active OR any_permissions
ORDER BY
any_permissions DESC, -- puts nulls after non-nulls
permissions,
office_order
Allows editing office and user permissions without having to manually add rows to the database.
This was supposed to be a nice intro project for working on the website but then I just sorta stopped working on it.