Angular: Local Admin Statistical Categories Port

[Kyle Huckins]

Evergreen: master

We’ll be porting the Statistical Categories UI from DOJO to Angular. This is effectively a tabbed grid, with an inline form like the fm-editor, but which will need to show certain fields depending on the type of statistical category.

[Mike Risher (mrisher)]

I pushed a branch with this work:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat

[Mike Risher (mrisher)]

In response to feedback from the demo, I revised the work. The button for new stat cats is now consistent with other grids. Same branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat

[Anna Goben (agoben)]

I'm not in a place to test the branch, but was wondering if this port also addressed Bug #1752367?

[Anna Goben (agoben)]

Ah, I see from the git notes that it does. Sorry for the confusion.

[Tiffany Little (tslittle)]

This seems to work well for me!

I have tested this code and consent to signing off on it with my name, Tiffany Little, and my email <email address hidden>.

[Jennifer Pringle (jpringle-u)]

On the Copy Editor Category Entry Editor page when you add a new entry the Entry Owner and Stat Cat fields should be pre-populate based on the statistical category you selected on the previous screen. Ideally you would not be able to edit either of these fields; if you want to create a new entry for a different stat cat you should have to select that stat cat from the previous screen.

[Jennifer Pringle (jpringle-u)]

Currently you can save a stat cat value with a different Entry Owner org unit (ie CONS) than the stat cat Owning Library (ie. BR1)

[Terran McCanna (tmccanna)]

Removing signoff as per Jennifer's comments.

[Lynn Floyd (lfloyd)]

Testing this one, there seams to be a major disconnect between Stat cats and permissions also.

It's looking like anyone can edit any stat cats.

[Mike Risher (mrisher)]

I've made an update. Now when creating a new entry the stat cat and owner fields are set to the current stat cat's values by default. They cannot be edited.

This branch has the stat cat updates: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat-default

Please note -- This work required a new FM editor feature that allows you to set a new default record. That new feature is not yet on master, so to make this work I needed to build off of unrelated work done elsewhere. My work is based off of Jane Sandberg's branch mentioned in comment 8 over here: https://bugs.launchpad.net/evergreen/+bug/1840287

[Mike Risher (mrisher)]

Thanks for pointing out the issue with permissions. I've revised this work in response. Now you need create permission to create, delete permission to delete, etc. Some notes:

1. There is a read permission, but it appears to be unused in the old UI. Instead the old UI uses the permission to update as the permission to read. I kept it this way.
2. There is a default stat cat entry permission, but I've checked the old UI and don't see the default stat cat in use. I've kept it the same in the new UI.
3. When creating a new stat cat the owning org always matches the current focus org.
4. When creating a new stat cat entry the owning org always matches the parent stat cat's org.

Let me know if there are objections to any of the above or other feedback. This is ready for another look. Thanks! Branch here:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=008520b772bc5690e1c0c5454d99890445ccfb36

[Mike Risher (mrisher)]

Made a small adjustment to insure the grids refresh properly when items are deleted.

Branch here:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat-perms

[Mike Risher (mrisher)]

Another adjustment was made so that stat cat entries can have the same org as the parent stat cat or a descendant org:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat-perms

[Lynn Floyd (lfloyd)]

Just tested this on https://acorn.evergreencatalog.com/eg2/en-US/staff/admin/local/asset/stat_cat_editor
and several others. It's giving just a configuration screen with no headings, titles, nothing. See attached image.

[Terran McCanna (tmccanna)]

This is installed on https://butternut.evergreencatalog.com for bug squashing week and I'm seeing it appear (logged in as admin) and was able to add a stat cat entry with no problems, but I haven't fully tested it.

[Andrea Neiman (aneiman)]

Testing this on butternut, using a BR1 Local Admin user.

I can successfully create, edit, and remove both patron and item stat cats. I can edit and remove entries for each as well.

Patron Stat Cats:

Good news: this work seems to resolve bug 1888682 ; however, there are no warnings/constraints for deleting a stat cat entry that is in use. The patron record(s) in question retain the (now-deleted) stat cat value, but the deleted entry is not available to a patron with the category previously not set. This might be existing bug 1091805.

Similarly, if I edit a patron stat cat entry, the same behavior occurs - patrons with that existing stat cat entry are not updated. Since it seems that stat cat behavior in the past didn't allow editing of individual entries, I'm not sure what the precedent should be here.

I cannot delete a patron stat cat if there are patrons with values set for that stat cat - I get an error toast.

There does not appear to be a way to unset a patron stat cat from the patron edit screen.

Patron stat cats used to include an option "Default entry for [OU dropdown]" though it looks like this wasn't working, or at least it's not working for me on a different (3.5) test server.

