Angular: Local Admin Circulation Policy Port

Bug #1855781 reported by Kyle Huckins on 2019-12-09
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Unassigned

Bug Description

Evergreen: master

We’ll be porting the Circulation Policies UI from DOJO to Angular. This appears to be a grid with a fairly extensive fm-editor, and linked (Circulation) Limit Sets. Some of the code done for the Surveys Port(bug 1845240) may be useful inspiration.

Jane Sandberg (sandbej) on 2020-01-06
Changed in evergreen:
assignee: nobody → Jane Sandberg (sandbej)
Jane Sandberg (sandbej) wrote :

Thanks very much for the branch, Zavier and Kyle. I think there are two things that this needs before being signed off:

* When I run ng lint, there are several lint errors. Most of them look pretty easy to fix (like missing semicolons, etc.)
* The dojo interface had a small line dividing fields used to match circulation policies vs. the actual policy effects. I think that some sort of separation is necessary. It doesn't have to be that little line -- actually, it would be nice to have a heading or something else that says what the groups actually are.

Changed in evergreen:
assignee: Jane Sandberg (sandbej) → nobody
Tiffany Little (tslittle) wrote :

I'm looking at this on the feedback fest server and have a few observations.

* I do see the dividers, but it looks like there's no padding between the dividers and the fields, so it causes the dividers to overlap into the fields slightly.

* The Cancel and Save buttons at the bottom of the form are misaligned. Cancel is too far down.

* Aside from the Permission Group, it doesn't appear to show which fields are required. It looks like at the very least Active, Org Unit and Group can't be null but only Permission Group appears to have the "Required?" flag.

* Following up on the previous bullet, if you save the editor with only active=true and permission group set (no org unit set), the form saves successfully. However, it appears to randomly pick an org unit.

* I'm an avid proponent of help popovers, so a help popover next to the Add button for Circ Limit Set Name saying that these are set in Local Admin > Circulation Limit Sets would be lovely. But that's a wishlist item and others may not think it necessary, so not a dealbreaker for me either way.

Joan Kranich (jkranich) wrote :

I am also looking at the feedback fest server.

When I click in the Circ Limit Set Name nothing displays. If I start to type Test then I see the Testme set.

When I click in boxes Copy Circ Lib, Copy Owning Lib, and Copy Location a list of codes display. It would be good if the Circ Limit Set Name displayed a list.

I wasn't able to add a Circulation Modifier so I could not test the behavior of that box.

Changed in evergreen:
milestone: none → 3.5-alpha
Zavier (zavierbanks) wrote :

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

Here's the updated branch.There is still an issue with the required fields however. Checkboxes
and org unit selectors cannot be labeled as required. So
there are fields that are required, but can't be labeled
as such.

Jane Sandberg (sandbej) on 2020-03-07
Changed in evergreen:
importance: Undecided → Wishlist
Changed in evergreen:
milestone: 3.5-beta → 3.5.0
Changed in evergreen:
milestone: 3.5.0 → 3.5.1
Jane Sandberg (sandbej) on 2020-07-21
Changed in evergreen:
milestone: 3.5.1 → 3.6-beta
Jennifer Bruch (jbruchpails) wrote :

Can someone check to see if this bug persists? I just ran into it on 3.3.4 for PaILS.

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

Jane Sandberg (sandbej) wrote :

I did some more review of this branch today. It is looking good (and Jennifer, it does take care of bug 1771875), but I did catch two issues:

1) The shelving location dropdown is not showing the org unit labels next to the names of the shelving location. Right now, the dropdown includes entries like:

Stacks
Stacks
Stacks

which should actually be:

Stacks (BR1)
Stacks (CONS)

and so forth. The new <eg-item-location-select> component seems like a good tool for this task, but it's not quite ready to do so. That component takes an OU (or the workstation's OU) and generate a list of locations at that OU or its ancestors. However, an administrator might be working on shelving locations at a sibling or cousin OU, and would be unable to do so with the <eg-item-location-select> as it currently stands.

2) A few of the boolean fields can be NULL in the database. In the dojo UI, this was represented by dropdowns with the values "True", "False", and "Unset"/"Inherited". The Angular version doesn't give this option -- it only allows for True/False. Here are the ones that can be NULL:
* Renewal?
* Juvenile?
* Reference?
* Circulate?

Jane Sandberg (sandbej) wrote :

Oh, and one other thing! I wasn't able to save my grid settings (additional columns, etc.)

Changed in evergreen:
milestone: 3.6-beta → 3.next
Lynn Floyd (lfloyd) wrote :

I was testing this on the bug during Bug squashing week, and among the things in comment #8. I could not see any thing in the grids at all. I checked this on several different Bug Squashing Week servers.

Terran McCanna (tmccanna) wrote :

In addition to the issues Jane found in comment #8, I have a few more:

1) The option to "Save Grid Settings" is missing

2) The interface has the option to select multiple rows and Edit Selected rows, but it looks like that only edits one of the selected items, which is confusing.

3) I don't see an option to Delete a circulation policy row, but I don't see that in the old interface either, so that may be intentional.

I'm going to remove the pullrequest for the time being.

tags: added: needsrepatch
removed: pullrequest
Lynn Floyd (lfloyd) wrote :

Update on refreshed version on test server during Bug squashing Week.

1. Grid settings cannot be saved.
2. Copy Location still does not contain owning Org unit
3. Nullability does not exist on fields that are nullable.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers