wishlist: Acquisitions Sprint A - Z39.50 and other fixes

[Andrea Neiman]

Belatedly realizing this never got a Launchpad item! My bad.

Anyway, Sprint A encompasses many pieces of Acq and many other bugs (all of which have now been marked duplicate). Z39.50 and PO bugfixes are the primary goals of the work.

- https://bugs.launchpad.net/evergreen/+bug/980115
- https://bugs.launchpad.net/evergreen/+bug/1768846
- https://bugs.launchpad.net/evergreen/+bug/1659709
- https://bugs.launchpad.net/evergreen/+bug/1909598
- https://bugs.launchpad.net/evergreen/+bug/1775885
- https://bugs.launchpad.net/evergreen/+bug/1740898
- https://bugs.launchpad.net/evergreen/+bug/1735490
- https://bugs.launchpad.net/evergreen/+bug/1830442
- https://bugs.launchpad.net/evergreen/+bug/1167380

Full specifications are here:
https://yeti.equinoxoli.org/dev/public/techspecs/angacq_sprintA_202305.pdf

[Ruth Frasur Davis (redavis)]

ECDI is in bugfix stage for development of this sprint

[Mike Rylander (mrylander)]

A branch for this AND the Sprint A work on bug 2006970 is available at:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-2006970-2039609-acq-sprints-B-and-A

Please see the release notes and updated documentation (top 2 commits) included in that branch for all the details.

[Ruth Frasur Davis (redavis)]

I and other ECDI testers have tested this code and consent to signing off on it with my name, Ruth Frasur Davis and my email address, <email address hidden>.

[Blake GH (bmagic)]

The permissions added to permission.perm_list in the DB upgrade script aren't also reflected in the seed data. So it seems that only the folks upgrading will receive the required permissions. The ID numbers are old too, creating conflict with current main.

Here's a collab branch with those lined up:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/blake/lp2039609_acquisitions_sprint_a_z3950_and_other_fixes

[Blake GH (bmagic)]

I added another commit to that collab branch. I found an erroneous Windows link file and some docs "NOTE:" lines that don't make the ASCIIdoc "NOTE" graphic properly.

[Andrea Neiman (aneiman)]

This is available on public test server https://butternut.evergreencatalog.com/ (admin / demo123 ; Concerto data).

[Terran McCanna (tmccanna)]

Not all of the commits are labeled with the LP bug number so it's difficult to tell which are relevant - is it the top 6 commits at https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/blake/lp2039609_acquisitions_sprint_a_z3950_and_other_fixes ?

[Bill Erickson (berick)]

I was doing some code review and noticed most of the commits do not have the LP number in the message. Do we want the commits to log their original bugs or just tag them all with LP2039609?

[Bill Erickson (berick)]

Hah I just saw Terran's comment! Related, there are several commits which have no sign offs.

Other testing notes:

* I could not get Brief Record templates to do anything (local test VM and butternut). Maybe I'm expecting the wrong thing.

* Applying a brief record template as a default saves the value, but does not load the applied template on page reload.

* Invoice comboboxes show repeated options (e.g. "Paper" shows twice for if it's set on page load).

* Needs "ng lint --fix"

* For consistency, for blanket charges, the Billed and Paid amounts should be empty when creating a new invoice.

* Minor console error fix:

+++ b/Open-ILS/src/eg2/src/app/staff/acq/lineitem/brief-record.component.html
@@ -36,7 +36,7 @@
     <div class="flex-1">
       <button
         class="btn btn-secondary"
- [disabled]="selectedMARCTemplate.id == '__blank__'"
+ [disabled]="selectedMARCTemplate && selectedMARCTemplate.id == '__blank__'"
         (click)="setWSDefaultTemplate()" i18n>Set Default</button>
     </div>
   </div>

[Terran McCanna (tmccanna)]

Adding back needswork tag as per comments 7-9

[Andrea Neiman (aneiman)]

Thanks for the feedback - we will be addressing this & post an updated branch shortly.

[Mike Rylander (mrylander)]

looking at comment #9 right now...

Thanks, Bill!

 * For the first two regarding templates, you won't see a change in the interface when you select a template. That changes which MARCXML record is used to construct the eventual bib. Mostly, that means the leader, 007, and 008 are different once you hit save.

 * Jumping to the end of the list, I'll look at that console error ... it looks like an angular version thing? Regardless, I will take that change or do the moral equiv.

 * Re invoice comboboxes, unless I'm misunderstanding, that seems like just how comboboxes work -- show what's selected, then show the whole list.

More on the rest later.

[Mike Rylander (mrylander)]

Back up to comment #4 ...

Blake, thanks. Permissions are always shifting, so it will often be up to the testing admin to adjust as needed based on what branch they decide to put the commits on top of.

My assumption has always been that advanced-dev-testing folks can handle dealing with those sorts of conflicts, and we can just clean up the ids at commit time. That is what I do for others' permission-introducing branches, at least, when I test and commit them.

I will not be working on the collab branch, because a rebase (pending) and other fixing commits will cause the content of commits to shift around, and I want to deal with just one shifting target right now.

[Mike Rylander (mrylander)]

Oh, and, regarding the infrastructure commits remaining separate and lacking the LP number, that was intentional. They're not part of the feature code per se, and git will (well, can) show you that they're not in main. I left them separate and /not/ feature-LP tagged for ease of review by folks that know the core widgets, to look for logic changes that could impact other interfaces.

I personally find it MUCH easier to review shared-component changes separately this way -- no need to hunt through much bigger feature-focused commits to review infrastructure additions -- and the final merge committer can polish/squash/re-message them if necessary.

[Mike Rylander (mrylander)]

I've addressed the combobox not actually selecting the saved default. Combobox claims to be able to set its selected entry several different ways, but the only mechanism that works (at least here) is to use ViewChild to forcibly call the selectedId input in the surrounding component's code.

I've also loosened the rules on saving a default. Specifically, you can now save "Blank Record" as your default, as otherwise you would have to go to the workstation admin screen and delete the backing setting in order to remove a template-based saved default.

Regarding the blanket charges point, the consensus here after testing is that the behavior as currently implemented is correct. Maybe we're misunderstanding, though. Could you (Bill) or others can give us a more detailed description of the problem you see?

From our testing, the ONLY time those are prefilled is when there is a direct charge on the PO, and you use the PO → Create Invoice From PO Prices function, then it prefills with the estimated amount from the PO. IOW, this is intentional and desired behavior because the user chose "create from PO" and the PO included direct charges.

Finally, we'll ng-lint at the end...

Update to the branch with brief-record fixes coming soon!

[Mike Rylander (mrylander)]

Branch updated:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-2006970-2039609-acq-sprints-B-and-A