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

Accessory checkin via API reported wrong target user #13463

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

uberbrady
Copy link
Collaborator

Fixes [fd-34518]

We were reporting the user_id of the accessories_users table, when we wanted to report the assigned_to instead. user_id is the identity of the Admin doing the thing.

I need to test this before I can sign off on it, so I'm leaving it at draft - but I'll run a couple of API tests against my instance and see if this fix does fix the bug. Then I'll take it off of Draft.

@what-the-diff
Copy link

what-the-diff bot commented Aug 15, 2023

PR Summary

  • Modification in Accessory Controller
    The way we keep track of actions related to accessories has been changed. Instead of using the user_id, we shifted to using assigned_to property of the accessory_user. This will provide a more accurate tracking of actions.

  • Update in Log Checkin Method
    The logCheckin method, responsible for logging when items are checked in, is now using the above modified variable. This enhances the logging accuracy and data reliability.

  • Improved Record Deletion
    We introduced a way to delete a specific record from accessories_users table. Now, when needed, we're able to remove the record where the id equals the accessory_user's id. This further increases the flexibility and control over accessory user-related data.

@uberbrady
Copy link
Collaborator Author

Okay, I've tested this now - without my change, I can confirm that the 'target' is set to the admin user (not the actual user that it was checked out to. With my change, it is correctly the actual targeted user.

So this is now good to go.

@uberbrady uberbrady marked this pull request as ready for review August 15, 2023 17:39
@uberbrady uberbrady requested a review from snipe as a code owner August 15, 2023 17:39
@snipe snipe merged commit 2ddf5c9 into snipe:master Aug 15, 2023
@snipe
Copy link
Owner

snipe commented Aug 15, 2023

You targeted this to master :( I didn't notice before I merged it

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.

2 participants