Permissions cleanup (part 1) #5489
Conversation
| if get_cms_setting('PERMISSION'): | ||
| can_add = super(PageAdmin, self).has_add_permission(request) | ||
|
|
||
| if can_add and get_cms_setting('PERMISSION'): |
There was a problem hiding this comment.
Does it make sense to put something like: CMS_PERMISSION = get_cms_setting('PERMISSION') at the top of this file and then use the "constant": CMS_PERMISSION everywhere instead? Its unlikely to change between requests =)
There was a problem hiding this comment.
Yup. I'll keep it in mind for the next PR, to avoid re-running tests, etc.. on this one.
|
Great work... and long overdue! I see now why you had to write so many tests. In the end, to we go up or down in tests count and test coverage? |
|
OK to merge. Looking forward to part 2. |
|
Thanks, test count has gone up over 100 new tests. We'll have to wait on coverage to run and give us the verdict. |
czpython
force-pushed
the
feature/permissions-cleanup
branch
2 times, most recently
from
b867fc2
to
01c763a
Compare
czpython
force-pushed
the
feature/permissions-cleanup
branch
from
01c763a
to
1ec6ef7
Compare
|
@yakky Please review :) |
|
Sorry, terrible week. Ok to merge |
|
The next PR will have the changelog entries. |
This PR introduces multiple bug fixes for the permissions system, as well as small cleanups and a complete overhaul of permission related tests.
As the title suggests, this is not a finished product but it's the "core" of the cleanup, future efforts will be based on this.
High level overview:
This is because plugins should have no logic aside from the builtin admin logic
to validate and allow an add request. Placeholders on the other hand provide a way
to extend these checks in third party apps.
test_placeholder,test_adminandtest_pageinto the more specific
test_page_adminmodule.test_page_user_adminmodule for tests that interact with thePageUseradmintest_page_user_group_adminmodule for tests that interact with thePageUserGroupadmin.
test_placeholder_app_adminmodule for tests that interact with thePlaceholderAdminMixinclass.Most of the additions/changes are tests, there's several bug fixes but usually one liners.
/cc @divio/django-cms-core