Bulkimport #11779
Bulkimport #11779
Conversation
github-actions
bot
added
DEV: backend
Build Artifacts
|
kolibri/core/auth/models.py
Outdated
| @@ -639,7 +639,7 @@ def create_user(self, username, email=None, password=None, **extra_fields): | |||
| if extra_fields.get("facility") is None: | |||
| extra_fields["facility"] = Facility.get_default_facility() | |||
| if self.filter( | |||
| username__iexact=username, facility=extra_fields["facility"] | |||
| username=username, facility=extra_fields["facility"] | |||
There was a problem hiding this comment.
This should not be updated - the intention was for the bulk import flow to match the enforcement that happens here, not for this enforcement to be removed.
| # If UUID is not specified, check for a username clash | ||
| if values["uuid"] == "": | ||
| existing_user = FacilityUser.objects.filter( | ||
| username=user, facility=self.default_facility |
There was a problem hiding this comment.
We should do a case insensitive match here as well, to parallel the changes you made above.
|
@rtibbles I have changed what you said. |
There was a problem hiding this comment.
Excellent, this looks good. We'll have manual QA test this, but I think this should be mergeable soon.
We will hold off on merging it until the 0.16.0 release is out.
In the meantime, could you address the linting issue? For now we can ignore the complexity requirement by doing this:
def build_users_objects(self, users): # noqa C901
This is a directive comment to our linter to ignore this rule for this function for now.
|
@rtibbles , do you have an estimate for when this will be merged? |
|
After 0.16.0 is released! We're in the final stages of QA now, and made a release candidate yesterday, so it is close :) |
|
Hi @nick2432 and @rtibbles - I confirm that it's no longer possible to import a duplicate username (regardless of the letter case) if it's already extant in the facility. 2024-02-15_18-21-57.mp4 |
|
That's a good catch, Peter - I think we can file that as a follow up issue for now, so as not to block @nick2432 further here, but very happy for him to pick it up if he's interested! |
|
Thank you for the clarification, @rtibbles . I'll proceed with the current PR as suggested. I'll make a note of the additional issue with importing several duplicate usernames and will file it as a follow-up task. If there are any specific considerations or details I should be aware of while working on the current PR, please let me know. Appreciate your guidance! |
Summary
Description of the Change:
The proposed changes aim to enhance the user creation process in the FacilityUserManager class by addressing issues related to case-insensitive username matching and the verification of duplicate usernames during bulk user imports.
Case-Insensitive Username Matching (FacilityUserManager):
In the create_user function of the FacilityUserManager class, case sensitivity is introduced to the username matching process. This ensures that usernames are checked in a case-sensitive manner during user creation.
Link to the relevant code: [FacilityUserManager]
Bulk User Import Duplicate Username Check (bulkimportusers.py):
Previously, the bulk import code only checked for username clashes if a UUID was specified for the user. The proposed changes extend this check to cases where the UUID is not specified, providing a more comprehensive verification process.
…
References
Fixes #11081
…