Resolve "Manage permissions for persons (users) and groups (Django groups) in frontend" #2178

Merged
Owner

Closes #285

Closes #285
Author
Owner

List view

image

Select modal

image

Assign permission form

image

### List view ![image](/uploads/b5da84dc98296eb3ddc4430ce29c1975/image.png) ### Select modal ![image](/uploads/b124cb1a4c12beb5425c93c0ec94ba48/image.png) ### Assign permission form ![image](/uploads/edcb322cee4525dd1d7081be1911c698/image.png)
Author
Owner

added 4 commits

  • 885cd0c1...3b5b720d - 3 commits from branch master
  • af90190f - Add support for managing permissions in the frontend

Compare with previous version

added 4 commits <ul><li>885cd0c1...3b5b720d - 3 commits from branch <code>master</code></li><li>af90190f - Add support for managing permissions in the frontend</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=6530&start_sha=885cd0c1b6ad28543fb5e74dbbe7cb6b3c47cb1a)
Author
Owner

assigned to @fph and unassigned @hansegucker

assigned to @fph and unassigned @hansegucker
Author
Owner

assigned to @nik and unassigned @fph

assigned to @nik and unassigned @fph
Author
Owner

requested review from @ZugBahnHof

requested review from @ZugBahnHof
Author
Owner

added 94 commits

  • af90190f...1c4be21f - 93 commits from branch master
  • 930f3723 - Merge branch 'master' into...

Compare with previous version

added 94 commits <ul><li>af90190f...1c4be21f - 93 commits from branch <code>master</code></li><li>930f3723 - Merge branch &#39;master&#39; into...</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=6532&start_sha=af90190fe4138f51a30e565813d5a167c43267b4)
Author
Owner

added 3 commits

  • 930f3723...262b664b - 2 commits from branch master
  • 034b17dd - Merge branch 'master' into...

Compare with previous version

added 3 commits <ul><li>930f3723...262b664b - 2 commits from branch <code>master</code></li><li>034b17dd - Merge branch &#39;master&#39; into...</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=6557&start_sha=930f37234dd800410fc1769d672685626aeca0d9)
Author
Owner

added 1 commit

  • 78920a4b - Refactor modal for assigning permissions to card to avoid JS issues

Compare with previous version

added 1 commit <ul><li>78920a4b - Refactor modal for assigning permissions to card to avoid JS issues</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=6558&start_sha=034b17dde91194541cba542cfee346b2d7192161)
Author
Owner

added 1 commit

Compare with previous version

added 1 commit <ul><li>a7a1f720 - Fix typo</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=6559&start_sha=78920a4b24e6b030807be2e651831a0d278fbf13)
Author
Owner

added 1 commit

  • f4ac489e - Remove deprecated mechanism for getting count of newly created permissions

Compare with previous version

added 1 commit <ul><li>f4ac489e - Remove deprecated mechanism for getting count of newly created permissions</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=6560&start_sha=a7a1f720b503dcba21d937f2722d809ca45c28d2)
Author
Owner

added 73 commits

  • f4ac489e...183a0dbf - 72 commits from branch master
  • f0e29a55 - Merge branch 'master' into...

Compare with previous version

added 73 commits <ul><li>f4ac489e...183a0dbf - 72 commits from branch <code>master</code></li><li>f0e29a55 - Merge branch &#39;master&#39; into...</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=7411&start_sha=f4ac489e10fa22a5a91811ca835eaf88c0b8587e)
Author
Owner

added 1 commit

  • 178df480 - Add views to delete permissions

Compare with previous version

added 1 commit <ul><li>178df480 - Add views to delete permissions</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=7424&start_sha=f0e29a557b7dcfe07efd0c4903f5b0d59fc3b0b0)
Author
Owner

marked this merge request as ready

marked this merge request as **ready**
Owner

Does this / should this include the possibility for persons to add object permissions on themselves / groups they own?

Does this / should this include the possibility for persons to add object permissions on themselves / groups they own?
Author
Owner

No, it doesn't and adding this function makes things much more complicate.

