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

SIANXKE-380: prepare option to disable single attachments in inline #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anx-abruckner
Copy link
Collaborator

@anx-abruckner anx-abruckner commented Oct 25, 2024

@nezhar I had the case that I needed to have only specific attachments (in an in line admin) readonly/uneditable while the other attachments in the list should remain editable. Not sure how exactly to prepare this (highly custom) use case, but I added a "template" as a custom inline admin form and mentioned it briefly in the README.

  • Adds customizable InlineAdminForm with disable_inline_fields method to override if needed

Comment on lines +159 to +163
if self.disable_inline_fields():
for field in self.fields:
self.fields[field].disabled = True

Choose a reason for hiding this comment

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

Setting disabled = True on a field does not disable the delete checkbox of the surrounding template (it's mentioned as a requirement in the ticket description).

Sadly, has_xxx_permission of the inline refers to the parent model (which makes no sense at all in this case), so the custom form is a valid workaround. To disable the delete checkbox, one would probably need to overwrite the template - which is a rather painful and error-prone process in Django admin.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's true, that the checkbox is still available, but on usage the form will render a corresponding error message and prevent the deletion. I think that is OK. Another option would be to use a custom FormSet where the checkbox is simply disabled - I'll look into a way to combine this with the current approach.

Choose a reason for hiding this comment

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

I almost spat out my coffee when I saw the DELETE field in the formset. Nice solution. 👍

Comment on lines 187 to 188
class AttachmentInlineAdmin(BaseAttachmentInlineAdmin):
form = AttachmentInlineForm

Choose a reason for hiding this comment

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

🎨 I'm not quite happy with setting the form by default, because it has to be overwritten to function - and therefore the AttachmentInlineAdmin would need to be overwritten as well to set the overwritten form.

Maybe change disable_inline_fields to raise NotImplementedError and don't assign the form to AttachmentInlineAdmin and extend the description in the readme instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@christophbuermann I updated the PR so that the AttachmentInlineAdmin does not use the new DynamicallyDisabledAttachmentInlineForm and DynamicallyDisabledAttachmentInlineFormSet. Instead an examplary usage is described in the README.

@anx-abruckner anx-abruckner force-pushed the SIANXKE-380_readonlyAdminInlineFields branch from a3c3ffe to 66b344f Compare November 29, 2024 14:08
Comment on lines +166 to +176
class DynamicallyDisabledAttachmentInlineFormSet(BaseGenericInlineFormSet):
def add_fields(self, form, index):
"""
Override to disable the DELETE checkbox of entries matching a specific custom condition.
E.g. to make this dependent of the DynamicallyDisabledAttachmentInlineForm's disable_inline_fields method:

super().add_fields(form, index)
if hasattr(form, 'disable_inline_fields') and form.disable_inline_fields():
form.fields['DELETE'].disabled = True
"""
raise NotImplementedError()

Choose a reason for hiding this comment

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

If we inherit from this and use super().add_fields(form, index) we get a NotImplementedError because the method raises it. I suggest using the example implementation as the actual implementation here:

Suggested change
class DynamicallyDisabledAttachmentInlineFormSet(BaseGenericInlineFormSet):
def add_fields(self, form, index):
"""
Override to disable the DELETE checkbox of entries matching a specific custom condition.
E.g. to make this dependent of the DynamicallyDisabledAttachmentInlineForm's disable_inline_fields method:
super().add_fields(form, index)
if hasattr(form, 'disable_inline_fields') and form.disable_inline_fields():
form.fields['DELETE'].disabled = True
"""
raise NotImplementedError()
class DynamicallyDisabledAttachmentInlineFormSet(BaseGenericInlineFormSet):
def add_fields(self, form, index):
"""
Disables the DELETE checkbox of entries matching a specific custom condition.
"""
super().add_fields(form, index)
if hasattr(form, 'disable_inline_fields') and form.disable_inline_fields():
form.fields['DELETE'].disabled = True

Comment on lines +159 to +163
if self.disable_inline_fields():
for field in self.fields:
self.fields[field].disabled = True

Choose a reason for hiding this comment

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

I almost spat out my coffee when I saw the DELETE field in the formset. Nice solution. 👍

Comment on lines +86 to +95
# customize the DynamicallyDisabledAttachmentInlineFormSet to determine which inline attachments' DELETE checkbox
# should be readonly
MyCustomDynamicallyDisabledAttachmentInlineFormSet(DynamicallyDisabledAttachmentInlineFormSet):
super().add_fields(form, index)
"""
DELETE checkbox for Attachments that return True for
MyCustomDynamicallyDisabledAttachmentInlineForm's disable_inline_fields method should be disabled
"""
if hasattr(form, 'disable_inline_fields') and form.disable_inline_fields():
form.fields['DELETE'].disabled = True

Choose a reason for hiding this comment

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

With the already implemented code in DynamicallyDisabledAttachmentInlineFormSet this part could be removed.

Suggested change
# customize the DynamicallyDisabledAttachmentInlineFormSet to determine which inline attachments' DELETE checkbox
# should be readonly
MyCustomDynamicallyDisabledAttachmentInlineFormSet(DynamicallyDisabledAttachmentInlineFormSet):
super().add_fields(form, index)
"""
DELETE checkbox for Attachments that return True for
MyCustomDynamicallyDisabledAttachmentInlineForm's disable_inline_fields method should be disabled
"""
if hasattr(form, 'disable_inline_fields') and form.disable_inline_fields():
form.fields['DELETE'].disabled = True


class MyCustomDynamicallyDisabledAttachmentInlineAdmin(AttachmentInlineAdmin):
form = MyCustomDynamicallyDisabledAttachmentInlineForm
formset = MyCustomDynamicallyDisabledAttachmentInlineFormSet

Choose a reason for hiding this comment

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

Suggested change
formset = MyCustomDynamicallyDisabledAttachmentInlineFormSet
formset = DynamicallyDisabledAttachmentInlineFormSet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants