-
Notifications
You must be signed in to change notification settings - Fork 27
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
Person identifier cleaning #2433
Conversation
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.
I haven't run this code locally, but I've had a quick read over the diff. Here's a few thoughts..
ynr/apps/people/management/commands/reformat_person_identifiers.py
Outdated
Show resolved
Hide resolved
ynr/apps/people/management/commands/reformat_person_identifiers.py
Outdated
Show resolved
Hide resolved
ynr/apps/people/management/commands/reformat_person_identifiers.py
Outdated
Show resolved
Hide resolved
Oh, and your build is ❌ failing 😆 |
- Audit all person identifiers and their validation - Check the domain and the username for valid entries
0a7329a
to
6327f12
Compare
Ok, I reckon that's all of those comments addressed? |
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.
Latest changes look pretty sensible. Ta
This PR combines and replaces #2400 and #2406 into one PR.
I combined them because both PRs contained the same management command with different code in it. I decided to re-write it to make it more generic.
I also did some cleaning up of the validation and cleaning code.
Tested on a staging copy of the database.