-
Notifications
You must be signed in to change notification settings - Fork 0
create CLI command #5
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
base: dev/8.1.x
Are you sure you want to change the base?
Changes from all commits
2edc98e
85350dd
66f6568
fb6454f
35d2c5a
a946b5a
f61def5
76e091e
2de33ee
cdb9308
f7defd7
aa3c056
c47dccd
d56d60f
dafb8c9
e9e464f
9da0153
dd9a168
2b8cc7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,100 @@ | ||||||||||||||||||||||||||
| from django.contrib.auth import get_user_model | ||||||||||||||||||||||||||
| from django.core.management.base import BaseCommand, CommandError | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from arches.app.utils.bulkupload import ( | ||||||||||||||||||||||||||
| user_has_provisional_edits, | ||||||||||||||||||||||||||
| approve_all_provisional_edits_for_user, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| class Command(BaseCommand): | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| Approves all provisional edits for specified users. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| This command can process users by either user IDs or usernames, with comprehensive | ||||||||||||||||||||||||||
| validation and graceful handling of non-existent users. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Arguments: | ||||||||||||||||||||||||||
| --user_ids: One or more user IDs to approve edits for (separate by space) | ||||||||||||||||||||||||||
| --user_names: One or more usernames to approve edits for (separate by space) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Examples: | ||||||||||||||||||||||||||
| python manage.py bulk_approve --user_ids 1 2 3 | ||||||||||||||||||||||||||
| python manage.py bulk_approve --user_names john_doe jane_smith admin | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def add_arguments(self, parser): | ||||||||||||||||||||||||||
| group = parser.add_mutually_exclusive_group(required=True) | ||||||||||||||||||||||||||
| group.add_argument( | ||||||||||||||||||||||||||
| "-u", | ||||||||||||||||||||||||||
| "--user_ids", | ||||||||||||||||||||||||||
| type=int, | ||||||||||||||||||||||||||
| nargs="+", | ||||||||||||||||||||||||||
| metavar="USER_ID", | ||||||||||||||||||||||||||
| default=[], | ||||||||||||||||||||||||||
| help="One or more user IDs to approve edits for (separate by space)", | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| group.add_argument( | ||||||||||||||||||||||||||
| "-n", | ||||||||||||||||||||||||||
| "--user_names", | ||||||||||||||||||||||||||
| type=str, | ||||||||||||||||||||||||||
| nargs="+", | ||||||||||||||||||||||||||
| metavar="USER_NAME", | ||||||||||||||||||||||||||
| default=[], | ||||||||||||||||||||||||||
| help="One or more user names to approve edits for (separate by space)", | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def handle(self, *args, **options): | ||||||||||||||||||||||||||
| user_ids = options.get("user_ids") | ||||||||||||||||||||||||||
| user_names = options.get("user_names") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
razekmh marked this conversation as resolved.
|
||||||||||||||||||||||||||
| User = get_user_model() | ||||||||||||||||||||||||||
| if user_ids: | ||||||||||||||||||||||||||
| existing_users = User.objects.filter(pk__in=user_ids).values_list( | ||||||||||||||||||||||||||
| "id", flat=True | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| if not existing_users: | ||||||||||||||||||||||||||
| raise CommandError(f"User(s) with ID(s) {user_ids} do(es) not exist.") | ||||||||||||||||||||||||||
| missing_user_ids = set(user_ids) - set(existing_users) | ||||||||||||||||||||||||||
| if missing_user_ids: | ||||||||||||||||||||||||||
| self.stdout.write( | ||||||||||||||||||||||||||
| self.style.WARNING( | ||||||||||||||||||||||||||
| f"User(s) with ID(s) {missing_user_ids} do(es) not exist and will be skipped." | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| user_ids = list(existing_users) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if user_names: | ||||||||||||||||||||||||||
| existing_user_names_and_ids = User.objects.filter( | ||||||||||||||||||||||||||
| username__in=user_names | ||||||||||||||||||||||||||
| ).values("id", "username") | ||||||||||||||||||||||||||
| if not existing_user_names_and_ids: | ||||||||||||||||||||||||||
| raise CommandError( | ||||||||||||||||||||||||||
| f"User(s) with name(s) {user_names} do(es) not exist." | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| found_usernames = {user["username"] for user in existing_user_names_and_ids} | ||||||||||||||||||||||||||
| missing_usernames = set(user_names) - found_usernames | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if missing_usernames: | ||||||||||||||||||||||||||
| self.stdout.write( | ||||||||||||||||||||||||||
| self.style.WARNING( | ||||||||||||||||||||||||||
| f"User(s) with name(s) {missing_usernames} do(es) not exist and will be skipped." | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| user_ids = [user["id"] for user in existing_user_names_and_ids] | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| for user_id in user_ids: | ||||||||||||||||||||||||||
|
razekmh marked this conversation as resolved.
|
||||||||||||||||||||||||||
| if not user_has_provisional_edits(user_id): | ||||||||||||||||||||||||||
| self.stdout.write( | ||||||||||||||||||||||||||
| self.style.WARNING( | ||||||||||||||||||||||||||
| f"No provisional edits found for user ID {user_id} or username {User.objects.filter(pk=user_id).first().username}" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| f"No provisional edits found for user ID {user_id} or username {User.objects.filter(pk=user_id).first().username}" | |
| # Fetch all usernames for the user_ids in a single query | |
| user_id_to_username = dict(User.objects.filter(pk__in=user_ids).values_list("id", "username")) | |
| for user_id in user_ids: | |
| username = user_id_to_username.get(user_id, "<unknown>") | |
| if not user_has_provisional_edits(user_id): | |
| self.stdout.write( | |
| self.style.WARNING( | |
| f"No provisional edits found for user ID {user_id} or username {username}" |
Copilot
AI
Aug 13, 2025
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.
The .first().username call could raise an AttributeError if the user doesn't exist. Although user_ids are validated earlier, this could still fail if a user is deleted between validation and processing.
| f"No provisional edits found for user ID {user_id} or username {User.objects.filter(pk=user_id).first().username}" | |
| f"No provisional edits found for user ID {user_id} or username {User.objects.filter(pk=user_id).first().username if User.objects.filter(pk=user_id).first() else '<deleted user>'}" |
Copilot
AI
Aug 6, 2025
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.
The command should handle potential exceptions from approve_all_provisional_edits_for_user() to prevent the command from crashing and provide meaningful error messages to users.
| approve_all_provisional_edits_for_user(user_id) | |
| try: | |
| approve_all_provisional_edits_for_user(user_id) | |
| except Exception as e: | |
| self.stdout.write( | |
| self.style.ERROR( | |
| f"Failed to approve provisional edits for user ID {user_id}: {e}" | |
| ) | |
| ) | |
| continue |
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 agree with the idea but I do not think Arches has a standard way to show the errors.
Copilot
AI
Aug 13, 2025
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.
This line performs another unnecessary database query inside a loop. The same user information should be cached to avoid repeated database hits.
| f"All provisional edits for user ID {user_id} or username {User.objects.filter(pk=user_id).first().username} have been approved." | |
| f"All provisional edits for user ID {user_id} or username {username} have been approved." |
Copilot
AI
Aug 13, 2025
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.
The .first().username call could raise an AttributeError if the user doesn't exist. Although user_ids are validated earlier, this could still fail if a user is deleted between validation and processing.
Uh oh!
There was an error while loading. Please reload this page.