Item Stat Cats:

Deleted item stat cat entries appear to be removed from the item record, though I am not warned or otherwise prohibited from deleting said entries from the stat cat editor.

Editing item stat cat entries updates the item record to the new stat cat value.

I can delete an item stat cat even if there are items with values set in that stat cat.

I think this needs some additional testing and a deeper look from someone more well-versed than I in the ins & outs of stat cats.

[Erica Rohlfs (erohlfs)]

Testing for Bug Squashing Week.

When adding entries, it would be nice to have a more intuitive way to go back to the main statistical category screen without clicking the back button. For example, if I'm editing the entries for a patron stat cat and finished. I have to click the back button, which lands the screen back on the default Copy stat cat tab. So, I think have to click on the Patron stat cat tab to continue working on patron stat cats.

In the DOJO interface, the user must select a specific stat cat to delete. In Angular, it's possible to select multiple (or all) stat cats, then select Delete. If the stat cat is not available for deletion (such as patron stat cat entries being assigned to patron records), then the entire action fails. So, even if a stat cat selected within the list is delete-able, it will not delete.

I'm not seeing OPAC Visible stat cats within the patron OPAC account, but I'm not 100 percent certain that functionality ever existed.

Like Andrea, I cannot delete a patron stat cat if it's associated with a patron record. I can delete a copy stat cat even if it's associated with a copy record. Within the DOJO interface, when deleting a copy stat cat, a message box appears reading "This will delete the selected statistical category and all attached entries. Are you sure you wish to continue?"

I would recommend not signing off on this until either the copy stat cat cannot be deleted when associated to the copy record or the warning message is restored.

[Mike Risher (mrisher)]

Thanks for all the great feedback. I've modified the port so that now it won't let you delete a copy stat cat if it's in use. Likewise, if trying to delete several at once it will fail if one of them is in use. Branch here ready for another look:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat-v2

re: "it would be nice to have a more intuitive way to go back to the main statistical category screen without clicking the back button."

I have an idea how to fix this issue, and plan to revisit it to make this improvement.

[Mike Risher (mrisher)]

I made an adjustment so that when editing stat cat entries you can return to the stat cat UI and you return to the tab you were at previously. In other words, when editing patron stat cat entries, you can click a button which will take you back to patron stat cats.

This latest commit is based off the latest master and has all adjustments made to date:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1857911-stat-cat-v3-tabs

[Jennifer Pringle (jpringle-u)]

Copy Statistical Categories look good.

Currently, nothing happens when "New Patron Statistical Category" is clicked. I get the following error in the Chrome Console.

 main.aaf699093452f3ed3a31.js:1 ERROR TypeError: Cannot set property 'mode' of undefined
    at u.newStatCat [as action] (9.24b9ef0c7b9b1e6a81cb.js:1)
    at e.performButtonAction (7.bd759607910def0203ba.js:5)
    at 7.bd759607910def0203ba.js:5
    at _o (main.aaf699093452f3ed3a31.js:1)
    at i (main.aaf699093452f3ed3a31.js:1)
    at HTMLButtonElement.<anonymous> (main.aaf699093452f3ed3a31.js:1)
    at l.invokeTask (polyfills.bfd9691d61468b064477.js:1)
    at Object.onInvokeTask (main.aaf699093452f3ed3a31.js:1)
    at l.invokeTask (polyfills.bfd9691d61468b064477.js:1)
    at i.runTask (polyfills.bfd9691d61468b064477.js:1)

[Mike Risher (mrisher)]

Thanks for catching that. I found the error. I only needed to adjust one line to fix. Branch with the fix is the same:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=733a25e7701c6a72537b1bc92047ecfff488b0a2

[Jessica Woolford (jwoolford)]

Works well for me. However, there used to be a way to make a patron stat cat entry the default value for a particular OU. This has never worked in the web client, but we used in XUL. As of 3.5.2, there is still a table in the DB that references this (actor.stat_cat_entry_default). It saved library staff a lot of clicks in our particular use case. Is there any thought to adding that back to the GUI, or is that beyond the scope of this bug ticket?

[Kyle Huckins (khuckins)]

It sounds like a small enough enhancement, but definitely worthy of its own bug ticket once this is merged

[Jennifer Pringle (jpringle-u)]

The angular interface makes it possible to edit and delete stat cat entries but I'm not sure the current behaviour is the desired behaviour - see https://bugs.launchpad.net/evergreen/+bug/1752367

[Jennifer Pringle (jpringle-u)]

In the angular interface it's no longer possible to edit the owning library for a stat cat. This is possible in dojo interface if you have the right level of permissions (ie. a global admin can edit in dojo but not in angular).

