Enhance Mark Item Discard/Weed Functionality

Bug #1779467 reported by Jason Stephenson
24
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Unassigned

Bug Description

When either weeding or finding individual items that should be removed
from the collection, circulation staff need an easy and efficient way
to mark items for discard/weed so that they can be sent to cataloging
staff for further resolution. Currently, only staff with cataloging
permissions can change an item status to discard/weed. As a result,
current workflow for circulation staff is to either mark items as
missing in order to place it in a status where it is invisible in the
public facing OPAC and not holdable, and/or to put items in a copy
bucket for later processing by cataloging staff. This enhancement
will provide the ability for circulation staff to directly mark an
item discard/weed similar to the process to mark an item missing or
damaged.

The following list outlines the changes proposed to make the above
possible. These changes include a few alterations to current behavior
when marking items as Missing or Damaged in order to make these
actions safer and to enhance consistency.

 1. Add a Discard/Weed option to all Action and right-click menus
    where Mark Item Missing and Mark Item damaged appear. This
    command will allow those staff with the MARK_ITEM_DISCARD
    permission to mark an item for Discard/Weed, i.e. change the
    item's status to that of Discard/Weed.

    The Mark Item options presently occur in menus on both the list
    and detail views of Item Status, the Holdings View, Check in and
    Renew Items, Record Holds, the Patron Holds list, the Pull List
    for Holds Requests, and Holds Shelf.

 2. A staff member will be permitted to mark an item's status as
    Discard/Weed via this new menu option if they have either the
    MARK_ITEM_DISCARD or UPDATE_COPY permission at the item's owning
    library. This behavior is consistent with the current code for
    changing an item's status via Mark Item Missing or Mark Item
    Damaged. It is also consistent with the current back end code for
    marking items in general.

 3. If the item to be marked Discard/Weed is checked out to a patron,
    a dialog will be presented to staff to allow them to cancel or to
    continue with the operation. If staff choose to continue with the
    operation, the item will be checked in before being marked
    Discard/Weed.

    Evergreen does not presently show a warning dialog to staff when
    marking a checked out item as Missing or Damaged. Items marked as
    Missing are not checked in before having their status changed, but
    items to be marked Damaged are checked in. It is not the
    intention of this development to change these aspects of marking
    items Missing or Damaged.

 4. If the item is in transit at the time it is marked Discard/Weed,
    the staff will again be presented with a dialog prompting them to
    continue or to cancel the action. If staff continue, the transit
    will be aborted and the item's status changed to Discard/Weed.

    As with checked out items, Evergreen does not currently prompt
    staff before marking an item as Missing or Damaged. Furthermore,
    the behavior is inconsistent with either of these statuses. If
    and item to be marked Missing is in transit at the time, the item
    has its status changed to Missing and the transit remains in
    place. If staff attempt to mark such an item as damaged, the
    attempt to mark the item silently fails.

    Unlike with checked out items, this development does propose
    changing the behavior of making items in transit as Missing or
    Damaged to be consistent with that proposed for marking the item
    Discard/Weed. The reason being that the leftover transits can
    stick around in the system for quite a while, and if the item's
    status ever changes back to available (which they do with more
    frequency than one might wish), these transits can cause problems
    for staff.

 5. Should the item to be marked Discard/Weed be the only copy that
    can fill one or more holds, a confirmation dialog will prompt
    staff to continue or to cancel the operation. Should staff
    continue, the item will be marked Discard/Weed. The holds will be
    left for the hold targeter to deal with. This means that they
    will likely go untargeted for some time until they reach
    expiration. The holds should not be immediately canceled as a new
    item could be added to fill the holds, or this item's status could
    change in the future if staff decide that marking it Discard/Weed
    was an error.

 6. As marking an item Discard/Weed is one step away from deleting the
    item, staff will be prompted for an override when they attempt to
    mark an item that does not meet one of the previous criteria and
    is in a status that has the restrict_copy_delete field set to
    true. Marking the item's status will fail unless the staff person
    has the COPY_DELETE_WARNING.override permission.

 7. Items in other statuses, and that do not meet any of the criteria
    in the previous points, will be marked Discard/Weed without any
    additional prompts or conditions.

It is important to note that this proposal does not affect the ability
of staff to change an item's status via the Volume/Copy Editor or
other interfaces in Evergreen.

This development is sponsored by PINES.

Elaine Hardy (ehardy)
Changed in evergreen:
status: New → Confirmed
Changed in evergreen:
status: Confirmed → In Progress
Revision history for this message
Jason Stephenson (jstephenson) wrote :

While working on this today, I came up with some thoughts on the back
end code for marking items in the various statuses. The following
statements are all my opinion and the direction I am taking with the
code. If anyone disagrees, then please chime in.

1. Any item that is checked out, should be checked in before having
its status changed. Currently this only happens for marking an item
damaged.

2. All of the back end functions should accept and deal with the
handle_checkin argument. If the handle_checkin parameter is not
supplied, or is a false value as far as Perl is concerned, a new event
will be thrown: ITEM_TO_MARK_CHECKED_OUT.

3. If an item is in transit at the time its status is changed, then
the transit should be aborted before the status change. Currently,
this is not done for any status changes via the mark item back end.

4. A new handle_transit parameter will be added to the mark item
family. It will function in a similar manner to the handle_checkin
change mentioned in list item #2. The new event for this will be
ITEM_TO_MARK_IN_TRANSIT.

5. This is not a change in behavior, but a clarification: If the user
lacks any of the permissions to perform the mark item or any
associated actions, then an appropriate event will be returned. There
will be no chance to override this event.

6. The code for handling checkins and marking an item damaged actually
run before the user's permission to mark the item are checked. This
may not be a problem since everything is done in a transaction, but
this code will be moved below the check for the appropriate
permissions.

NOTE: This marks a departure from my original plan to not make as many
changes to the current behavior of marking items damaged or missing.
After starting on this, I decided that the users and the code would
benefit from more consistent behavior.

Revision history for this message
Terran McCanna (tmccanna) wrote :

+1 for consistent behavior

Revision history for this message
Mike Rylander (mrylander) wrote :

Hi Jason,

Regarding the points in comment #1:

1) I think this is probably correct /except/ for marking an item Lost. In that case, we're "blaming" the patron and it should stay on their items out list.

2) +1, that seems it will provide the flexibility to deal with future UIs where we might want to /not/ force a checkin -- and even provide a hook for a YAOUS to specify that behavior for existing UIs (as in, a set of "don't check in on marking BLAH" YAOUSen).

3) I disagree with canceling transits generally, not just for the feature you're working on. Since we have the barcode value available, then especially for marking discard/weed, but also for other statuses, I don't understand why we aren't simply changing the copy_status field on the transit. Then the transit will either naturally finish, and the copy will get the new status and trigger the routing event at checkin, or the barcode can be checked in if the copy is in hand. It's an extra step in the case of a copy that has disappeared while in transit, but it also avoids throwing out state information about the copy's location (on a truck). All that said, the deciding evidence would be whether it is more common for a copy to be lost in transit than to simply still be in transit. If lost-in-transit is more common, then I see the argument for canceling the transit. Otherwise, it just seems wrong -- the right thing will happen at the right time when the copy shows up.

4) Is this meant to allow editing of action.transit_copy.copy_status rather than canceling, or just to prevent the action via an event in the face of an outstanding transit? If the former, +1. ;)

5) Thanks

6) +1

Thanks!

Revision history for this message
Elaine Hardy (ehardy) wrote : Re: [Bug 1779467] Re: Enhance Mark Item Discard/Weed Functionality
Download full text (8.7 KiB)

3/4) Cancelling transits. Remember that this development is for circ staff
to place items in discard/weed to then be deleted from the database by
cataloging staff, whether lost in transit or still in transit on the shelf.
Items lost in transit would be found with a report and would be handled
differently from those found on the shelf in transit, to be weeded. Either
way, the copy is to be deleted by cataloging staff and would no longer be
in the database and checking in would do no good if it did make it's way
back. I think the important thing with this is that the status of transit
is removed from the item to be placed in discard/weed, and that transaction
closed, so that cataloging staff can delete the item, without having a
transaction that would otherwise not be completed, hanging around in the
database.

J. Elaine Hardy
PINES & Collaborative Projects Manager
Georgia Public Library Service/PINES
1800 Century Place, Ste. 580
Atlanta, GA 30045

404.548.4241 Cell
<email address hidden>
Helpdesk: http://help.georgialibraries.org

On Tue, Jul 17, 2018 at 10:42 AM, Mike Rylander <email address hidden> wrote:

