Port MARC Batch Import/Export UI (Vandelay) to Angular(6)

Bug #1779158 reported by Bill Erickson
34
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

Companion bug for bug #1514085.

As part of our continued Dojo deprecation efforts and to solve a number of UI issues with Vandelay UI, the plan is to port the UI to (new) Angular. This will target a post-3.2 milestone (possibly 3.3).

Potentially use this opportunity to address bug #1329852.

Revision history for this message
Bill Erickson (berick) wrote :

We have a lot of interest in improving this locally, so I've started on the migration, assuming it will take some time to complete. My branch in progress:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1779158-ang6-vandelay

This branch will be rebased over collab/berick/lp1775466-ang6-base-app at regular intervals.

Changed in evergreen:
status: New → In Progress
Revision history for this message
Bill Erickson (berick) wrote :

Just noting significant progress has been made on this. The UI is mostly done. Final steps will be to integrate the changes from bug #1514085 (async progress updates) once it's merged, including the ability to exit and return to in-progress imports.

Revision history for this message
Bill Erickson (berick) wrote :

Changes from bug #1514085 have been integrated. I've also added a new interface to Vandelay called "Recent Imports" which displays active import sessions with progress bars and any completed sessions created after the selected date filter.

TODO:
* commit squishes
* release notes

Revision history for this message
Bill Erickson (berick) wrote :

Pushed a few minor repairs, squashed the code commits down to a single commit, and added release notes. I have tested thoroughly along the way, but it's a lot of new code, so I suspect a lot of testing will be needed.

When installing, an 'npm update' is required in Open-ILS/src/eg2/ to pick up the new file-saver dependency (which we also have in angualrjs, fwiw).

The navigation menu is updated to direct the user to the new UI, so testing is simply a case of navigating to "MARC Import/Export" and doing vandelay stuff PLUS testing the new "Recent Imports" interface.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1779158-ang6-vandelay

tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.next
assignee: Bill Erickson (berick) → nobody
status: In Progress → New
Revision history for this message
Bill Erickson (berick) wrote :

I have pushed another commit to this working branch (w/ master branch rebase) to implement the features described in bug #1800481 (import form templates). Includes release notes additions.

Revision history for this message
Tiffany Little (tslittle) wrote :

Bill, we haven't added this branch to our test server so I haven't seen this in action yet, but I had a question--how much of this changes Load MARC Order Records? Or would be easily transferable?

Revision history for this message
Bill Erickson (berick) wrote :

Tiffany, the branch does not touch Load MARC

Revision history for this message
Bill Erickson (berick) wrote :

Arg, didn't mean to submit yet...

The branch does not touch the Load MARC Order Records interface or make any other changes to Acquisitions. Those instances of the Dojo-based UI are still there as before.

The idea would be to replace all occurrences of the Vandelay UI over time, but I'm not sure if that needs to be a requirement for this bug.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Doing some testing on this - I noticed that, in the event of a page refresh, the templates list is cleared. Navigating away and back persists the data, so it's only in the event of a refresh.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Correction for my last post, navigating away to the other vandelay pages doesn't persist the data, it only seems to persist when I press the home button and come back to the page.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Kyle. To clarify, you enter some import form values, give the new template a name, then save the template. Then you navigate to a different Vand. tab, come back, and the saved template is not in the list?

Revision history for this message
Kyle Huckins (khuckins) wrote :

That's correct. I've used a combination of different values for a varying amount of templates, saving each one and confirming they then populate the dropdown and apply settings as expected. Upon leaving and returning to the UI, the Apply/Create Form Template dropdown is empty, only containing the placeholder text.

Revision history for this message
Kyle Huckins (khuckins) wrote :

Disregard my previous comments, I double-checked that the sql updates were applied, and the issue doesn't recur.

Revision history for this message
Jane Sandberg (sandbergja) wrote :

It's looking good so far, Bill. A few things I noticed:

* In the dojo version of the import tab, the Remove MARC Field Groups selector disappears when the user selects authority records from the dropdown. In the angular version, it does not. I'm not sure if that functionality just doesn't exist yet, but it seems like it should disappear or be greyed out to maintain parity. In addition, it also seems like "Auto-overlay In-process Acquisitions Copies" should disappear/be inactive for authority records, since it's not relevant to authority records.

* I wasn't able to upload an authority record at all. I got an error message in the console: "ERROR open-ils.vandelay.authority.process_spool failed! stat=404 msg=Method [open-ils.vandelay.authority.process_spool] not found for OpenILS::Application::Vandelay"

* There is some indication of which fields on the import tab are required (a great improvement over dojo! Thanks!), but it relies on color. This is a problem for blind and colorblind users: https://www.w3.org/TR/2016/NOTE-WCAG20-TECHS-20161007/F81

* The <input>s in the import tab are not associated with their labels. They should be associated for better accessibility and usability.

* In general, the import tab has a cluttered feel. It might be easier for users if there were some more separation + whitespace used to group like settings.

* There is a misspelling: "Select or Create a Qeueue" -- "Qeueue" should be "Queue"

* A usability issue: the upload button should light up after I type in a name for the queue and select a file to upload. However, if I start by choosing a file to upload, then type a name for the queue, the upload button does not light up until after I remove focus from the queue name field, by clicking out of it or tabbing to another field.

Additionally, I'm not totally sure when I'd use the recent imports tab and when I'd use the inspect queue tab. Could you please tell me a bit more about why those would be two separate tabs?

Thanks very much for your work on this! It was fun to try it out. Since you mentioned that you want a lot of testing for this, I also gave this the needsdiscussion tag, so hopefully more end users can take a look.

tags: added: needsdiscussion
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Jane! Couple of notes...

The authority import issue is related to a bug in the new (3.2) Vandelay session tracker code. It will affect all interfaces. LP entry forthcoming on that.

The main purpose of the recent imports tab is to have an entry point to check the status of long-running imports. This way staff can navigate away from the import UI and return later to check the progress.

Suggestions on how best to group/structure the imports tab appreciated.

Revision history for this message
Jane Sandberg (sandbergja) wrote :

Thanks, Bill. Is there any way that the Inspect Queue and Recent Imports tabs could be combined into a single tab with a big huge grid, that could be used to check on any upload, whether it be currently running, recently completed, and long-ago completed?

Revision history for this message
Bill Erickson (berick) wrote :

I have rebased to master and force-pushed these changes:

* Associate <input>'s with their labels in the import and export forms
* Hide the MARC removal groups selector when record type is authority.
* Disable ACQ copy overlay option when record type is authority.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've opened up bug #1806968 in regards to an issue with record_type, relating to the process_spool error Jane noted. I have an experimental branch that still needs some testing, which I'll be posting to that branch shortly.

Kyle Huckins (khuckins)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
Revision history for this message
Kyle Huckins (khuckins) wrote :

I've been looking further into the authority record import issues, with the changes introduced in bug #1806968, with a working branch here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/khuckins/lp1779158-ang6-vandelay

First thing I should note, with the changes in the aforementioned bug, it appears that auth records are properly imported. Upon viewing their queue, however, a new issue pops up: nothing displays in the grid, and errors are thrown that rec.import_items() isn't found. This is because we're trying to return authority records(IDL: vqar), which doesn't have the import_items field. Bib records(IDL: vqbr) do have the import_items field, which has a has_many relationship to Imported Items(IDL: vii) class' record field. vii.record() is, itself, a link to a particular vqbr. This seems like a potential design issue, where the current vandelay fieldmapper implementation is designed almost entirely around bib records. This might be worth opening a separate bug for in the same way that bug #1806968 was better off as a separate issue.

Revision history for this message
Kyle Huckins (khuckins) wrote :

After some discussion with Bill, we decided to go with a front-end based approach. The above collab branch has been updated to resolve the issues I've noticed with importing authority records.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've identified a couple more issues, and am working on fixes for them:

1. Selecting a single item from the bib queue takes you to that specific item's information. The same functionality exists for auth queues, however the interface breaks when attempting to go to a single item from a queue.

2. When inspecting an auth queue, "Imported As" links to a bib record rather than an authority record.

I've also pushed a commit that resolves the issue where no actions were available when inspecting auth queues.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've done some rebasing on my prior commit, resolving the issues I noted earlier today. klbeck is currently working on an improvement to turn the "Match Set Type" text input into a Combobox when editing or creating a new record match set.

Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

New commits pushed here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1779158-ang6-vandelay

1. Sign-offs for Kyle's bug fix commits.
2. Adding an AlertDialogComponent
3. Using the new AlertDialogComponent to alert users about duplicate queues.
4. Only display complete=false queues in the import dialog queue selector.
5. Ensure newly created queues are added to the local queue cache.

===

Regarding the Match-Set-Type combobox, we'll need to 1) teach fm-editor to use comboboxes instead of <selects> for lists of things and 2) we'll need to teach the fm-editor how to accept a list of values to pass to the combobox, since in this case the values are plain text and can't be derived from the database via foreign key. This will also impact bug #1809288, which has fm-editor tweaks.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

Did some more testing today and found a couple of other small issues. Pushing fixes shortly...

Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

I have pushed 3 more commits by my branch:

1. Repairs/additions to workstation settings
2. Fixed an import template variable name error
3. Always clear the import selection when navigating away from the import UI.

Bill Erickson (berick)
Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

Opened for combobox and canned-options support: bug #1811288

Revision history for this message
Bill Erickson (berick) wrote :

Branch (user/berick/lp1779158-ang6-vandelay @ working) rebased to master and updated with Ang7 and ng-lint repairs.

Note that once bug #1811288 is merged, we'll need to provide default values to the fm-editor when setting values for "Match Set Type"

Bill Erickson (berick)
tags: added: angular
Dan Wells (dbw2)
Changed in evergreen:
assignee: nobody → Dan Wells (dbw2)
milestone: 3.next → 3.3-beta1
Revision history for this message
Dan Wells (dbw2) wrote :

This still has a few rough edges (in particular, I agree that the Import screen needs some kind of grouping, a la the old version), but it is so full of general goodness, it needed to get in. It is also a viable gateway drug for hooking folks/devs on the new Angular, IMHO. Thanks for spearheading this, Bill!

I am closing this out, and will open a new bug for the Match Set Type dropdown.

Changed in evergreen:
assignee: Dan Wells (dbw2) → nobody
status: New → Fix Committed
Revision history for this message
Dan Wells (dbw2) wrote :

Follow-up is now bug #1816679.

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.