Booking module refresh

[Jane Sandberg]

I am working with BC Libraries Cooperative to make numerous usability improvements to the booking module. The highlights of this project include:
* A redesigned interface for creating new reservations;
* Better management of existing reservations, including a central management screen for staff and the ability to edit existing reservations;
* Allowing staff to add, edit, and remove notes about specific reservations;
* Porting several Booking interfaces to Angular.

I'm attaching a Statement of Work for this project. Several existing bugs will be addressed as part of this project, so I'm marking them as duplicates of this bug.

[Michele Morgan (mmorgan)]

Adding a link to an old bug that still is relevant in case it falls in the scope of this work: bug 1175766.

[Jane Sandberg (sandbergja)]

Thanks for the interest, and for the link, Michele. Unfortunately, that will not be within the scope of this project.

[Bill Erickson (berick)]

Jane, I am working through bug #1821382 -- Angular holdings maintenance -- and there are 2 menu entries in the holdings grid for booking actions: "Make Items Bookable" and "Book Item Now". Will the interfaces that drive these actions be included in your booking refresh work? Just want to be sure we are not duplicating effort, thanks.

[Jane Sandberg (sandbergja)]

Thanks for asking, Bill. Book Item Now will be part of the Booking Refresh project, but "Make Items Bookable" will not.

[Bill Erickson (berick)]

Thanks, Jane!

For Book Item Now, can you please ensure that chunk of code / interface can act as a standalone component? That way the Holdings Maintenance menu option can reuse the code, either directly, or by inserting it into a dialog.

[Jane Sandberg (sandbergja)]

I will be sure to do that, Bill. The dialog option seems more likely.

By the way, for anybody interested, here is my work in progress branch: user/sandbergja/lp1816475_booking_refresh

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

[Jane Sandberg (sandbergja)]

FYI, I've rebased this against master, and added a few bugfixes here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1816475_booking_refresh_rebased_against_ang_catalog

Branch name: user/sandbergja/lp1816475_booking_refresh_rebased_against_ang_catalog

[Jason Etheridge (phasefx)]

Jane, could you rebase again? I'm having trouble reconciling it with current master. Also, if you still have an unsquashed branch, I wonder if you'd mind putting the reusable shared components into a separate commit? What really brought me here was your daterange-select. :D :D Thanks!

[Jason Etheridge (phasefx)]

Oops! I relied on tab completion and actually pulled the wrong branch.

[Jane Sandberg (sandbergja)]

Thanks for your interest, Jason. That branch should now have two commits. The first has the two new date-related components: daterange-select and datetime-select. The second has everything else. :-)

FWIW, the daterange-select is taken almost verbatim from ng-bootstrap's example: https://ng-bootstrap.github.io/#/components/datepicker/examples#range

[Jason Etheridge (phasefx)]

Gracias! I'll try to give booking a lookover as well, but I have no domain expertise there.

[Jane Sandberg (sandbergja)]

A note that this branch is now built atop my branch for https://bugs.launchpad.net/evergreen/+bug/1828840. Ideally, that one will be merged first. :-D

[Jane Sandberg (sandbergja)]

This is ready for community review in the following branch: user/sandbergja/lp1816475_booking_refresh_for_coop

Here is a link: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1816475_booking_refresh_for_coop

Note that this branch is built atop this branch: https://bugs.launchpad.net/evergreen/+bug/1834662, so that one will need to be merged first or simultaneously with this one.

Looking forward to hearing what you think!

[Jane Sandberg (sandbergja)]

One quick note: I had two []s showing up in the Create Reservations screen for my own testing purposes. I forgot to take them out, so I just pushed a follow-up commit to the user/sandbergja/lp1816475_booking_refresh_for_coop branch that removes them.

[Remington Steed (rjs7)]

One quick comment from glancing through the branch: There seems to be a lot of whitespace-only changes, at least in the main "Booking Refresh" commit, in this file:

eg2/src/app/share/grid/grid.component.ts

That makes it hard to tell if there are any non-whitespace changes. Jane, if the whitespace changes were intentional, are you able to move them into a separate commit?

[Jane Sandberg (sandbergja)]

Good point! I've force-pushed a change to that file to get rid of the whitespace changes.

[Terran McCanna (tmccanna)]

Hi Jane,

I'm walking through the steps on this and I encountered a usability issue when creating a Resource Attribute Map:

First, I created two types of attributes, "Number of Seats" and "AV", each with multiple possible values (10/25/50 and Projector/TV, respectively). When I go to create a new atrribute map and I select the Resource Attribute from the dropdown, I expect the Attribute Value dropdown to then only show me the appropriate options, but I see all of the options.

That is, if I select Number of Seats, I should only see 10, 25, and 50 in the Attribute Value options, but I'm seeing all the other atrribute values for the other attribute types as well. (See screenshot.)

[Terran McCanna (tmccanna)]

Point of confusion: On our test server we have Circulator set to have branch level access to the booking perms and we have System Administrator set to the system level.

In testing, I created a Resource Type of "Meeting Room" at the system level, and then created a couple of meeting room resources at the Branch level. That part worked fine. If I log in as a System Administrator which has system-level perms, the Create Reservation options work.

However, when I log in as a Circulator which only has branch-level permissions and tried to create a reservation, the "Choose resource by type" tab doesn't work becuase Search by Resource Type is empty. I assume this is a permissions issue, but shouldn't the dropdown still retrieve the list of system-level resource types even though the circulator only has branch-level perms?

[Terran McCanna (tmccanna)]

Two more notes:

1) When I create a reservation from a patron account, it is not pulling the patron barcode into the form (screenshot attached).

2) Suggest changing "Pickup library" to just "Library" or to "Reservation library" since not everything that is reserved (like a meeting room) would necessarily be picked up.

[Jane Sandberg (sandbergja)]

Good catches, Terran. I will take a look at those.

By the way, for others who are interested in testing this, here's a quick video that walks through the refreshed interface: https://youtu.be/ZO3Eg8wsiPQ. It even includes a long awkward pause when I run into note #1 from Terran's comment #20. :-D

[Terran McCanna (tmccanna)]

The video is very helpful, Jane!

[Jane Sandberg (sandbergja)]

Thanks for your patience as I address your comments, Terran!

For #18: the booking admin screens were part of some earlier development, so I split that off into a different bug: https://bugs.launchpad.net/evergreen/+bug/1840809

For #19: I've pushed a fix to the permissions in the same branch

For #20 1) I've pushed a fix for this in the same branch

For #20 2) I'm hesitant to change the terminology here. "Library" seems overly vague (and I think users might think it's the owning library). "Reservation library" sounds like the library that made the reservation, which might or might not be the library where the user goes to pick up or use the item. I could be convinced, though!

[Terran McCanna (tmccanna)]

Thanks, Jane! We'll try to get the changes tested. For the terminology in #20, what about something like "Location" instead of Pickup Library?

[Tiffany Little (tslittle)]

Re: terminology -- I'm going to throw out "Resource Location"? Or "Reservation Location"? Something to indicate where the resource is to be used/retrieved from, whether that's an item being picked up, or a room reservation, or what. I can't think of a good word for that. I do agree that Pickup Library is a little restrictive, but Library by itself is too vague. Could also include a help popover whatever word is chosen, to help staff if they're unclear about the meaning.

[Jane Sandberg (sandbergja)]

+1 to a help popover -- good idea, Tiffany. How about "Reservation Location" with a help popover that says "The library where the resource is picked up or used"?

[Terran McCanna (tmccanna)]

+1!

[Jane Sandberg (sandbergja)]

Branch updated to include the terminology changes, and fixing a warning that Jeff Davis discovered.

[Jane Sandberg (sandbergja)]

Just threw another commit into the branch to address the issue that Christine pointed out: https://bugs.launchpad.net/evergreen/+bug/1738494/comments/2

[Christine Burns (christine-burns)]

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

[Jane Sandberg (sandbergja)]

Here is a branch rebased against master, including Christine's sign off: user/sandbergja/lp1816475_booking_refresh_more_rebasing

[Galen Charlton (gmc)]

Pushed to master for inclusion in 3.4. Thanks, Jane and Christine.

I've also pushed follow-ups to address some issues I found in testing:

* manage reservations: edit selected modal shows current time as the end time, not the actual end time
* manage reservations: grid doesn't get refreshed after editing a reservation
* lint issues

There remain some issues that should be addressed before the release candidate:

* manage reservations: changing resource type filter doesn't seem to work; need to remove filter first
* booking return: item just returned not showing up in the lower grid without a hard refresh

as well as some display and usability issues:

* create reservation: date input oddly formatted
* create reservation: grid does not make it clear that you need to right or double-click to create a reservation
* pick up and return reservation: similarly, grid does not make it clear that you need to use the context menu
* why does switching to the booking Angular app appear to cause a full refresh?
* booking pull list: formatting of timestamps isn't localized
* patron barcode input should ignore trailing whitespace

[Jane Sandberg (sandbergja)]

Thanks for your testing and pushing, Galen.

I've started a collab branch here to start addressing these issues: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/sandbergja/lp1816475_booking_followup

So far, I added a button to the Create Reservation grid to create a button, which should help address "create reservation: grid does not make it clear that you need to right or double-click to create a reservation" -- props to Christine for the idea. I also fixed a spacing issue caused by a material icon.

I will try to chip away at these, but I'm out of the office, so I may not be super fast.

[Jane Sandberg (sandbergja)]

I threw a few more fixes into that branch. Here are the issues Galen found that I haven't gotten to yet:

* manage reservations: changing resource type filter doesn't seem to work; need to remove filter first
* create reservation: date input oddly formatted (I think that this is due to a material icon's spacing misbehaving)
* why does switching to the booking Angular app appear to cause a full refresh?
* booking pull list: formatting of timestamps isn't localized
* patron barcode input should ignore trailing whitespace

In addition, the BC Libraries Cooperatives crowd had the following recommendation:

* Create --- It's possible to easily (accidentally?) create a reservation in a past. It says "booking created" but the booking is not visible anywhere. This is problematic as you might mistakenly use the wrong day by mistake and not realize, and then you won't realize the booking is gone and/or you can't edit it.

[Jane Sandberg (sandbergja)]

I broke the follow-up work off into its own bug here: https://bugs.launchpad.net/evergreen/+bug/1843938