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

Add model toggle to skip adding missing Dynamic Group filter fields #2168

Merged
merged 2 commits into from
Aug 10, 2022

Conversation

jathanism
Copy link
Contributor

Closes: #DNE

What's Changed

  • Introduced FooModel.dynamic_group_skip_missing_fields Boolean which defaults to False to allow toggling whether missing filter form fields found in the model's filterset are automatically added to the filter form. Defining this at the model and setting it to True will cause the missing form fields to be skipped on filter form generation.
  • Additionally, any missing model form fields that do get added will have their initial value stripped when the filter form is generated prior to the instance's filter data being passed into it.

TODO

  • Explanation of Change(s)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

- Introduced `FooModel.dynamic_group_skip_missing_fields` Boolean which defaults to `False` to allow toggling whether missing filter form fields found in the model's filterset are automatically added to the filter form. Defining this at the model and setting it to `True` will cause the missing form fields to be skipped on filter form generation.
- Additionally, any missing model form fields that do get added will have their `initial` value stripped when the form is generated prior to the instance data being passed into it.
missing_fields = set(modelform_fields).difference(filterform_fields)
else:
logger.debug("Will not add missing form fields due to model meta option.")
missing_fields = set()

# Try a few ways to see if a missing field can be added to the filter fields.
for missing_field in missing_fields:
Copy link
Contributor

Choose a reason for hiding this comment

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

Given missing_fields is a set, I wonder if we should be sorting it in some way before iterating over it, so as to ensure consistent ordering in the generated form?

Copy link
Contributor Author

@jathanism jathanism Aug 10, 2022

Choose a reason for hiding this comment

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

Yes. There was a lingering todo item to also augment/extend the form's existing field ordering and re-order the fields using Form.order_fields(field_order) but got stuck on how we decide to order the fields. Existing fields with missing fields at the bottom? Missing at the top? Alpha sorted?

nautobot/extras/models/groups.py Outdated Show resolved Hide resolved
nautobot/extras/tests/test_dynamicgroups.py Outdated Show resolved Hide resolved

try:
group.model.dynamic_group_skip_missing_fields = True
fields = group._map_filter_fields
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit weird that this is a property, given its name I would have expected it to be a method. But I can certainly see the benefit of allowing it to be cached.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's so that it can be cached as a @cached_property. The method get_filter_fields() calls this.

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
@jathanism jathanism merged commit 9e9d462 into next Aug 10, 2022
@jathanism jathanism deleted the jathanism-dg-missing-fields branch August 10, 2022 17:42
jathanism added a commit that referenced this pull request Aug 10, 2022
smk4664 added a commit to smk4664/nautobot that referenced this pull request Aug 10, 2022
* next: (24 commits)
  Added release-notes for nautobot#2168
  Add model toggle to skip adding missing Dynamic Group filter fields (nautobot#2168)
  Add release-note for nautobot#2165
  Fix up relationship-association API test issue (nautobot#2165)
  Fix missing RelationshipModelForm class, add @class_deprecated_in_favor_of decorator (nautobot#2151)
  Fix nautobot#2090: don't reset Tag content-types on a PATCH (nautobot#2164)
  Add release-note for nautobot#2106
  Notes api (nautobot#2120)
  Fix release note for nautobot#2150
  Only populate statuses on Integration TestCases. (nautobot#2160)
  fix ObjectChange missing user object when making changes through API (nautobot#2158)
  Fix release note for nautobot#2091
  Feature to Mark an Entire Job's Parameters as Sensitive (nautobot#2125)
  Update release date, version.
  Add release-note for nautobot#2095
  Documenting Redis TLS and adjusting healthcheck. (nautobot#2097)
  Extending filtering of date custom field type (nautobot#2126)
  Bug fixes and add missing API functionality for Dynamic Group Memberships (nautobot#2144)
  Add release-notes for nautobot#2073, nautobot#2080, nautobot#2143
  Add local option to runjob (nautobot#2073)
  ...

# Conflicts:
#	nautobot/docs/development/style-guide.md
#	nautobot/docs/release-notes/version-1.4.md
#	nautobot/extras/forms/base.py
#	nautobot/extras/forms/mixins.py
#	nautobot/extras/tests/test_forms.py
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

2 participants