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

Implement django-natural-keys as base for BaseModel #2900

Closed
Tracked by #1574
jathanism opened this issue Nov 22, 2022 · 3 comments · Fixed by #3423
Closed
Tracked by #1574

Implement django-natural-keys as base for BaseModel #2900

jathanism opened this issue Nov 22, 2022 · 3 comments · Fixed by #3423
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

@jathanism
Copy link
Contributor

jathanism commented Nov 22, 2022

Proposed Changes

Base models should inherit from django-natural-keys base NaturalKeyModel and use NaturalKeyModelManager so that…

  • natural_key() is automatically provided using the uniqueness constraints
    • Currently for each model, we must define natural_key() method
  • Foo.objects.get_by_natural_key() is also automatically provided, reducing boilerplate
    • Currently for each model, we must subclass RestrictedQuerySet in order to publish a get_by_natural_key() method
    • And then set objects = FooRestrictedQuerySet.as_manager() on the model
  • Each model will also receive a natural_key_slug that is immutable derived from the natural key fields for that object. This can be used in URL fragments in the "natural key or PK" pattern we have begun to adopt for filters.

Justification

We have decided to align around natural keys. We need less boilerplate, not more. In the common case, this should work automatically and result in objects having deterministic uniqueness/natural key constraints with minimal effort. For the uncommon/edge case, we will need to adapt and revise the pattern.

@jathanism jathanism added the type: housekeeping Changes to the application which do not directly impact the end user label Nov 22, 2022
@jathanism jathanism added this to the v2.0.0 milestone Nov 22, 2022
@bryanculver
Copy link
Member

#2692

@glennmatthews
Copy link
Contributor

Since django-natural-keys provides a minimal/naive natural_key_slug implementation (for one, it doesn't account for the possibility that any of the key components may contain a - character), we probably need to come up with our own design here. Since one of the main uses of natural-key-slugs may be in URL patterns, we need something that's URL-safe - therefore I'm proposing that we consider either percent-encoding or punycode as a starting point.

@bryanculver
Copy link
Member

Completed with #3423

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 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.

3 participants