No, it doesn't and adding this function makes things much more complicate.
Member

I have two questions:

  1. Why don't you use "real" frontend tabs and simply switch between the tables, instead of having multiple URLs and views?
  2. Why do you have a multipage form in two different designs, one in a modal (Popup) and the other one on a different page?
I have two questions: 1. Why don't you use "real" frontend tabs and simply switch between the tables, instead of having multiple URLs and views? 2. Why do you have a multipage form in two different designs, one in a modal (Popup) and the other one on a different page?
Member

Please make sure the user will know, this is a form to filter, which is purely optional. You could maybe even hide and toggle it using JS, but at least a subheading like e.g. "Filter results" would be nice.

Please make sure the user will know, this is a form to filter, which is purely optional. You could maybe even hide and toggle it using JS, but at least a subheading like e.g. "Filter results" would be nice.
Member

I'm not sure whether this is a permission for something, or rather a permission to do something or even a permission on a certain topic.

Examples:

  1. You have the permission to delete users.
  2. I would like to have the permission for changing a theme.
  3. Can I have permissions on this server?
I'm not sure whether this is a permission _for_ something, or rather a permission _to_ do something or even a permission _on_ a certain topic. Examples: 1. You have the permission to delete users. 2. I would like to have the permission for changing a theme. 3. Can I have permissions on this server?
Author
Owner

added 11 commits

  • 178df480...c7fae2a0 - 10 commits from branch master
  • 915790ff - Merge branch 'master' into...

Compare with previous version

added 11 commits <ul><li>178df480...c7fae2a0 - 10 commits from branch <code>master</code></li><li>915790ff - Merge branch &#39;master&#39; into...</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=7594&start_sha=178df4807484d606984e36ddc200c10ca4f898fa)
Author
Owner
  1. Why don't you use "real" frontend tabs and simply switch between the tables, instead of having multiple URLs and views?

There are multiple reasons. For example, after submitting the permission assignment form one should be redirected automatically to the matching table. Then, additionally, it makes things much more complex to include four different models and filter forms in one view.

  1. Why do you have a multipage form in two different designs, one in a modal (Popup) and the other one on a different page?

To build the assignment form and show the correct entries for the objects list, we must know the content type/model before we render the form.

> 1. Why don't you use "real" frontend tabs and simply switch between the tables, instead of having multiple URLs and views? There are multiple reasons. For example, after submitting the permission assignment form one should be redirected automatically to the matching table. Then, additionally, it makes things much more complex to include four different models and filter forms in one view. > 1. Why do you have a multipage form in two different designs, one in a modal (Popup) and the other one on a different page? To build the assignment form and show the correct entries for the objects list, we must know the content type/model **before** we render the form.
Author
Owner

Ok, in this case, it is a permission on something.

Ok, in this case, it is a permission _on_ something.
Author
Owner

resolved all threads

resolved all threads
Author
Owner

changed this line in version 10 of the diff

changed this line in [version 10 of the diff](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=7598&start_sha=915790ff96220fcc0ea817eb8e796d827434ac2c#622da2712f4801473aba0b611c3232f8148ef126_414_414)
Author
Owner

added 1 commit

  • 17189375 - Improve layout of permission management views

Compare with previous version

added 1 commit <ul><li>17189375 - Improve layout of permission management views</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=7598&start_sha=915790ff96220fcc0ea817eb8e796d827434ac2c)
Author
Owner

@ZugBahnHof Please fix the frontend.

@ZugBahnHof Please fix the frontend.
Owner

I see no reaction to this request.

I see no reaction to this request.
Owner

I somehow remeber we used to translate our strings…

I somehow remeber we used to translate our strings…
Owner

Please split the strings so translation will stay in sync.

What is target="_self" suppoed to do? The target attribute is forbidden, and _self is the default anyway…

Please split the strings so translation will stay in sync. What is `target="_self"` suppoed to do? The `target` attribute is forbidden, and `_self` is the default anyway…
Owner

Don't do that.

There should always be exactly one way of doing things, and doing text search is done using haystack in AlekSIS.

(You will remember that we already got a long-standing security issue with haystack, we don't need a second mechanism with the same security issue)

Don't do that. There should always be exactly one way of doing things, and doing text search is done using haystack in AlekSIS. (You will remember that we already got a long-standing security issue with haystack, we don't need a second mechanism with the same security issue)
Author
Owner

Materialize can be ugly sometimes: https://materializecss.com/tabs.html#external

Materialize can be ugly sometimes: https://materializecss.com/tabs.html#external
Author
Owner

There should always be exactly one way of doing things, and doing text search is done using haystack in AlekSIS.

We also make use of django-filter for filtering tables and that is the supposed use case.

> There should always be exactly one way of doing things, and doing text search is done using haystack in AlekSIS. We also make use of `django-filter` for filtering tables and that is the supposed use case.
Author
Owner

@ZugBahnHof Any progress on this?

@ZugBahnHof Any progress on this?
Owner

marked this merge request as draft

marked this merge request as **draft**
Author
Owner

added 675 commits

  • 17189375...5261b30c - 674 commits from branch master
  • 36388114 - Merge branch 'master' into...

Compare with previous version

added 675 commits <ul><li>17189375...5261b30c - 674 commits from branch <code>master</code></li><li>36388114 - Merge branch &#39;master&#39; into...</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=13186&start_sha=17189375a54ec502aefbd4af6377c93f1b4c6351)
Author
Owner

@nik Any response?

@nik Any response?
Owner
https://django-haystack.readthedocs.io/en/master/multiple_index.html
Owner
            if isinstance(field, (CharField, TextField)):
```suggestion:-0+0 if isinstance(field, (CharField, TextField)): ```
Author
Owner

changed this line in version 12 of the diff

changed this line in [version 12 of the diff](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=13762&start_sha=36388114a020947d94567ee3cfab424fc45b5915#31aaf5cb8764acc77c91f36a9c348108a933d0d3_17_17)
Author
Owner

changed this line in version 12 of the diff

changed this line in [version 12 of the diff](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=13762&start_sha=36388114a020947d94567ee3cfab424fc45b5915#9104f183b254581056a368588770f82082815153_281_281)
Author
Owner

changed this line in version 12 of the diff

changed this line in [version 12 of the diff](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=13762&start_sha=36388114a020947d94567ee3cfab424fc45b5915#9104f183b254581056a368588770f82082815153_285_285)
Author
Owner

added 45 commits

  • 36388114...b8e230b9 - 40 commits from branch master
  • f8d5017c - Simplify and rename get_search_fields to get_filter_fields
  • eef8c0bf - Merge branch 'master' into...
  • f4a2e38f - Update changelog
  • c25ade6b - Add missing trans tag
  • a38f53b5 - Reformat

Compare with previous version

added 45 commits <ul><li>36388114...b8e230b9 - 40 commits from branch <code>master</code></li><li>f8d5017c - Simplify and rename get_search_fields to get_filter_fields</li><li>eef8c0bf - Merge branch &#39;master&#39; into...</li><li>f4a2e38f - Update changelog</li><li>c25ade6b - Add missing trans tag</li><li>a38f53b5 - Reformat</li></ul> [Compare with previous version](/AlekSIS/official/AlekSIS-Core/-/merge_requests/533/diffs?diff_id=13762&start_sha=36388114a020947d94567ee3cfab424fc45b5915)
Author
Owner

marked this merge request as ready

marked this merge request as **ready**
Author
Owner

resolved all threads

resolved all threads
Author
Owner

removed review request for @ZugBahnHof

removed review request for @ZugBahnHof
nik scheduled this pull request to auto merge when all checks succeed 2021-11-28 21:11:28 +01:00
Author
Owner

resolved all threads

resolved all threads
Owner

mentioned in commit 02f693133c

mentioned in commit 02f693133cf65facf09b80bd0c33b98a3dbff6f8
nik merged commit 02f693133c into master 2021-11-28 21:24:40 +01:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
aleksis/AlekSIS-Core!2178
No description provided.