-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fixes #20731 add data file data source to config template bulk import #20778
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
base: main
Are you sure you want to change the base?
Fixes #20731 add data file data source to config template bulk import #20778
Conversation
…urce-to-config_template-bulk-import
…urce-to-config_template-bulk-import
|
@ifoughal could you address the linter errors please? |
…-import' of https://github.com/ifoughal/netbox into 20731-add-data_file-data_source-to-config_template-bulk-import
|
@jeremystretch I have fixed the linter issues. |
jeremystretch
left a comment
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.
The functionality is there, but please avoid making changes that deviate from the established style or that modify components out of scope for the PR.
| from django.contrib.postgres.forms import SimpleArrayField | ||
| from django.core.exceptions import ObjectDoesNotExist | ||
| from django.utils.translation import gettext_lazy as _ | ||
|
|
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.
Revert
| from utilities.forms import CSVModelForm | ||
| from utilities.forms.fields import ( | ||
| CSVChoiceField, CSVContentTypeField, CSVModelChoiceField, CSVModelMultipleChoiceField, CSVMultipleChoiceField, | ||
| CSVMultipleContentTypeField, SlugField, |
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.
Revert
| CSVMultipleContentTypeField, SlugField, | ||
| CSVMultipleContentTypeField, SlugField | ||
| ) | ||
| from core.models import DataSource, DataFile |
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.
Combine with line 9 above
| model = ExportTemplate | ||
| fields = ( | ||
| 'name', 'object_types', 'description', 'environment_params', 'mime_type', 'file_name', 'file_extension', | ||
| 'name', 'object_types', 'description', 'environment_params', |
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.
Revert (this class isn't even in scope for the FR)
| CSVChoiceField, CSVContentTypeField, CSVModelChoiceField, CSVModelMultipleChoiceField, CSVMultipleChoiceField, | ||
| CSVMultipleContentTypeField, SlugField, | ||
| CSVMultipleContentTypeField, SlugField | ||
| ) |
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.
Revert
| queryset=DataFile.objects.all(), | ||
| required=False, | ||
| to_field_name='path', | ||
| help_text=_('DataFile containing the template code') |
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.
help_text should be human-friendly
| help_text=_('DataFile containing the template code') | |
| help_text=_('Data file containing the template code') |
| auto_sync_enabled = forms.BooleanField( | ||
| required=False, | ||
| label=_('auto sync enabled'), | ||
| help_text=_("Enable automatic synchronization of data when the data file is updated") |
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.
| help_text=_("Enable automatic synchronization of data when the data file is updated") | |
| help_text=_("Enable automatic synchronization of template content when the data file is updated") |
| queryset=DataSource.objects.all(), | ||
| required=False, | ||
| to_field_name='name', | ||
| help_text=_('data_source of the config template') |
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.
| help_text=_('data_source of the config template') | |
| help_text=_('Data source which provides the data file') |
| def clean_template_code(self): | ||
| # Make sure template_code is None when it's not included in the uploaded data | ||
| if not self.data.get('template_code') and not self.data.get('data_file'): | ||
| raise forms.ValidationError(_("Must specify either local content or a data file")) | ||
|
|
||
| return self.cleaned_data['template_code'] |
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.
This logic should be run under the form's clean() method as it involves more than one field.
| def clean_auto_sync_enabled(self): | ||
| # Make sure is_primary is None when it's not included in the uploaded data | ||
| if not self.data.get('auto_sync_enabled'): | ||
| self.cleaned_data['auto_sync_enabled'] = False | ||
| return self.cleaned_data['auto_sync_enabled'] |
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.
This does not appear to be needed, as auto_sync_enabled is a boolean field.
Fixes: #20731
During bulk import of config templates, it is not possible to set a data-source with data-file for the config-templates to be imported.
The change adds 3 additional fields to template_code as optional fields: