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

Change custom field "name" to a proper "slug" field #824

Closed
glennmatthews opened this issue Aug 17, 2021 · 5 comments · Fixed by #3426
Closed

Change custom field "name" to a proper "slug" field #824

glennmatthews opened this issue Aug 17, 2021 · 5 comments · Fixed by #3426
Assignees
Labels
impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release type: housekeeping Changes to the application which do not directly impact the end user
Milestone

Comments

@glennmatthews
Copy link
Contributor

glennmatthews commented Aug 17, 2021

Proposed Changes

Follow-on to discussion originally in #464. Custom fields currently have both a label (which is meant to be human-readable) and a name (which is used internally as a dictionary key, as well as in the GraphQL schema).

Even though the name is used in various programmatic and API features, it is currently treated more like the human-readable name present on various other models than like a proper slug - in particular, it doesn't currently have any of the format enforcement applied to similarly-used slug fields in most other models.

The proposal here is to migrate name to slug in the next major (breaking) release of Nautobot, both for self-consistency and also for the prevention of bugs such as #464.

Note that some of the (non-breaking) groundwork for this change has already been laid in #735; implementing this change should include a thorough review of the various TODO comments added in that PR.

Justification

  • Provide a proper CustomField slug field that can be used safely/correctly in URL patterns, dictionary keys, GraphQL, REST API, etc.
  • Reduce the number of redundant "human-readable" fields (name/label/description) currently present on CustomField model.

TODO

Future follow-on: Should label on both custom and computed fields now become the "name" of the field, with a call-out that it is the human-readable version

@glennmatthews glennmatthews added status: future type: housekeeping Changes to the application which do not directly impact the end user impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release labels Aug 17, 2021
@smk4664
Copy link
Contributor

smk4664 commented Aug 20, 2021

I know there was some interest in standardizing Slug fields across all of Nautobot. I am curious why we would choose to depreciate the name field when that is the field that the Slug is taken from in most of the other models. Would it make more sense to depreciate the label instead?

I don't think all of this needs to wait till the next major release to introduce all of these changes. I believe we can safely accomplish most of the slug implementations ahead of time.

In addition to this, I was working on a Data migration the other day, and found the Django System Check Framework, which I would like to add to the chosen depreciated field so we can start alerting users ahead of time of the new field.

@glennmatthews
Copy link
Contributor Author

We could arguably rename label to name as part of this same migration, that might be a good thing to do. Ultimately we would want this to match other models in having a name and a slug; the issue is that right now the field called name is being used in places where a slug would be more appropriate.

@jathanism
Copy link
Contributor

jathanism commented Nov 15, 2022

Inventory of changes needing to be made to satisfy this based on 2.0 TODO comments in the code base, broken down by filename:

nautobot/core/graphql/generators.py
- # 2.0 TODO: #824 rename `name` to `slug`

nautobot/core/graphql/schema.py
- # 2.0 TODO: #824 replace field.name with field.slug
- # 2.0 TODO: #824 field.slug
- # 2.0 TODO: str_to_var_name is lossy, it may cause different fields to map to the same field_name

nautobot/core/views/generic.py
- # 2.0 TODO: #824 custom_field.slug
- # 2.0 TODO: #824 use custom_field_slug
- # 2.0 TODO: #824 this won't really be needed once obj.cf is indexed by slug rather than by name
- # 2.0 TODO: #824 when we use slug in obj.cf we can just do obj.cf[field_name[3:]]

nautobot/dcim/models/device_component_templates.py
- # 2.0 TODO: #824 use field.slug

nautobot/extras/api/customfields.py
- # 2.0 TODO: #824 use cf.slug as lookup key instead of cf.name
- # 2.0 TODO: #824 remove this translation

nautobot/extras/api/serializers.py
- # 2.0 TODO: #824 remove `name` entirely from the model; for now it's required.
- # 2.0 TODO: this is to fix up existing usage when caller specifies a name but not a label

nautobot/extras/filters.py
- # 2.0 TODO: #824 use cf.slug instead

nautobot/extras/forms/mixins.py
- # 2.0 TODO: #824 cf.name to cf.slug throughout
- # 2.0 TODO: #824 self.instance.cf.get(cf.slug)
- # 2.0 TODO: #824 will let us just do:

nautobot/extras/management/commands/fix_custom_fields.py
- # 2.0 TODO: #824 use cf.slug rather than cf.name
- # 2.0 TODO: #824 use custom_field.slug rather than custom_field.name

nautobot/extras/models/customfields.py
- # 2.0 TODO: #824 field.slug rather than field.name
- # 2.0 TODO: #824 replace cf.name with cf.slug
- # 2.0 TODO: #824 replace cf.name with cf.slug
- # 2.0 TODO: #824 remove `name` field as redundant, make `label` mandatory, populate `slug` from `label` field.
- # 2.0 TODO: #824 once self.name is no longer used as a dict key, can remove this constraint
- # 2.0 TODO: #824 use self.slug as key instead of self.name
- # 2.0 TODO: #824 self.field.slug instead of self.field.name
- # 2.0 TODO: #824 self.field.slug instead of self.field.name
- # 2.0 TODO: this is to fixup existing ORM usage when caller specifies a name but not a label;
- # 2.0 TODO: this is to handle the UI case where `name` is no longer a directly configured form.

nautobot/extras/signals.py
- # 2.0 TODO: #824 instance.slug rather than instance.name

nautobot/extras/tables.py
- # 2.0 TODO: #824 Remove name column
- # 2.0 TODO: #824 Remove name column

nautobot/extras/tasks.py
- # 2.0 TODO: #824 field.slug rather than field.name
- # 2.0 TODO: #824 field.slug rather than field.name
- # 2.0 TODO: #824 rename field_name to field_slug
- # 2.0 TODO: #824 field.slug rather than field.name

nautobot/extras/tests/example_jobs/test_site_with_custom_field.py
- # 2.0 TODO: #824 cf.slug rather than cf.name

nautobot/extras/tests/integration/test_customfields.py
- # 2.0 TODO: #824 replace custom_field.name with custom_field.slug

nautobot/extras/tests/test_api.py
- # 2.0 TODO: #824 remove name entirely

nautobot/extras/tests/test_customfields.py
- # 2.0 TODO: slug and label will become mandatory fields to specify.
- # 2.0 TODO: #824 remove name field
- # 2.0 TODO: #824 slug rather than name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 replace .name with .slug
- # 2.0 TODO: #824 cf.slug rather than cf.name
- # 2.0 TODO: #824 replace site2_cfvs[name] with site2_cfvs[slug]
- # 2.0 TODO: #824 replace .name with .slug

nautobot/extras/tests/test_views.py
- # 2.0 TODO: #824 removal of `name` field altogether

nautobot/extras/views.py
- # 2.0 TODO: #824 use obj.slug instead of obj.name

nautobot/utilities/tables.py
- # 2.0 TODO: #824 replace customfield.name with customfield.slug

nautobot/utilities/testing/views.py
- # 2.0 TODO: #824 custom_field.slug rather than custom_field.name

@bryanculver
Copy link
Member

Based on feedback in #3426 we're going to use key instead of slug.

@glennmatthews
Copy link
Contributor Author

Implemented for 2.0 by #3426.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release type: housekeeping Changes to the application which do not directly impact the end user
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants