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

Restricted custom fields #3495

Merged
merged 1 commit into from
Dec 17, 2019
Merged

Restricted custom fields #3495

merged 1 commit into from
Dec 17, 2019

Conversation

romcheg
Copy link
Contributor

@romcheg romcheg commented Dec 11, 2019

Add an optional managing group property to custom fields. If set,
adding, changing or deleting values of those custom fields will
be restricted to mebmerb of a specified groups only.

Custom field form will not show such custom field values to
unauthorized users.

@coveralls
Copy link

coveralls commented Dec 12, 2019

Coverage Status

Coverage increased (+0.04%) to 84.494% when pulling e930064 on romcheg:customfields_group_2 into efb5ce6 on allegro:ng.

@romcheg romcheg changed the title [WIP] Add managing group to custom fields Add managing group to custom fields Dec 12, 2019
@romcheg romcheg requested a review from ar4s December 12, 2019 14:48
Blejwi
Blejwi previously approved these changes Dec 13, 2019
MarekBleschke
MarekBleschke previously approved these changes Dec 16, 2019
def _user_can_manage_customfield(self, user, custom_field):
return (
custom_field.managing_group is None or
custom_field.managing_group in user.groups.all()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try user.groups.filter(pk=custom_field.managing_group.pk).exists()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks more efficient. Thanks!

def __init__(
self, data=None, files=None, instance=None, save_as_new=None,
prefix=None, queryset=None, request=None, **kwargs
):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can write def __init__(self, request=None, *args, **kwargs): if you only need request. Also super clause should simplify to super().__init__(request=request, queryset=queryset, *args, **kwargs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

)

super().__init__(
data=data, files=files, instance=instance, save_as_new=save_as_new,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not expected there.

@@ -135,3 +135,42 @@ def changeform_view(
return super().changeform_view(
request, object_id, form_url, extra_context
)

def _create_formsets(self, request, obj, change):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you use this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It it used internally by the ModelAdmin class

@romcheg romcheg dismissed stale reviews from MarekBleschke and Blejwi via 8e57a4d December 16, 2019 14:07
ar4s
ar4s previously approved these changes Dec 16, 2019
Blejwi
Blejwi previously approved these changes Dec 16, 2019
Add an optional managing group property to custom fields. If set,
adding, changing or deleting values of those custom fields will
be restricted to mebmerb of a specified groups only.

Custom field form will not show such custom field values to
unauthorized users.
@romcheg romcheg dismissed stale reviews from Blejwi and ar4s via e930064 December 17, 2019 09:00
@romcheg romcheg changed the title Add managing group to custom fields Restricted custom fields Dec 17, 2019
@romcheg
Copy link
Contributor Author

romcheg commented Dec 17, 2019

The commits have been squashed. Since no other changes have been made, I'm going to merge this with a single approve.

@romcheg romcheg merged commit 65116e9 into allegro:ng Dec 17, 2019
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.

None yet

5 participants