-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fixes #17443: Adds ExportTemplate.file_name field #18911
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
Conversation
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.
Thanks @jnovinger! Just had some very minor cleanup suggestions.
@@ -155,6 +155,10 @@ class ExportTemplateBulkEditForm(BulkEditForm): | |||
max_length=50, | |||
required=False | |||
) | |||
file_name = forms.CharField( |
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.
We should add file_name
to nullable_fields
below as well, to enable the "set null" option in the bulk edit form.
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.
Any guidance on when a field should or should not be included in nullable_fields
?
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.
Ideally any optional, non-M2M field on the model should be nullable, I think. There are probably some exceptions to that rule but none come to mind offhand.
@jeremystretch , rather than adding another schema migration, I edited the existing migration to reflect the new max length for |
Ah, looks like there are some conflicting migrations from a recent merge to |
- Adds `file_name` to `ExportTemplateBulkEditForm.nullable_fields` - Shortens max length of `ExportTemplate.file_name` to 200 chars - Adds tests for `ExportTemplateFilterSet.file_extension`
b78824e
to
76bf80a
Compare
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.
Thanks @jnovinger!
Fixes: #17443
ExportTemplate.file_name
fieldExportTemplate.render_to_response()
to usefile_name
if populatedfilename_from_model
, to standardize generating export filenames for models