Angular Acquisitions Sprint 3: Acquisitions Search

Bug #1850547 reported by Andrea Neiman
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

This work is sponsored by the Evergreen Community Development Initiative (formerly MassLNC), the Georgia Public Library Service, the Indiana State Library, and C/W MARS.

Following on the infrastructure work released in 3.4, the next piece of the Angular Acquisitions project is focused on the Acquisitions Search feature. It was originally designated as Sprint 3 but the funding partners opted to move it up in the timeline (don't worry, you haven't missed Sprints 1 & 2 - they are still forthcoming).

Acquisitions Search will be reimplemented in Angular, and there will be a tabbed search interface with one tab each for Purchase Order Search, Selection List Search, Line Item Search, and Invoice Search.

Since full actions in this interface will not be implemented until a later sprint, a link to the Legacy Search interface will be included.

New features include the ability to set a default search per-tab which will be saved as a workstation setting, and a toggle to automatically run the saved search when its search tab is opened.

The approved specification can be seen here:
https://yeti.esilibrary.com/dev/public/techspecs/angacq_sprint3_final.pdf

A branch will be forthcoming at the conclusion of partner testing.

tags: added: search
Galen Charlton (gmc)
Changed in evergreen:
milestone: 3.next → 3.6-beta
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

The feature has passed testing by its sponsoring partners and is ready for a pull request:

user/gmcharlt/lp1850547-angular-acquisitions-search /
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1850547-angular-acquisitions-search

This patch-set includes several new and modified Angular components.

tags: added: pullrequest
Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
status: New → Confirmed
Revision history for this message
Galen Charlton (gmc) wrote :

The branch also includes a patch for bug 1862022. That patch can be reviewed separately if desired, but is useful for anybody who wants to test Angular acq search as an acquisitions administrator.

Revision history for this message
Galen Charlton (gmc) wrote :

I've rebased to account for the upgrade to Angular 10 and the Angular staff catalog now becoming the default. The current branch for the pull request is now:

working/user/gmcharlt/lp1850547-angular-acquisitions-search-v2 /
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1850547-angular-acquisitions-search-v2

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

Did an initial review of the code and UI. I have no major concerns. Everything I tried worked great.

A few requests/suggestions come to mind:

* Allowing staff to save default searches without search values is very useful, but we should probably disable/ignore the 'retrieve results immediately' checkbox when no value is present. On the lineitem tab, for example, it defaulted to search for "id > 0" (i.e. all) -- paged, of course, but probably not ideal.

* Supporting double-click on result rows to perform the same action you could get when clicking on the main identifier would be helpful.

* Suggest moving the 'New Selection List' action out of the actions-for-selected menu and over to an eg-grid-toolbar-button instead, since it's unrelated to the selected items.

* li_state and po_state labels need i18n wrappers in the seed data.

* I think the search form buttons and checkbox could be laid out a little better to reduce stacking on narrower screens (testing with 1350px wide). There's a lot of unused horizontal space to take advantage of.

Having others test as well would be ideal, of course.

Looks good. Thanks!

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

I gave this a look. Some of the testing wasn't as thorough as I'd like, due to some problems using some interfaces. For instance, I wasn't able to set a Payment type on a newly created invoice even when I verified the "Invoice Item Types" had the payment type I was trying to add.

From what testing I managed to do, I see 4 issues:

1. On the 4 tabs used for navigation, one tab says "Inovices Search" s/b "Invoices Search"

2. Beneath the 4 tabs there is a grey box for entering search terms. On the top right of this box is an arrow used to hide the form or show the form. That arrow is invisible to my screen reader (NVDA) and it gets skipped right over when tabbing through the interface. I believe this is because it's using an <a> html tag, but the tag is missing an "href" attribute.

3. I wasn't able to set a default search on any of the 4 tabs. Trying to set a default line item search gives this error:

ERROR Error: Uncaught (in promise): No user or workstation setting type exists for: "eg.acq.search.default.lineitems".
Create a ws/user setting type or use setLocalItem() to store the value locally.

Trying to set a default invoice search gives this error:
ERROR Error: Uncaught (in promise): No user or workstation setting type exists for: "eg.acq.search.default.invoices".
Create a ws/user setting type or use setLocalItem() to store the value locally.

4. Whenever I visit the "Purchase Orders Search" tab I get 3 console errors. I don't run into this on any other tab. Here's the 3 errors:

main.a0af4c705f18a7e99990.js:1 open-ils.pcrud.retrieve.acqpostlbl failed! stat=500 msg=Severe query error -- see error log for more details

open-ils.pcrud.retrieve.acqpostlbl failed! stat=404 msg=An unknown server error occurred

ERROR open-ils.pcrud.retrieve.acqpostlbl failed! stat=500 msg=Severe query error -- see error log for more details

The error logs tell me this:

"2020-08-20 22:52:50 kcls-icelandic open-ils.pcrud: [ERR :3288:oils_sql.c:7047:1597963968363013] open-ils.pcrud ERROR No "datatype" attribute for field "id"
2020-08-20 22:52:50 kcls-icelandic open-ils.pcrud: [ERR :3288:oils_sql.c:7047:1597963968363013] open-ils.pcrud ERROR No "datatype" attribute for field "id"
2020-08-20 22:52:50 kcls-icelandic open-ils.pcrud: [ERR :3288:oils_sql.c:6049:1597963968363013] open-ils.pcrud: Error retrieving acq::po_state_label with query [SELECT "acqpostlbl".id, oils_i18n_xlate('acq.po_state_label', 'acqpostlbl', 'label', 'id', "acqpostlbl".id::TEXT, 'en-US') AS "label" FROM acq.po_state_label AS "acqpostlbl" WHERE "acqpostlbl".id = 'on-order';]: 6844177 6844177: ERROR: relation "acq.po_state_label" does not exist
LINE 1: ..., "acqpostlbl".id::TEXT, 'en-US') AS "label" FROM acq.po_sta..."

Revision history for this message
Andrea Neiman (aneiman) wrote :

Hi Mike - thanks for testing!

I got a good laugh out of the fact that dozens of testers (including me!) failed to notice "Inovices Search". Thanks for catching that.

Did you load the branch to your own environment for testing? Because looking at this on angular-acq-test, I'm able to save default searches & I don't see the console noise on Purchase Orders Search.

Also, this work didn't change anything directly relating to invoices - but when you say "Payment Type", do you mean "Payment Method"? That is set in "Invoice Payment Method" and stock install doesn't include values there. I've tossed a couple values in on angular-acq-test. "Invoice Item Types" (which does come with preloaded values) maps to "Charge Types" on an invoice, which you can see if you add a charge directly on the invoice (under "Direct Charges, Taxes, Fees, etc.)

Revision history for this message
Mike Risher (mrisher) wrote :

Hello Andrea,

Yes, I was loading all this into my own environment for testing. I tried again using angular-acq-test and can confirm I'm not getting the errors I reported yesterday. So you can ignore items #3 and #4 above.

I also managed to create a new invoice and set a "Payment Method" (not "Type") to an invoice, and that Payment Method is displayed when I find it by searching for invoices. So that part looks fine.

Thanks for the help!

Revision history for this message
Galen Charlton (gmc) wrote :

Mike, for reference, the errors you were seeing were likely caused by the schema upgrade scripts in Open-ILS/src/sql/Pg/upgrade note being run, assuming you were starting from an existing database. Specifically, the scripts in question are

XXXX.data.angular_acq_search.sql
YYYY.data.acq_permissions.sql
ZZZZ.schema.acq_state_views.sql

Revision history for this message
Galen Charlton (gmc) wrote :

I have pushed a new branch:

user/gmcharlt/lp1850547-angular-acquisitions-search-v3

This is rebased and squashed and includes the following changes to address the feedback by Bill and Mike:

- "Inovices" is no more
- improvements to the search form for screen readers:
  - controls to add and remove search rows
  - controls to hide and show the search form
- adding the I18N wrapper to seed data for the PO and LI state views
- improving the responsive layout of the search form on smaller screens
- not running the search immediately if there are no search terms set
- adding a double-click handler to each of the results grid to open the selected record in a new tab
- moving the New Selection List action to become a grid toolbar button

Revision history for this message
Jason Boyer (jboyer) wrote :

The changes mentioned in Galen's comment #9 have been applied to https://angular-acq-test.evergreencatalog.com/ if anyone is still grooving on that feedback fest productivity!

Revision history for this message
Mike Risher (mrisher) wrote :

I can confirm the issues I reported are resolved. (The first 2 on Galen's list above)

Revision history for this message
Ruth Frasur Davis (redavis) wrote :

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

Galen Charlton (gmc)
tags: added: signedoff
Revision history for this message
Chris Sharp (chrissharp123) wrote :

I've just tried installing this on my master server and am hitting the following error:

ERROR in src/app/staff/acq/search/picklist-results.component.html:44:36 - error NG8002: Can't bind to 'disableOnRows' since it isn't a known property of 'eg-grid-toolbar-button'.
1. If 'eg-grid-toolbar-button' is an Angular component and it has 'disableOnRows' input, then verify that it is part of this module.
2. If 'eg-grid-toolbar-button' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.
3. To allow any property add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.

44 (onClick)="openCreateDialog()" [disableOnRows]="createNotAppropriate">
                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  src/app/staff/acq/search/picklist-results.component.ts:23:16
    23 templateUrl: 'picklist-results.component.html',
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    Error occurs in the template of component PicklistResultsComponent.

Output of "npm --version":
6.12.0

Output of "ng --version":

     _ _ ____ _ ___
    / \ _ __ __ _ _ _| | __ _ _ __ / ___| | |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__| | | | | | |
  / ___ \| | | | (_| | |_| | | (_| | | | |___| |___ | |
 /_/ \_\_| |_|\__, |\__,_|_|\__,_|_| \____|_____|___|
                |___/

Angular CLI: 10.0.5
Node: 12.13.0
OS: linux x64

Angular: 10.0.8
... animations, common, compiler, compiler-cli, core, forms
... language-service, localize, platform-browser
... platform-browser-dynamic, router
Ivy Workspace: <error>

Package Version
-----------------------------------------------------------
@angular-devkit/architect 0.1000.5
@angular-devkit/build-angular 0.1000.5
@angular-devkit/build-optimizer 0.1000.5
@angular-devkit/build-webpack 0.1000.5
@angular-devkit/core 10.0.5
@angular-devkit/schematics 10.0.5
@angular/cli 10.0.5
@angular/http 7.2.16
@ngtools/webpack 10.0.5
@nguniversal/common 10.0.1
@nguniversal/express-engine 10.0.1
@schematics/angular 10.0.5
@schematics/update 0.1000.5
rxjs 6.6.2
typescript 3.9.7
webpack 4.43.0

Revision history for this message
Galen Charlton (gmc) wrote :

Thanks, Chris. I've pushed a follow-up to the v3 branch to address that problem.

Changed in evergreen:
assignee: nobody → Tiffany Little (tslittle)
Revision history for this message
Tiffany Little (tslittle) wrote :

I tested this on one of our copies of our production data. Everything that I tested looks great. I have one tiny issue, then I'm very excited to sign off.

There should be a little padding between the Set Default Search / Reset Search buttons. Right now they're crammed up against each other.

Thanks Galen!

Revision history for this message
Galen Charlton (gmc) wrote :

Thanks for testing, Tiffany. I've pushed a follow-up to the branch adding the spacing you requested.

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

Thank you Galen!

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

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

Thanks, All. I have added Tiffany's sign offs and my own and merged to master for 3.6. Yay ACQ.

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Bill Erickson (berick) → nobody
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.

Other bug subscribers

Remote bug watches

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