I'm not sure how important the ability to edit the owning library is.

[Mike Risher (mrisher)]

Do want want this port to be able to edit and delete stat cat entries?

[Kyle Huckins (khuckins)]

+1 on needsdiscussion, I think it'd be good idea to iron out a list of requirements that might not be obvious(such as permissions required for specific actions), to avoid juggling sign-offs

[Ted Peterson (devted)]

Had a merge conflict with master after adding this working branch.

Fixed:
vi Open-ILS/src/eg2/src/app/staff/admin/local/routing.module.ts /var/tmp/merges/

Merged the two new paths for asset/stat_cat_editor and asset/stat_cat_editor/:tab.

Was careful to add "}, {" where the "===" was, which was stripped-out in the diff.

[Ted Peterson (devted)]

please disregard "/var/tmp/merges/" in the above comment (#28).

[Jennifer Bruch (jbruchpails)]

I messed around with the fix today on the Mobius Test Server.

I was able to create patron Stat Cats, assign them, then delete or edit them in the Admin interface. The patrons were not updated with the change, but I do not see this as a huge issue, just the beginning of a data clean up project.

As a user and administrator, I feel that being able to edit these Stat Cats in the port is a valuable addition and a step closer to addressing a need. I have two libraries that want to do this right now. Even better if the issue of being able to unassign a stat cat back to a null or blank value would be great too.

Do the permissions issues need to cleared up, yes.

Adding a message when editing or deleting Stat Cat to inform the user that patrons with those stat cats need to be reviewed and updated would be useful.

Another useful thing would be a message on the page about the ideal workflow of editing and deleting stat cats would be ideal as well.

As a user, I would prefer to be able to make the change and then do a report of the users affected and then I can edit the users in a bucket to have a new stat cat assigned.

[Christine Morgan (cmorgan-z)]

I looked at this on the Mobius test server, specifically in relation to how it affects the holdings editor. I created a few stat cats, one of which I made required. I found the new interface to be a great improvement over the old one.

In the holdings editor I noticed the following:

- I could not distinguish between the required stat cats and the stat cats that were not required. They have the same appearance. Currently, if a stat cat is required it is highlighted in red.

- I found that I was able to save a record without setting the required stat cat. Currently the Save & Exit button is greyed out until the required stat cats are set.

[Jennifer Weston (jweston)]

Tested on MOBIUS server during BSW.

I would be in favor of not including the new edit/delete functionality to the port initially to allow creation of new bug for discussion of behavior, permissions, and additional testing.

In my testing, I noticed the following:

-There is no option to Save the Column grid configuration on either the Stat Cat Editor or the Copy Stat Cat Entry Editor.

-Cannot unset the patron stat cat from the patron screen (this is also true in current version)

-Cannot unset the copy stat cat on the Holdings Editor where currently you have the option to choose “None”

-Allows deletion of patron stat cat entry on stat cat entry editor page even if that stat cat is being used by a patron account

-Copy Stat Cat options are displayed on Holdings Editor that are not used by the copy’s owning library; this affects libraries that do centralized cataloging. For example, you are editing a copy belonging to BR1 on a workstation registered to BR3, all stat cats display regardless of stat cat owning library. You can choose a stat cat owned by BR3 for a copy owned by BR1 and save.

The new interface is a nice improvement over the old one. Thanks for all of the work.

[Terran McCanna (tmccanna)]

Removing pullrequest due to testing comments 30, 31, and 32

[Elizabeth Thomsen (et-8)]

Confirming that there has been no apparent progress on this since March 2021. A lot of work in both coding and testing has already gone into this. The needsdiscussion tag was added but if there has been discussion, it's not reflected here. I am sending this to the EG Catalogers group to see if we can help get this one moving again!

[Andrea Neiman (aneiman)]

Equinox is actually working on finishing this up, with King County Library System as funding partner.

I've updated the assignment to reflect this.

We welcome any input from the Cataloging Interest Group but the plan right now is a straight Angular port.

[Elizabeth Thomsen (et-8)]

Well, that's good news!

[Jason Etheridge (phasefx)]

Top four commits at collab/phasefx/lp1857911-stat-cat-admin-alternative

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/lp1857911-stat-cat-admin-alternative

For those interested, more granular commits at:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/stat-cat-admin-wip-alternative

[Blake GH (bmagic)]

Bugsquash week: conflict on the docs, I'm going to get it merged onto my test machine anyways. Just FYI.

[John Amundson (jamundson)]

Testing this for bug squashing week on https://bugsquash.mobiusconsortium.org/, and I'm noticing a couple things so far.

1. Org Unit Selector doesn't "stick" when pulling up entries, which is fairly annoying when trying to edit CONS-owned entries. For example, the server has a patron stat cat called "Non-English Primary Language" owned by CONS, and when I click Entries, no entries are returned at first because it defaults back to the workstation library. This is confusing. Ideally, the chosen org unit should persist.

1a. In a similar vein, when you go to add an entry to a CONS stat cat, for example, the entry owner defaults to the workstation, as well, which is also annoying/confusing.

2. There doesn't appear to be any way to mark an entry as a default for the stat cat. In the old editor, you can mark an entry as default. As I write this, I'm reading past comments that mention that this option didn't work in the web client. I haven't found that to be true. It just didn't work in Chrome, but it works fine in Firefox - see attached screenshot. Since this feature is missing, I would consider that a regression. Note in the screenshot that default entries are marked with an * in the Entries dropdown field, as well.

[Lindsay Stratton (lstratton)]

Testing https://bugsquash.mobiusconsortium.org/, logged in as admin / CONS.

Using the new Item Statistical Categories Editor, cannot create a new stat cat.

I *am* able to create a new stat cat using the old style Statistical Categories Editor and the new stat cat is visible in the updated interface.

Similarly, cannot create a stat cat entry in the updated interface, but can in the old interface.

Cannot edit stat cat entries. In new interface, the only one with the edit functionality, the toast indicates success but no change is saved.

[Terran McCanna (tmccanna)]

Removing pullrequest due to comments 39-41

[Andrea Neiman (aneiman)]

Thanks for the testing & feedback, all. I'll read this all more carefully tomorrow.

I will note that in comment #41 by Lindsay that registering a workstation at "CONS" for testing purposes can cause all kinds of weird failures to occur.

This is because CONS by default shouldn't have workstations, BUT "admin" is a superuser that can register a workstation at CONS anyway. So I'd recommend testing at a non-CONS workstation to see if the same issues are occurring.

[Lindsay Stratton (lstratton)]

I retested using a HPPL workstation, logged in as admin. Outcomes were the same.

I was able to *delete* a stat cat, which I didn't test previously.

[Elizabeth Davis (elidavis)]

I tested using HPPL yesterday and was unable to add new categories as Lindsay shared.

[Andrea Neiman (aneiman)]

Thanks, both - I just saw Blake's message about some issues with the MOBIUS test server so I would appreciate a retest if that's not too much to ask!

(adding / editing were specifically tested by me on this branch recently so I want to try to track down where the issue is)

[Lindsay Stratton (lstratton)]

I retested, again using HPPL / admin login. Was successfully able to create a stat cat using Item Statistical Categories Editor, and add entries. Also able to edit (and delete) both stat cats and entries.

[Andrea Neiman (aneiman)]

Thank you Lindsay! We'll take a look at the other issues mentioned.

[Shannon Dineen (sdineen)]

BC Libraries reports for bug squashing as follows:

As patron records are not updated by edits and deletions in the editor, and no warning is provided when deleting a patron stat cat in use, BC Libraries Cooperative does not think this is ready for prime time, even though it mostly works. We would prefer to see function/feature parity with the Item stat cat editor and affected records

I can successfully add, edit and delete patron and item stat cats using the angular editors.

I found navigating non intuitive, I do not see a button to get back to main screen of editor, had to use the browser back arrow.

Agree that org unit selector for Ancestors/Descendant should be sticky when selected, thanks for addressing that.

[Jason Etheridge (phasefx)]

I've force-pushed a new version of the branch, collab/phasefx/lp1857911-stat-cat-admin-alternative, addressing some of the things brought up.

    * AdminPage component -> Return button if page was invoked with gridFilters
    * AdminPage component -> options to hide delete and edit actions
    * Stat cat admin pages -> disable edit and delete for stat cat entries to match behavior of legacy interfaces, until we discuss something better
    * AdminPage component -> stock delete confirmation for AdminPage component
    * Stat cat admin pages -> custom delete confirmation prompts for stat cats
    * AdminPage component -> sticky org selector options for org fields in fmEditor
    * AdminPage component -> option for new record org fields to follow context org
    * Stat cat admin pages -> enable org field follows context org featuer
    * orgFamilySelect -> persistKey support
    * Stat cat admin pages -> use a persistKey for main org selector

[Jane Sandberg (sandbergja)]

Merged for inclusion in 3.11. Lots of nice improvements to shared components in this branch!
 Thanks, Jason and all the many folks who have worked on this ticket in various roles.

I pushed a follow-up commit to get the test suite passing again -- one of the mocks needed to be updated to accept the nice new AdminPageComponent options.