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

Adds management command to find possible username conflicts #629

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

jkachel
Copy link
Contributor

@jkachel jkachel commented Jun 16, 2022

Pre-Flight checklist

  • Testing
    • Code is tested
    • Changes have been manually tested

What are the relevant tickets?

#259

What's this PR do?

Adds a management command to identify usernames that would conflict after normalization. This is step one of the process for resolving #259.

The management command is find_username_conflicts and running it will load all user IDs and usernames from the database and run through them to see if a normalized username would be generated more than once. If the normalized username would be generated more than once, it outputs that normalized username with the IDs of the users that would cause a conflict. These can then be manually edited (in Django Admin or in the database directly).

The command does not change usernames at all and the standard caveats apply to modifying usernames manually - they will need to be modified on the Open edX side as well.

This command will need to be run and the affected accounts adjusted before #627 can be merged, as it adds a uniqued normalized_username field and automatically normalizes existing usernames.

How should this be manually tested?

Create two users: their usernames should share spelling but one should contain accented characters where the other doesn't. (For example, taylor and tâylör.) Then, run the find_username_conflicts command. It should identify those two accounts specifically as accounts that may conflict after normalization.

@odlbot odlbot temporarily deployed to mitxonline-ci-pr-629 June 16, 2022 16:48 Inactive
@jkachel jkachel mentioned this pull request Jun 16, 2022
5 tasks
@briangrossman briangrossman changed the title Adds management command to find possible username conflics Adds management command to find possible username conflicts Jun 17, 2022
@briangrossman
Copy link

@jkachel

Is it correct that the process will be:

  1. While find_username_conflicts returns conflicts:
    • Manually update MITx Online usernames
  2. Merge Adds username normalization #627
    ?

Note that the steps to update an MITx Online username are:

  • Update the username for the user record in the MITx Online admin
  • Update the username for the user record in the MITx Online Open edX admin
  • If the learner has a Micromasters account:
    • Update the username (UID) for the user's social auth in the Micromasters admin (the mitxonline one, not the edX one)

@jkachel
Copy link
Contributor Author

jkachel commented Jun 17, 2022

@briangrossman yes, that'll be exactly the process I had in mind and should prevent #627 from failing to deploy successfully. (One of the migrations in there will automatically normalize the incoming usernames and it'll fail if it hits a conflict.)

@arslanashraf7 arslanashraf7 self-assigned this Jun 21, 2022
@arslanashraf7
Copy link
Contributor

arslanashraf7 commented Jun 21, 2022

@jkachel Can this PR be reviewed independently or it should be reviewed after #627?

@jkachel
Copy link
Contributor Author

jkachel commented Jun 21, 2022

@arslanashraf7 this actually needs to be reviewed/merged before #627

Copy link
Contributor

@arslanashraf7 arslanashraf7 left a comment

Choose a reason for hiding this comment

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

I tested it with the following cases:

  • Create normal users (No results)
  • Create a user with a normal username and then a user with some accented character in the same username (Outputs usernames & ids)
  • Create a user with accented characters based username & then a new user with a normal username without accented chars (Prints ids of the conflicts)
  • Above step with multiple users with the same username but different accented chars (Prints multiple IDs of those users)

For more readability, we could print email ids too for the conflicted users because even the usernames would be the same considering accents the email ids might be different. But I'm okay with this since this looks more like a one-timer.

Looks good 👍

@jkachel jkachel force-pushed the jkachel/259-part-1-accented-username-command branch from 41bbb10 to fedef4b Compare June 22, 2022 16:17
@jkachel jkachel merged commit edc0dbd into main Jun 22, 2022
@jkachel jkachel deleted the jkachel/259-part-1-accented-username-command branch June 22, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants