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

Bug: Editing an asset resets current location to default location #4802

Open
2 tasks done
ckinsler opened this issue Jan 11, 2018 · 14 comments
Open
2 tasks done

Bug: Editing an asset resets current location to default location #4802

ckinsler opened this issue Jan 11, 2018 · 14 comments
Labels
✋ bug Confirmed bug 👩‍💻 ready for dev These issues are ready for someone to work on them - take your pick!

Comments

@ckinsler
Copy link

ckinsler commented Jan 11, 2018

Expected Behavior (or desired behavior if a feature request)

We just ran into an odd issue where we checked in an asset and changed it's location at the same time, moving it into a "Lab" location. This allows us to reimage the PC to get it ready for the next deployment before going into storage. This check in piece DID move the current location as expected, however a subsequent EDIT of the asset (to update an custom "image date" field) subsequently moved the asset back to its DEFAULT location instead of leaving it where it was.

This ONLY seems to happen on assets that ARE NOT checked out to someone (if they are deployed, it seems to leave the locations alone)

So Several points here:

  • Ideally when editing an asset, the currently assigned location should stay the same, and not be changed back to its default location (just like it does when an asset is in a deployed state)

  • If this is done: For non checked-out assets, we probably also need to expose the (current) Location field in the edit asset screen as well, since there will be no way to move the asset BACK somewhere without a subsequent check-out\check-in action (as only the default location field is editable)

.

For clarity, here's the business case for the asset flow:

  1. Asset is in an inventory location (likely it's default)
  2. Asset gets checked out to user (and takes on a current location of the user's profile)
  3. Asset is returned from user, and at time of check-in is moved to a "Lab" location
  4. Asset is refurbished\re-imaged, and a custom data field is edited on asset indicating the refresh data (this is the step that is currently overwriting the current location data)
  5. Asset location at some point needs to be updated back to Inventory location (or could be any location) for next deployment -- this wouldn't necessarily always be it's default location if it was moved to another Lab, inventory area, etc

Actual Behavior

See above

Please confirm you have done the following before posting your bug report:


Provide answers to these questions:

  • Is this a fresh install or an upgrade?
    Fresh

  • Version of Snipe-IT you're running
    v4.1.9 - build 3128 (master)

  • Version of PHP you're running
    PHP 7.0.22

  • Version of MySQL/MariaDB you're running
    mysql Ver 15.1 Distrib 10.1.30-MariaDB

  • What OS and web server you're running Snipe-IT on
    Ubuntu 16.04.3 x64, Apache2

  • What method you used to install Snipe-IT (install.sh, manual installation, docker, etc)
    install.sh

  • WITH DEBUG TURNED ON, if you're getting an error in your browser, include that error
    N/A

  • What specific Snipe-IT page you're on, and what specific element you're interacting with to trigger the error
    See Above

  • If a stacktrace is provided in the error, include that too.
    N/A

  • Any errors that appear in your browser's error console.
    N/A

  • Confirm whether the error is reproducible on the demo: https://snipeitapp.com/demo.
    Yes, all confirmed via demo URL's

  • Include any additional information you can find in app/storage/logs and your webserver's logs.
    N/A

  • Include what you've done so far in the installation, and if you got any error messages along the way.
    N/A

  • Indicate whether or not you've manually edited any data directly in the database
    N/A

@snipe
Copy link
Owner

snipe commented Jan 11, 2018

Thanks for the detailed bug report - we'll take a look.

@snipe snipe added the ✋ bug Confirmed bug label Jan 16, 2018
@snipe
Copy link
Owner

snipe commented Jan 17, 2018

I can see where/why this is happening, I'm just not exactly sure how to get around it.

The code itself is very simple:

if ($asset->assigned_to=='') {
            $asset->location_id = $request->input('rtd_location_id', null);
}

The purpose for that is so that if you change the asset's current default location during a normal edit, it updates the location_id to whatever the new default location is - which is generally a thing you'd want. It's an unusual use-case for someone to change the location to a non-default when it's not checked out, versus just updating the default RTD location.

So I'm trying to figure out how to solve for your use-case without donking things upon for other folks who don't override locations on checkin.

@ckinsler
Copy link
Author

As a stop gap, maybe only run that update IF the default location is actually edited? We had edited only a custom field when we did our update, which (since it wasn't checked out) changed the location of the asset

We could also internally change our process to just check out the asset back to a location (vs. just changing the location as part of the check-in), but that'd probably be confusing to folks since there is an option to change the location as part of the check-in

I remember seeing a prior feature request to allow you to check in and re-check out at the same time -- maybe that would be the better long term answer? So you'd actually be reassigning the asset properly vs just changing its current location ID?

@snipe
Copy link
Owner

snipe commented Jan 17, 2018

maybe only run that update IF the default location is actually edited

How would we know that though?

@ckinsler
Copy link
Author

Is there not a way to compare the before\after values on an update to see if they change?

Another thought -- What if you exposed the asset's current location (in addition to default) as an editable field on the asset screen? That way you could change it's physical location without necessarily checking it out (which may be a more frequently used use case, to still keep the asset available for check-out).

Right now if you DO change the location during a check-in event, there's currently no way to update it back to some place else anyway (sans a checkout or this edit event). So that would also fix that issue as well

Thoughts?

@snipe
Copy link
Owner

snipe commented Jan 17, 2018

Is there not a way to compare the before\after values on an update to see if they change?

There is, but it will look the same when submitted whether it was pre-populated (because of edit) or whether it was changed during the edit.

What if you exposed the asset's current location (in addition to default) as an editable field on the asset screen?

We could do that, but then we'd have to remove the location override in checkin/audit, since we'd just run into the same problem all over again.

@ckinsler
Copy link
Author

ckinsler commented Jan 18, 2018

We could do that, but then we'd have to remove the location override in checkin/audit, since we'd just run into the same problem all over again.

Not sure I understand here -- you can already change the current location away from default on a check-in action. If you made this change, that current location field would ultimately be visible and editable under the edit screen. (I think) the only thing you'd need to remove is the location overwrite on the asset edit save (the code where you're overwriting location if the assigned_to is blank). This code wouldn't be needed anymore, as the user could change the current location AND default location from the same asset edit screen at the same time.

It's been a long day, so I apologize if maybe I'm missing something?

@snipe snipe added the 👩‍💻 ready for dev These issues are ready for someone to work on them - take your pick! label Jan 20, 2018
@stale
Copy link

stale bot commented May 5, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions!

@stale stale bot added the stale label May 5, 2018
@stale stale bot closed this as completed May 12, 2018
@JonathonReinhart
Copy link
Contributor

Here's another actual, acknowledged issue, closed by stale bot. Please re-open.

@eitler-terra
Copy link

We have the same Problem. Please re-open.

@athompsoncmc
Copy link

Same issue. We "assign" devices to classrooms by just bulk auditing the current location because the bulk checkout doesn't work with barcode scanners. If the device is edited then it's removed from the location, then we don't see it as assigned to the location.

@snipe snipe reopened this Jul 15, 2021
@stale
Copy link

stale bot commented Jul 15, 2021

Okay, it looks like this issue or feature request might still be important. We'll re-open it for now. Thank you for letting us know!

1 similar comment
@stale
Copy link

stale bot commented Jul 15, 2021

Okay, it looks like this issue or feature request might still be important. We'll re-open it for now. Thank you for letting us know!

@stale stale bot removed the stale label Jul 15, 2021
@NMX-Drew
Copy link

NMX-Drew commented May 16, 2023

I'm having this issue with Version v5.3.0 - build 6470 (master), has any work been done on it? I'd like to add a +1 to this issue if not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✋ bug Confirmed bug 👩‍💻 ready for dev These issues are ready for someone to work on them - take your pick!
Projects
None yet
Development

No branches or pull requests

6 participants