> Hi Jason,
>
> Regarding the points in comment #1:
>
> 1) I think this is probably correct /except/ for marking an item Lost.
> In that case, we're "blaming" the patron and it should stay on their
> items out list.
>
> 2) +1, that seems it will provide the flexibility to deal with future
> UIs where we might want to /not/ force a checkin -- and even provide a
> hook for a YAOUS to specify that behavior for existing UIs (as in, a set
> of "don't check in on marking BLAH" YAOUSen).
>
> 3) I disagree with canceling transits generally, not just for the
> feature you're working on. Since we have the barcode value available,
> then especially for marking discard/weed, but also for other statuses, I
> don't understand why we aren't simply changing the copy_status field on
> the transit. Then the transit will either naturally finish, and the copy
> will get the new status and trigger the routing event at checkin, or the
> barcode can be checked in if the copy is in hand. It's an extra step in
> the case of a copy that has disappeared while in transit, but it also
> avoids throwing out state information about the copy's location (on a
> truck). All that said, the deciding evidence would be whether it is
> more common for a copy to be lost in transit than to simply still be in
> transit. If lost-in-transit is more common, then I see the argument for
> canceling the transit. Otherwise, it just seems wrong -- the right
> thing will happen at the right time when the copy shows up.
>
> 4) Is this meant to allow editing of action.transit_copy.copy_status
> rather than canceling, or just to prevent the action via an event in the
> face of an outstanding transit? If the former, +1. ;)
>
> 5) Thanks
>
> 6) +1
>
> Thanks!
>
> --
> You received this bug notification because you are subscribed to
> Evergreen.
> Matching subscriptions: <email address hidden>
> https://bugs.launchpad.net/bugs/1779467
>
> Title:
> Enhance Mark Item Discard/Weed Functionality
>
> Status in Evergreen:
> In Progress
>
> Bug description:
> When either weed...

Read more...

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Responding to Mike's observations in comment #3, except points 3 & 4 that Elaine responded to:

1. This development does not affect marking an item lost nor marking an item missing pieces. Specifically it will affect in some way the following statuses:

* Damaged
* Missing
* Discard/Weed
* Bindery
* On Order
* ILL
* Cataloging
* Reserves

The client currently only exposes Damaged and Missing to staff in any meaningful way. This development will add Discard/Weed to the client. The other status have their own back end calls that could be used in the future.

2. The implementation that I am working on is that the changing of the status will fail unless the circulation is checked in. If that is a problem, I can limit that only to the Discard/Weed and Damaged statuses.

Currently, Evergreen only checks in an open circulation in the event of marking an item Damaged. I think that it should check in any outstanding circulations before changing the copy status to any of the above, but I'm willing to be shown that I am wrong.

Changed in evergreen:
milestone: 3.2-beta → 3.next
Changed in evergreen:
status: In Progress → Triaged
assignee: Jason Stephenson (jstephenson) → nobody
status: Triaged → In Progress
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Branch has been gone through initial testing and I am told that it will be signed off, so removing myself, and adding the pullrequest tag.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
status: In Progress → Confirmed
tags: added: pullrequest
Revision history for this message
Elaine Hardy (ehardy) wrote :

Jason

I have not completed testing and don't have enough info yet to sign off on this. I ran into a problem yesterday where marking d/w failed silently. However, since my laptop was having issues with popups yesterday, I halted testing until I could do so on my desktop. I am resuming testing this morning and will let you know as soon as I can if I do find problems.

Elaine

Revision history for this message
Terran McCanna (tmccanna) wrote :

I was able to confirm the one Elaine saw yesterday:

- Place a copy-level hold on an item.
- Attempt to mark that item discard/weed.

It fails silently, when it should present an error / prompt.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Ok. I have removed the pullrequest tag. I think I know what the problem is, but I'll do some testing to verify it before pushing a potential fix. My suspicion is it has to do with the test for the last hold copy not working on copy level holds.

tags: removed: pullrequest
Revision history for this message
Elaine Hardy (ehardy) wrote :

Just finished testing and sent results to Kathy. Also posting here.

I've attached a document with my test results and one with the console error reports from the browsers.

With two exceptions that I found, the functionality works well and as expected. The exceptions are on the attached document and included here

1. In the Renew Items menu, mark Discard/Weed failed silently, with no dialog box to click through nor any error messages. I have the console error message for this in the test document attached.

2. For any item at different stages of being on hold (waiting for capture, on holds shelf, etc.) the item was not marked as discard/weed. Click through dialog boxes appeared but the hold status remained on the item, even when clicking OK/Continue. The doc with the console error messages are from this.

Revision history for this message
Elaine Hardy (ehardy) wrote :
Revision history for this message
Elaine Hardy (ehardy) wrote :
Revision history for this message
Elaine Hardy (ehardy) wrote :

Do let me know if you have any questions.

I was not able to locate an item in long overdue status or to force it into that status during the test. So was not able to include it.

Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have force-pushed a rebased branch to the same location with two additional commits:

One fixes a typo in the renew items app that I apparently fixed on my test virtual machine but neglected to put in the branch.

The second fixes two problems with checking for the last copy to fill a hold in the back end code (Circ.pm).

I have re-added the pullrequest tag.

If you want to retest this and have already checked it out, you can do the following with the branch checked out:

git fetch working
git reset --hard d48415a9

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
tags: added: pullrequest
Revision history for this message
Kathy Lussier (klussier) wrote :
Revision history for this message
Kathy Lussier (klussier) wrote :

Hi Jason,
I've been testing your branch today, and it looks really good! We saw only one minor issue where there is a typo in the Discard / Weed dialog. It says Dicard instead of Discard.

I was going to sign off on it with the typo corrected, but I'm having trouble running your perl live test. I get the following output:

ok 1 - Logged in
ok 2 - Found or registered workstation
ok 3 - Logged out
ok 4 - Logged in with workstation
ok 5 - 'Got available copy from BR1' isa 'Fieldmapper::asset::copy'
ok 6 - Mark available copy Discard/Weed
ok 7 - Copy has Discard/Weed status
ok 8 - 'Got available copy from BR3' isa 'Fieldmapper::asset::copy'
Can't use string ("1") as a HASH ref while "strict refs" in use at zz-lp1779467-mark-item-discard.t line 119.

Revision history for this message
Kathy Lussier (klussier) wrote :

Actually, ignore my last comment. I loaded it on another test server and had no trouble with the test. I'm guessing I must have changed something in the database on the first server to cause the problem. Signoff is forthcoming!

Revision history for this message
Kathy Lussier (klussier) wrote :

My signoff, with an additional commit to fix the typo, is available at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp1779467-mark-item-discard-signoff.

I would like to give an opportunity for somebody uninvolved in this project to review the branch before merging it.

tags: added: signedoff
Changed in evergreen:
milestone: 3.next → 3.3-beta1
Revision history for this message
Terran McCanna (tmccanna) wrote :

On a 3.2.2 server with this code installed:

The changes here have broken the Mark Item Damaged billing process - if we have "Charge item price when marked damaged" set to True, we get a "Marking of item CONC40000630 with status Damaged failed: DAMAGE_CHARGE" type error.

(If we do not prompt for billing, it works.)

Revision history for this message
Terran McCanna (tmccanna) wrote :

Update: If the "Charge processing fee for damaged items" is in place it also causes the DAMAGE_CHARGE error and the status change fails.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have removed the pullrequest tag pending investigation of the issue reported in Terran's comments (#19 and #20).

tags: removed: pullrequest
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have confirmed the behavior reported by Terran in comments 19 and 20, and I am working on a fix.

I have removed the signedoff tag, though I am basing my fix on Kathy's sign off branch. The bug fix will require a new sign off.

tags: removed: signedoff
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Well, that turned out to be relatively simple. I reverted the change that this branch made to the mark_damaged functionality in the web staff client files. This means that the original code is still used for marking an item damaged in the web staff client. The backend changes remain and still seem to work with the original staff client code.

Terran, please resume testing and let me know if you find any other issues. I only tested this enough to see that it works with applying the item price and processing fees when marking an item damaged.

The new branch is at working/user/dyrcona/lp1779467-mark-item-discard-bugfix

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1779467-mark-item-discard-bugfix

I have added the pullrequest tag back to this bug, because I believe it is again ready for tesing and inclusion.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
tags: added: pullrequest
Revision history for this message
Terran McCanna (tmccanna) wrote :

Thanks, Jason - we've re-tested everything, and the damaged function is working properly and all of the other mark discard functions work great.

I consent to a second sign-off after Kathy's with my name, Terran McCanna, and email address, <email address hidden>.

tags: added: signedoff
Changed in evergreen:
milestone: 3.3-beta1 → 3.3-rc
Changed in evergreen:
milestone: 3.3-rc → 3.next
Changed in evergreen:
status: Confirmed → Fix Committed
milestone: 3.next → 3.4-beta1
Galen Charlton (gmc)
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers