Port Local Admin Shelving Location Order Editor to Angular

[Kyle Huckins]

Evergreen: master

We’ll be porting the Shelving Locations Order UI from DOJO to Angular. This will be a draggable list of available Shelving Locations for the selected Org Unit, and should be relatively straightforward

[Kyle Huckins (khuckins)]

Zavier's having some trouble posting the link, so on his behalf, https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/zbanks/lp1846552_port_local_admin_shelving_location_order_editor

[Zavier (zavierbanks)]

This bug has been resolved and the shelving location order editor has been successfully ported. The branch is ready for review, and the link can be found below:

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

[Jane Sandberg (sandbergja)]

Thanks for this nice interface, Zavier. Here are a few observations:

1) The new table list component is not usable with a keyboard, and does not use the aria-grabbed and aria-dropeffect properties. See here for more info: https://dev.opera.com/articles/accessible-drag-and-drop/ and https://salesforce-ux.github.io/dnd-a11y-patterns/#/sortB?_k=z75uvf

2) If it's not too inconvenient, could you rename copy-location in the code path, CopyLocation in the class names, and copy_location in the route to shelving location, to avoid the mismatched terminology?

3) It's very easy to lose changes if you navigate away from this interface without clicking the Save Changes button. It would be helpful to have an alert if the user tries to leave without saving their changes OR just saving the changes automatically whenever they move a location.

4) I'm wondering if table-list is the best name for the component. Maybe something like sortable-list instead?

[Bill Erickson (berick)]

Removing pullrequest tag pending responses to Jane's comments.

[Zavier (zavierbanks)]

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/zbanks/lp1846552_port_local_admin_shelving_location_order_editor

I made the required updates to the shelving order location.

[Terran McCanna (tmccanna)]

There was a merge conflict when applying this to current master - can you please rebase?

[Kyle Huckins (khuckins)]

I've pushed a rebase here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/khuckins/lp1846552-port-local-admin-shelving-location-order-editor

[Erica Rohlfs (erohlfs)]

I cannot sign off on this patch. It appears that my saves are silently failing. I'm not getting a message window suggesting success or failure. However, I go out of the interface and back in, and my changes have not been saved. Also, when I click the shift key, I don't see any resets occurring (the cursor just pops to the top of the screen like it does when I click the Control key). Also, there is a bit of a display issue - when I hover over a location name, I'm assuming there is supposed to be a text box displaying that location name in larger text. I make this assumption, because I can see the window attempting to display when I click on the location. Interestingly, when I click on a location, then click on the Control key, the text box remains visible, but at the top of the screen. In the attached image, I first clicked on Juvenile Non-Fiction (SYS1), then clicked Control key. The cursor jumps back to the top, highlighting Stacks, but the Juvenile Non-Fiction (SYS1) appeared up there, too. The only way to get rid of the text box is to click elsewhere.

[Ruth Frasur Davis (redavis)]

This error in the console (Chrome) = "Error: Cannot find module '@eg/staff/admin/local/shelving-location/shelving-location.module'"

[Christine Burns (christine-burns)]

I have tested on https://bugsquash2.mobiusconsortium.org/eg/staff/

1. There seems to be an issue with the Context Org Unit selector

If I choose BR2 I am seeing shelving locations listed for CONS, SYS1 & BR2

I would expect to only see shelving locations for BR2

2. I cannot save changes - If I try to save nothing happens on screen and I see an error in the console = ERROR TypeError: Cannot read property 'id' of undefined

[Terran McCanna (tmccanna)]

This one is tricky, because the shelving location order editor wasn't actually working in the first place. See these two bugs, which have some overlap:

https://bugs.launchpad.net/evergreen/+bug/1861526

and

https://bugs.launchpad.net/evergreen/+bug/1712659

My inclination is to close out those two bugs and wrap them into this project - does anyone else agree or disagree?

I am not sure about Christine's comment #11 - I would think that the parent shelving locations *would* be available since they can be selected in the dropdowns when cataloging. Is that not correct?

[Ted Peterson (devted)]

There was a merge conflict when committing Kyle's (#7) rebase (dd53b05bfc03ab34f070ed04c842f63e9a909fd0) so another rebase needs made.

I needed to accept the changes in admin-local-splash.component.html for our bugsquash machine to have the new course reserves under "Local Administration":

Open-ILS/src/eg2/src/app/staff/admin/local/admin-local-splash.component.html

[Kyle Huckins (khuckins)]

Applying a needsrepatch based on the above feedback from the previous feedback fest and prior. I'm inclined to agree with Terran on this, as porting an old non-working UI just to make a shiny non-working UI seems a bit silly

[Terran McCanna (tmccanna)]

Tested again against current master (3.7 beta-ish):

Keyboard accessible select, drag and drop now works great! (click enter to select, then up and down arrows to move, then enter to drop)

However, as per previous discussion, I think these open bugs need to be fixed before this work can be signed off:

https://bugs.launchpad.net/evergreen/+bug/1712659
https://bugs.launchpad.net/evergreen/+bug/1861526

Basically, the Apply Changes button that was broken prior to this still doesn't work (bug 1712659), so changes are not saved. It is unclear whether or not the changes will be picked up elsewhere in the client (bug 1861526) because of the broken save functionality.

I'm going to remove the pullrequest tag on this for the time being, since it can't be fully tested until the changes can be saved.

[Terran McCanna (tmccanna)]

Also noting that I'm getting a new error in this than I was getting in the previous version:

ERROR TypeError: t.unhashed_order is undefined

[Terran McCanna (tmccanna)]

* that I WASN'T getting in the previous version

[Bill Erickson (berick)]

Here's a branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1846552-copy-loc-order-ui

I decided to forgo the drag-and-drop approach, opting instead to use up/down/top/bottom arrows, similar to the grid column manager.

[Andrea Neiman (aneiman)]

Tested on terran-master.gapines.org.

I'm not seeing any of the issues reported in comment #8, yay!

Comment #11 remarked that this was showing parent & grandparent locations, and it is, but this is likely intentional. This also echoes what's currently in the (Dojo) Shelving Location Order interface. I think it looks messy in Concerto as a consequence of test data, and in a real-world situation this wouldn't be an issue. I am not seeing the save failure noted in comment #11.

I am able to edit the locations order and it saves (and includes a confirmation toast in the lower right). The Shelving Location Order interface appears to reflect the changes, but I am not seeing the new order reflected in the item editor or the shelving location search filter -- though echoing Terran's remark in bug 1861526 that there is scant documentation on where this is supposed to show up. I tried logging out & logging back in, and a couple hard refreshes, but no dice. Confirmed with one of my devs that this shouldn't require autogen or anything.

I agree with comment #15 above that bug 1861526 needs to be resolved as part of this port or else there's not really a way to test it.

I do think this addresses the issues noted in bug 1712659 so I've marked that one as duplicate.

The interface in this latest iteration is much improved - clear and easy to use, keyboard-friendly, saving changes (within itself). Now to get the other interfaces to listen to it :)

I've left my edited locations in place on the test server under BR2 - "New Arrivals" for BR2, SYS1, and CONS should be at the top of the list.

[Bill Erickson (berick)]

Thanks, Andrea.

I can confirm the location ordering only affects the holds pull list (related bug: #1919465). It does not affect any of the shelving location selectors, etc. in the staff client.

[Andrea Neiman (aneiman)]

Ah ok, with that in mind, let me re-test. Thanks Bill!

[Andrea Neiman (aneiman)]

OK, so what I'm seeing in the Pull List interface is that the Shelving Locations Order is reflected in the printed list - but not interface view.

I.e., the interface displays the BR2 Pull List sorted alphabetically by Shelving Location. The print preview shows the Shelving Location "New Arrival" at the top, with the remaining Shelving Locations alphabetic. This mateches what's in BR2 Shelving Location Order.

If that is what is expected, then I'll give this a signoff (and update the documentation to reflect that the Shelving Location Order only currently impacts the Holds Pull List).

[Bill Erickson (berick)]

Andrea, just to clarify, the pull list display does not honor the shelf location ordering in the current AngJS pull list or the new Angular one (bug #1919465)?

[Andrea Neiman (aneiman)]

The current AngJS pull list is what I tested against, and it wasn't honoring shelving location order in the interface display - but it WAS honoring it in the printed pull list.

I don't think the new Angular pull list is on the test server I was using (https://terran-master.gapines.org/)

[Josh Stompro (u-launchpad-stompro-org)]

I just tested the shelving location order on https://tiffany-master.gapines.org/ with the new Angular pull list interface, and it does seem to be working as intended.

For BR1 I moved the New Arrivals(BR1) to the top, and Easy Readers(BR1) to the bottom of the list and applied the changes.

After that, the displayed pull list, and the printed pull list used that order.

So I believe that the shelving location ordering is working in 3.8+ for the pull list.

Josh

[Mike Rylander (mrylander)]

I've rebased and signed off Bill's branch; it's ready to go, IMO, I cannot break it. My branch is at https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-1846552-copy-loc-order-ui-signoff

I'll pause for others to test, but I'd like to get this into master well before 3.9, so take a look if/when you can.

Thanks, Bill!

[Bill Erickson (berick)]

Thanks, Mike! I kicked the tires one more time and merged to master for 3.9.