Copy Alert Persistence and Suppression Matrix

Bug #1676608 reported by Galen Charlton on 2017-03-27
32
This bug affects 7 people
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Galen Charlton

Bug Description

Today, Evergreen provides two types of alerts for copy data: on-demand alerts that are displayed when an event occurs, such as the check-in of an item that has been marked Missing or Lost; and one-per-copy notification alert messages that are supplied by staff. Both alerts are presented to the user at check-in and checkout, with some exceptions, and staff intervention is required to force or stop the attempted action.

A new matrix of Alert Types and Events will be created, which will subsume the functionality provided by the current Alert Message and on-demand event alerting functionality. Staff will be able to attach multiple Alert Messages to a copy, and to define during which staff actions, such as check-in, checkout, and renew, those Alert Messages will be displayed. Staff will be able to mark each Alert Message as either Temporary or Persistent. Temporary Alert Messages will offer the alerted staff the option to Acknowledge the alert, at which time the Alert Message in question will be disabled, but not deleted, from the copy.

This will leverage the event matrix described above to generate a record of on-demand Alert Messages that occur at specific events. In addition, the ability to suppress the display of on-demand event alerts on a per-org-unit basis. Suppression will not prevent the generation of an event, but will instead act as if staff had intervened to force the attempted action. On-demand, system-generated alerts will be marked as temporary and auto-close, simply recording the fact of the alert. For direct checkout of Long Overdue items, a single new Org Setting will decided whether the preceding suppression-forced checkin will forgive fines or retain them. We will create a similar setting for Lost items.

Events will react to varying levels of granularity. Less granular events will be displayed at the time of any sub-events, whereas more granular alerts will be displayed only when applicable. For instance, "Non-renewal Check-in at Owning Library" Alert Types will only display when an item is checked in at it’s Owning Lib. However, general "Check-in" event alerts will display for each checkin event for an item, whether owning-library local or not, and even for renewal checkins.

Each ILS administrator with appropriate permissions will have the ability to associate one or more statuses with each Alert Type which, when used as a Temporary Copy Alert, will be presented to the alerted staff for use as the Next Copy Status, overriding the next natural status the copy would take. Persistent Alerts will not be able to make use of the Next Status values, if any are defined for the Alert Type. If one status is supplied by the administrator, that status will be applied to the copy after the Alert is processed, instead of the natural Next Status. If more than one Next Status is supplied, staff will be presented with the list to select from, with the first value in the list being selected by default. Suppressed on-demand events will not make use of the Next Status functionality. Only check-in event Alert Types will have associated logic that makes use of the Next Status selected by staff. Future development may add Alert Types for other events, such as the various transit types, or for events in the hold lifecycle. In that case, logic specific to those events would be added to make use of the Next Status selected by staff.

Finally, each Alert Type can be configured so that it will only display when the triggering event occurs at the owning or circulation library, or either.

Existing Copy Alert Message values will be copied and applied to new, stock Alert Types that occurs at non-renewal check-in and checkout, maintaining existing functionality. The Copy Alert field on the copy will then be cleared.

Full technical specifications for this feature are here:

http://yeti.esilibrary.com/dev/public/techspecs/alert-matrix.html

Galen Charlton (gmc) wrote :

The branch collab/gmcharlt/lp1676608_copy_alerts contains an implementation that has been tested by the library sponsoring work on this feature. The branch is rebased against master as of today, but is need in complete.

tags: added: alerts copies
Galen Charlton (gmc) wrote :

The last clause in my last comment should read "but is in need of some cleanup".

tags: added: pullrequest
Kathy Lussier (klussier) wrote :

Mike - Would you be able to rebase this against current master again? I'm getting merge conflicts.

Kathy Lussier (klussier) on 2017-08-31
Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
Kathy Lussier (klussier) wrote :

Thank you Mike!

I've run into a few problems with this branch.

1. There is a problem with the Item Status screen. If I click the 'view' link from the copy on a bib record page or if I try to scan a barcode in the item status screen, I get no copy details in the item status screen. Also, if I click the 'edit' link from the copy on a bib record page, it retrieves the item status interface instead of the copy editor. I see the following in the Console in all three cases:

angular.min.js:119 TypeError: Cannot read property 'copy_alerts' of undefined
    at item.js:155
    at angular.min.js:132
    at m.$eval (angular.min.js:147)
    at m.$digest (angular.min.js:144)
    at angular.min.js:147
    at f (angular.min.js:45)
    at angular.min.js:48

2. When I edit a copy or add volumes/copies, the Copy Alert button and the Copy Tag button are disabled. I'm unfamiliar with the Copy Tag feature, but I'm assuming this is supposed to be active too. I am using a superuser login, so it's not a permissions issue.

With those two issues, I don't think there is a way to add an alert to a copy to do further testing on alerts applied to copies. I'll see if I can get some testing done to suppress certain alerts for certain events.

Kathy Lussier (klussier) on 2017-08-31
tags: added: needsreleasenote
Kathy Lussier (klussier) wrote :

If I try to create a Copy Alert Suppression event, nothing appears in the alert type dropdown menu. I have verified there are alert types available (the sample ones that came with the branch) in the database. They also display in the Copy Alert Types admin interface.

Mike Rylander (mrylander) wrote :

Kathy,

There were a /tonne/ of conflicts, some caused by the separate of the new egItem service. I'm certain some of my rebasing missed stuff, and it's obvious some wires got crossed as well with functions that bring up new interfaces.

I can try again tomorrow morning, and may pull in others to look as well. It may be easier to handle if it's all squashed first, though. I'll update when I have something to look at...

Thanks for testing. :(

Mike Rylander (mrylander) wrote :

s/separate/seperation/ ... no type gud today.

Galen Charlton (gmc) wrote :

Kathy and Mike: given the merge conflicts, the wide-ranging changes to circulation status and alerts managing introduced by this branch, and the lack of community feedback or attention, I suggest that we declare victory... for 3.1. In other words, I propose that we bump this back to target 3.next and plan on making a special effort (maybe a dedicated dev meeting?) in the late Fall to work on this and ensure that everybody is on the same page.

Kathy Lussier (klussier) wrote :

I understand your concerns, but, all the same, I'm not too happy about pushing this project back since it's been in the works for such a long time. If we need to hold off on it, I think we need a smaller branch that adds an alert field to the copy editor to store our current-day alerts. I think it was okay to go without one while the web client was still in a pilot phase, but now that we're calling it production ready, I think people will need to be able to add alerts to their copies.

Changed in evergreen:
assignee: Kathy Lussier (klussier) → nobody
Galen Charlton (gmc) wrote :

I understand your frustration; this has been a long project. We can commit to ensuring a branch is created to implement support for legacy copy alert messages to achieve XUL parity and ready to be merged for 3.0. Per the note I sent to open-ils-dev, such a branch would fall on the list of exceptions to today's feature freeze deadline.

Kathy Lussier (klussier) wrote :

So that we do not lose track of the copy alert field for 3.0, I have filed a new bug at https://bugs.launchpad.net/evergreen/+bug/1719967

Galen Charlton (gmc) wrote :

I've pushed a rebase branch that fixes the errors Kathy saw here:

collab/gmcharlt/lp1676608_copy_alerts_rebase_2017_11

Dan Wells (dbw2) wrote :

I checked this out a bit on Blake's test machine, and have just a couple quick reactions to share.

1) We have settings of "At Owning Library?" and "At Circulation Library?" with options of "Do not care" and "Yes". This seems awkward. One possible fix here would be to reword the settings as "Only at Owning Library?", etc. (adding the word "Only"), which would allow the answers to change to the more typical "Yes"/"No" or "True"/"False". We might even go further and have it read "Show Only at Owning Library?" or "Alert Only at Owning Library?" if we want to make it even more expressive. I can construct the simple patch needed for this if we agree on a direction.

2) I noticed the "event" name is listed in the alert. I can see how this might be useful, but it feels a bit "debuggy" rather than natural and useful in the average case. I also notice it did not display in the doc screenshots, so somebody made the conscious decision to add it, so I wonder what the expressed need was. If we don't want to remove it outright, I can think of a number of ways to de-emphasize it vs. the actual alert message, which seems useful to me.

Other than these small things, looks great. My mind is going over ways this might help us with reserve check-in/check-out. Thank you for making this happen.

Blake GH (bmagic) wrote :

From the testing that we have done, it looks like the functionality is all there. When the dropdown menus become longer it would be nice if the list was alphabetical. See screen shot.

Blake GH (bmagic) wrote :

More thoughts:

1. The "Manage" button should probably not be clickable, or removed from the UI when there are 0 copy alerts to manage.

2. The "Add" and "Manage" buttons appear on the opposite side of the label in the "Item Status" -> Detailed View (see attached screenshot)

All in all though, I am liking this feature more and more!

Dan Wells (dbw2) on 2018-01-19
Changed in evergreen:
milestone: 3.next → 3.1-beta
Galen Charlton (gmc) wrote :

The branch collab/gmcharlt/lp1676608_copy_alerts_rebase_2018_02 has the latest and greatest, consistently of a squash of the previous branch, release notes, and patches to address feedback given during the hackfest. In particular:

- I've tweaked the labeling of the At Circulation and At Ownining Library fields in the copy alert type editor
- Copy alert types are now sorted in modal drop-downs
- Copy alerts are now sorted by ID in the management modal
- I've tweaked the location of the copy alert buttons on the item status detail view
- The Manage button on the item status detail view is now enabled only when there are copy alerts to manage. Also, refreshes are handled better.
- I've fixed a bug in egPCRUD's handling of the apply action

I did not remove the event from the copy alerts acknowledgement dialog, as my notes indicated that that was specifically requested to be there. That said, I'm open to patches to tweak how it styled.

With this, I now ask for a final round of review and merging to master.

Mike Rylander (mrylander) wrote :

Just a note about the "Do not care" option, that sets the flag to NULL, not FALSE. I need to look at the code to provide the full context (and my desktop machine just fell over), but that's intentionally distinct from either TRUE or FALSE. More in a bit...

Kathy Lussier (klussier) wrote :

Can somebody show me an example of the event that displays in the copy alerts acknowledgement dialog? I'm sure I was probably the person who requested it, but I am having a difficult time finding any reference to it in my notes.

Mike Rylander (mrylander) wrote :

Background:

OK, so the idea with the "at" restrictions is that if either or both of the "At blah" flags is set, it will only allow the alert to get back to the client if the context org (say, the circulation's circ lib) is equal to or a descendent of copy circ or owning lib (per the flag), as Dan's comment and suggestion implies.

If both are TRUE, but the owning lib is outside the scope, then it will /not/ send the alert to the client. Likewise, if both are TRUE and the circulation's circ lib is equal to or a descendent of the copy owning lib but is not for the copy's circ lib, it will not send the alert to the client. IOW, /all/ conditions defined on the alert must be met, and so these flags are more about denying the alert than allowing the alert at given locations. That was the thinking behind the label "Do not care" -- that means "we'll accept it" rather than "we DO (or DO NOT) want to see it".

I'm fine with the changes as they stand if they seem clear enough to everyone else, though I'll offer one (very late) option to consider:

  Event location matches restriction locations? [invert=Cannot match|Must match, default=Must match]
  Event can only occur {invert ? 'outside' : 'at'} copy circulation library or descendent: [at_circ=Yes|Not required, default=Not required]
  And event can only occur {invert ? 'outside' : 'at'} copy owning library or descendent: [at_owning=Yes|Not required, default=Not required]

Note: "Must match" for the first (invert) flag is FALSE. "Not required" for the other two means NULL. This is all just UI, and at_circ and at_owning would still be either TRUE or NULL.

Thoughts?

And, finally, apologies if all of this is already crystal clear to everyone, in which case, just ignore the noise. :)

Kathy Lussier (klussier) wrote :

How about something like this? You need to select a checkbox to mandate any location restrictions. Unchecked leaves it at null. The subsequent dropdown is disabled until you select the checkbox.

I think this would make it fairly easy for the user to understand what's happening here. With the Invert and Yes/Don't Care option, I had to stop and think for a few minutes to digest what was happening.

Kathy Lussier (klussier) wrote :

Adding my mockup again with the proper file extension.

Kathy Lussier (klussier) wrote :
Download full text (4.1 KiB)

I took some time to test the latest branch tonight. I'm very sorry that I didn't test the earlier branch sooner since I know it would have been better to have more time to address issues.

I did find a few issues in my testing:

1. I had a couple of problems with checkout alerts. I created a checkout alert owned by the consortium that would display for a normal checkout.
  - If an item was in transit when the item was being checked out, the item in transit alert boxes would display, and I never saw the alert message that was added to the copy.
  - If there were some other kind of alert on the for the transaction (patron exceeds max fines, copy needed to fill a hold), a COPY_ALERT_MESSAGE also displayed, but the message says "The requested copy has an alert message attached." In previous testing, the actual alert message showed, I created a screencast showing three instances of checking out the same copy with an alert message. In each case, we saw different behavior on if and how the alert displayed.

2. If I check in a copy that has an alert with a 'next status' and the copy also needs to go into transit or fill a hold, I can't force the new 'next status' to be applied to the copy. Instead, it wants to go into transit or fill the hold. The next copy status should override those actions.

3. If I try to apply the next status in situations where the item doesn't need to go somewhere else, I get the Star Trek warning alert, but the copy does check in with the new status applied. I see the following in the javascript console (I saw similar messages when trying to apply a Discard/Weed and custom Mending status too)

angular.min.js:119 TypeError: model.assign is not a function
    at ui.js:23
    at angular.min.js:160
    at f (angular.min.js:45)
    at angular.min.js:48
(anonymous) @ angular.min.js:119
circ.js:1472 Unhandled checkin copy status: 14 : Damaged

4. I'm sure this was an issue in my original testing, but I didn't notice it as a problem until now. The use case is a library checks in another library's item and then applies a temporary alert with a Next Status to send a message to the owning library (DVD needs to be cleaned, for example). The assumption is the owning library is the next one that will check in the item and apply that status. However, items sometimes get misdirected in delivery, and a third library may check in the item. There doesn't seem to be a way to proceed with the checkin without applying that next status. I think we need an option there to not apply a Next Status and proceed with the checkin.

5. Our standard system alerts (I tried Missing, Lost, Lost & Paid) are not appearing at checkin, even when I create an alert type for those events. They do appear at checkout. However, if I added them to Copy Alert Suppression grid, they do not suppress at checkout. I didn't try Claims Returned, Claims Never Checked Out or Damaged.

6. In our earlier testing, we had two Library Settings determining how to handle checkout of Lost or Long Overdue items if those alerts are suppressed. I don't see those settings on my test server, but I may just be staring right at them and not seeing them.

Everything else is fairly small (I ...

Read more...

Galen Charlton (gmc) on 2018-02-08
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers