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

Migrating away from postgres CI fields. #513

Merged
merged 1 commit into from
May 21, 2024

Conversation

terjekv
Copy link
Collaborator

@terjekv terjekv commented Jun 30, 2023

Caveats:

  • Assumes fields in LCI fields are all already in lowercase. No efforts are currently made to manipulate these fields. Enforcing lowercase during the migration is doable if required, but the existing implementation of these fields use get_db_prep_save to guarantee only lowercase data is written.
  • Email fields are migrated to models.EmailField. Case insensitivity is not preserved.
  • Needs (much) more testing.

Implementation details:

  • Creates two LowerCase-fields to replace the LCI-fields, LowerCaseCharField and LowerCaseDNSField. The latter inherits from the former. These fields will, like the LCI fields before them, save their data in lowercase by using get_db_prep_save.
  • Uses a manager to hook into the calls to objects.get, objects.filter, and objects.exclude to ensure they all use lowercase searches into the LowerCase fields. This functionality relies on all fields that are to use lowercase inherit from mreg.fields.LowerCaseCharField.
  • Uses a mixin for views to overload get_object for relevant detail views.

Noise:

  • Overloaded get_queryset() usage with prefetching has to manually use lower() on the relevant input due to the manager not applying to querysets. For django-filter, this is solvable by mixing in a filter class that does the lowercasing were applicable, but url_filter does not support this easily.

Fixes #489

@terjekv terjekv self-assigned this Jun 30, 2023
@coveralls
Copy link
Collaborator

coveralls commented Jun 30, 2023

Coverage Status

coverage: 98.937% (-0.06%) from 98.998%
when pulling 313aaac on terjekv:migrate-away-from-ci-fields
into 0decba3 on unioslo:master.

@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from 3aab3b9 to 52ccd16 Compare August 18, 2023 08:23
@terjekv terjekv marked this pull request as ready for review August 18, 2023 08:32
@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from 52ccd16 to 06b518a Compare August 28, 2023 13:05
@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from 06b518a to 113999f Compare September 18, 2023 07:09
@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from 113999f to 4a8c06c Compare November 29, 2023 13:11
@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from 4a8c06c to 7eaa56e Compare December 5, 2023 08:34
@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from 7eaa56e to 33d0178 Compare April 22, 2024 15:49
@oyvindhagberg oyvindhagberg self-requested a review May 16, 2024 10:32
Copy link
Contributor

@oyvindhagberg oyvindhagberg left a comment

Choose a reason for hiding this comment

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

Recently switched to trunk based development so the master branch is now considered development.

*Caveats:*

  - Assumes fields in LCI fields are all already in lowercase. No efforts are currently made to manipulate these fields. This is fixable if required.
  - Email fields are migrated to models.EmailField. Case insensitivity is not preserved.
  - Needs (much) more testing.

Implementation details:

  - Creates two LowerCase-fields to replace the LCI-fields.
  - Uses a manager to hook into the calls to objects.get, objects.filter, and objects.exclude (the latter is currently unused).
  - Uses a mixin for views to overload get_object for relevant detail views.
  - Overloaded get_queryset() usage to check for exists are handled manually, should be cleaned up.
@terjekv terjekv force-pushed the migrate-away-from-ci-fields branch from f9b4426 to 313aaac Compare May 21, 2024 09:31
@oyvindhagberg oyvindhagberg merged commit 313aaac into unioslo:master May 21, 2024
22 checks passed
terjekv added a commit that referenced this pull request May 23, 2024
  - We have now moved on to collations in #513 so these hacks are no longer required.
terjekv added a commit that referenced this pull request May 23, 2024
- We have now moved on to collations in #513 so these hacks are no longer required.
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.

Django 5 deprecates CiCharField.
3 participants