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

Default location not being set/updated upon check-in #13167

Closed
2 tasks done
CrypNZ opened this issue Jun 14, 2023 · 6 comments
Closed
2 tasks done

Default location not being set/updated upon check-in #13167

CrypNZ opened this issue Jun 14, 2023 · 6 comments
Assignees

Comments

@CrypNZ
Copy link

CrypNZ commented Jun 14, 2023

Debug mode

Describe the bug

When checking in an asset, you have the option to set the "location" of the asset. However, when you do this, the default location doesn't get set, only the location.

That means any checked in assets when you run the "php artisan snipeit:sync-asset-locations --output=all" command will have their location wiped, despite someone going in and updating the location while checking it in.

  1. It will be wiped if there is no default location to begin with
  2. It will be overwritten if another default location was set initially but now it's in a different location.

Disregarding the above command, I still feel like updating the location on the checkin page should change the default (rtd) location because the act of the checking in an asset puts it in a rtd state.

Instead an extra step needs to be taken to check in the asset and then edit the asset and change the default location.

This gets a bit more messy considering you can't then change the location unless you check out the asset again, or you edit the asset and simply save the page again/change default location & save.

Reproduction steps

Example 1:

While checked out, an asset (which is checked out to a user) has;
Location - New York
Default Location - New York

Check in the asset, on this screen change the location to Kansas.
Now...
Location - Kansas
Default Location - New York

Edit the asset, you have the option of changing New York from the Default Location but you can't change Kansas despite it showing up in the asset view.

Example 2:

While checked out, an asset has;
Location - New York
Default Location - N/A

Check in the asset, on this screen change the location to Kansas
Now...
Location - Kansas
Default Location - N/A

Edit the asset, you have the option of adding a Default Location but have no way of changing the Location despite it being Kansas.

Expected behavior

When checking in an asset and setting the location, the location you select (if any) should overwrite the default location as well.

Similar to the "edit mass assets" screen as shown below which gives you the option of doing both - although I think it makes sense to do this by default...

image

Screenshots

No response

Snipe-IT Version

6.10 & v6.1.1-pre build 10778

Operating System

Ubuntu 2204

Web Server

Apache

PHP Version

7.4

Operating System

No response

Browser

No response

Version

No response

Device

No response

Operating System

No response

Browser

No response

Version

No response

Error messages

No response

Additional context

No response

@CrypNZ
Copy link
Author

CrypNZ commented Jun 29, 2023

@inietov If I'm reading what you've commented in the PR correctly then it looks like your PR is exactly what I was after :) Looking forward to seeing it in action. Thank you!

@snipe
Copy link
Owner

snipe commented Jun 29, 2023

Disregarding the above command, I still feel like updating the location on the checkin page should change the default (rtd) location because the act of the checking in an asset puts it in a rtd state.

I'm not sure I agree here, or at least without specifically making it clear that this will also update the default location. If I check something in during, let's say, end of school year assembly, that's where I'm checking it in from, but that doesn't change that its default location is somewhere in the IT warehouse.

@CrypNZ
Copy link
Author

CrypNZ commented Jun 29, 2023

@snipe I suppose my follow-up question would be, what's the difference between location and default location while an asset is checked-in? I know fundamentally what the difference is but my thought process would be if I am checking in an asset and changing the location then I am specifying where it is when it's ready to deploy.

I'm happy for there to be a checkbox that lets you set both at the same time similar to my screenshot above in the edit multiple assets screen if that's what makes most sense to you - just with my current workflow it'd be awesome to be able to change the location/default location while checking in an asset as we will be updating the assets location upon check-in.

Thanks,

@inietov
Copy link
Collaborator

inietov commented Jul 6, 2023

I'm trying to understand completely this issue, sorry if I'm stating the obvious here, just trying to get this right for me:

  • An asset can have a default location (rtd_location) as I understand this could be for example, a warehouse where the assets are before we assign it to an user/asset/location.

  • It also can have a location, but this is usually not manipulated by the user it's the current location in which the asset is at the moment. The system infers the location if for example the asset is checked out to an user.

The check-in functionality let the user assign a current location where they left the asset when they returned it, and is explained in the check-in view:
image

So, I believe my solution is wrong and the best way to introduce this feature/workflow is exactly as in the bulk checkin and add an option to also change the default location at the same time, but that need to be an opt-in, since the current functionality is the expected behaviour. Gonna work on this while wait for @snipe's input, but feel free to correct me if my explanation and understanding of the issue is wrong.

@snipe
Copy link
Owner

snipe commented Jul 6, 2023

@CrypNZ @inietov's assessment is correct. The default location generally doesn't change, since it's where the thing is when it's not checked out. I'm not kicking this out of bed, I just worry how this might interrupt other people's workflows if they're not expecting this change.

@inietov
Copy link
Collaborator

inietov commented Jul 6, 2023

In the linked PR I added the radio buttons in the input to select if only change current location on checkin (selected by default), so it show same old behavior with no user interaction. Or to change both current and default location if needed. (edit to add a screenshot)

image

@snipe snipe closed this as completed in 44231fa Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants