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

Fixed accessories and consumables not being logged correctly during bulk check-in #15221

Merged
merged 33 commits into from
Aug 8, 2024

Conversation

marcusmoore
Copy link
Collaborator

@marcusmoore marcusmoore commented Aug 6, 2024

Description

This PR fixes an issue where accessories and consumables being bulk checked in were not being logged correctly. See this comment for more details.

In the process the PR adds tests around the destroy method in BulkUsersController.

Fixes #15109

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Copy link

what-the-diff bot commented Aug 6, 2024

PR Summary

  • Enhanced User Accessory and Consumable Logging
    The BulkUsersController now includes the method functionality to record check-in actions for Users' accessories and consumables. This will provide more detailed and accurate records for these actions in our system.

  • Improved Testing with User Deletion Tests
    A new test file, BulkDeleteUsersTest, has been added to our project. This file is focused on testing the bulk deletion of users, bolstering the project's overall test coverage and ensuring code quality and stability.

Comment on lines +288 to +314
private function logAccessoriesCheckin(Collection $accessoryUserRows): void
{
foreach ($accessoryUserRows as $accessoryUserRow) {
$logAction = new Actionlog();
$logAction->item_id = $accessoryUserRow->accessory_id;
$logAction->item_type = Accessory::class;
$logAction->target_id = $accessoryUserRow->assigned_to;
$logAction->target_type = User::class;
$logAction->user_id = Auth::id();
$logAction->note = 'Bulk checkin items';
$logAction->logaction('checkin from');
}
}

private function logConsumablesCheckin(Collection $consumableUserRows): void
{
foreach ($consumableUserRows as $consumableUserRow) {
$logAction = new Actionlog();
$logAction->item_id = $consumableUserRow->consumable_id;
$logAction->item_type = Consumable::class;
$logAction->target_id = $consumableUserRow->assigned_to;
$logAction->target_type = User::class;
$logAction->user_id = Auth::id();
$logAction->note = 'Bulk checkin items';
$logAction->logaction('checkin from');
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not loving the duplication but there is enough difference that using explicit method names instead of passing a handful of parameters is cleaner I think

@marcusmoore marcusmoore marked this pull request as ready for review August 7, 2024 20:35
@marcusmoore marcusmoore requested a review from snipe as a code owner August 7, 2024 20:35
@snipe snipe merged commit f76fbe7 into snipe:develop Aug 8, 2024
9 checks passed
@marcusmoore marcusmoore deleted the fixes/bulk-checkin-logging branch August 8, 2024 17:16
@marcusmoore marcusmoore mentioned this pull request Aug 8, 2024
2 tasks
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.

None yet

2 participants