From 6225be879b4fd3ba70fddd1f853ed8d04665ca8c Mon Sep 17 00:00:00 2001 From: Jane Sandberg Date: Tue, 13 Sep 2022 20:21:12 -0700 Subject: [PATCH] LP1913604: Course materials module should remember original circ_lib Signed-off-by: Jane Sandberg Signed-off-by: Michele Morgan --- Open-ILS/examples/fm_IDL.xml | 2 + .../course-associate-material.component.html | 6 ++ .../course-associate-material.component.spec.ts | 113 ++++++++++++++++++++ .../course-associate-material.component.ts | 87 ++++++++++------ .../src/eg2/src/app/staff/share/course.service.ts | 17 +-- .../src/eg2/src/app/staff/share/course.spec.ts | 116 +++++++++++++++++++++ .../perlmods/lib/OpenILS/Application/Courses.pm | 3 + Open-ILS/src/perlmods/live_t/31-courses.t | 10 +- Open-ILS/src/sql/Pg/040.schema.asset.sql | 1 + .../xxxx.schema.course_original_circ_lib.sql | 8 ++ 10 files changed, 319 insertions(+), 44 deletions(-) create mode 100644 Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.spec.ts create mode 100644 Open-ILS/src/eg2/src/app/staff/share/course.spec.ts create mode 100644 Open-ILS/src/sql/Pg/upgrade/xxxx.schema.course_original_circ_lib.sql diff --git a/Open-ILS/examples/fm_IDL.xml b/Open-ILS/examples/fm_IDL.xml index aa658bac87..53113ab18a 100644 --- a/Open-ILS/examples/fm_IDL.xml +++ b/Open-ILS/examples/fm_IDL.xml @@ -3307,6 +3307,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + @@ -3316,6 +3317,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + diff --git a/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.html b/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.html index f271c1eee9..afb4f57b64 100644 --- a/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.html +++ b/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.html @@ -7,6 +7,12 @@ + + + { + let component: CourseAssociateMaterialComponent; + + const mockLibrary = { + id: () => 5, + shortname: () => 'greatLibrary' + }; + + const mockLibrary2 = { + id: () => 22 + }; + + const mockItem = { + a: [], + classname: 'acp', + _isfieldmapper: true, + id: () => {}, + circ_lib: () => mockLibrary + }; + + const mockCourse = { + a: [], + classname: 'acmc', + _isfieldmapper: true, + owning_lib: () => mockLibrary2 + }; + + const authServiceSpy = jasmine.createSpyObj(['token']); + const courseServiceSpy = jasmine.createSpyObj(['associateMaterials']); + courseServiceSpy.associateMaterials.and.returnValue({item: mockItem, material: new Promise(() => {})}); + const netServiceSpy = jasmine.createSpyObj(['request']); + const pcrudServiceSpy = jasmine.createSpyObj(['retrieveAll', 'search', 'update']); + pcrudServiceSpy.search.and.returnValue(of(mockItem)); + const toastServiceSpy = jasmine.createSpyObj(['success']); + const permServiceSpy = jasmine.createSpyObj(['hasWorkPermAt']); + permServiceSpy.hasWorkPermAt.and.returnValue(new Promise((resolve) => resolve({UPDATE_COPY: [5, 22]}))); + const modalSpy = jasmine.createSpyObj(['open']); + const dialogComponentSpy = jasmine.createSpyObj(['open']); + dialogComponentSpy.open.and.returnValue(of(true)); + const rejectedDialogComponentSpy = jasmine.createSpyObj(['open']); + rejectedDialogComponentSpy.open.and.returnValue(of(false)); + + beforeEach(() => { + component = new CourseAssociateMaterialComponent(authServiceSpy, courseServiceSpy, + netServiceSpy, pcrudServiceSpy, + toastServiceSpy, permServiceSpy, modalSpy); + component.confirmOtherLibraryDialog = dialogComponentSpy; + component.currentCourse = mockCourse; + }); + + describe('#associateItem method', () => { + afterEach(() => { + courseServiceSpy.associateMaterials.calls.reset(); + }); + + describe('item circ_lib is different from course owning lib', () => { + it("attempts to change item circ_lib to the course's library", waitForAsync(() => { + const paramsWithCircLib = { + barcode: '123', + relationship: 'required reading', + isModifyingLibrary: true, + tempLibrary: 22, // the Library that owns the course, rather than the item's circ_lib + currentCourse: mockCourse, + isModifyingCallNumber: undefined, isModifyingCircMod: undefined, isModifyingLocation: undefined, isModifyingStatus: undefined, + tempCircMod: undefined, tempLocation: undefined, tempStatus: undefined + }; + component.associateItem('123', 'required reading'); + + setTimeout(() => { // wait for the subscribe() to do its work + expect(courseServiceSpy.associateMaterials).toHaveBeenCalledWith(mockItem, paramsWithCircLib); + }, 500); + })); + + it('asks the user to confirm', (waitForAsync(() => { + component.associateItem('123', 'required reading'); + setTimeout(() => { // wait for the subscribe() to do its work + expect(dialogComponentSpy.open).toHaveBeenCalled(); + }, 500); + }))); + + it("sets the owning library's shortname in the UI", (waitForAsync(() => { + component.associateItem('123', 'required reading'); + setTimeout(() => { // wait for the subscribe() to do its work + expect(component.itemCircLib).toBe('greatLibrary'); + }, 500); + }))); + + it('does not proceed if the user says "no" in the different library confirmation dialog', waitForAsync(() => { + component.confirmOtherLibraryDialog = rejectedDialogComponentSpy; + component.associateItem('123', 'required reading'); + + setTimeout(() => { // wait for the subscribe() to do its work + expect(rejectedDialogComponentSpy.open).toHaveBeenCalled(); + expect(courseServiceSpy.associateMaterials).not.toHaveBeenCalled(); + }, 500); + })); + + }); + }); +}); diff --git a/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.ts b/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.ts index 590360d8b7..cafc739799 100644 --- a/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.ts +++ b/Open-ILS/src/eg2/src/app/staff/admin/local/course-reserves/course-associate-material.component.ts @@ -1,6 +1,7 @@ +import { PermService } from '@eg/core/perm.service'; import {Component, Input, ViewChild, OnInit} from '@angular/core'; -import {Observable, merge, of, EMPTY} from 'rxjs'; -import {switchMap} from 'rxjs/operators'; +import { Observable, merge, of, EMPTY, throwError, from } from 'rxjs'; +import { switchMap, concatMap } from 'rxjs/operators'; import {DialogComponent} from '@eg/share/dialog/dialog.component'; import {AuthService} from '@eg/core/auth.service'; import {NetService} from '@eg/core/net.service'; @@ -43,6 +44,7 @@ export class CourseAssociateMaterialComponent extends DialogComponent implements @ViewChild('materialAddDifferentLibraryString', { static: true }) materialAddDifferentLibraryString: StringComponent; @ViewChild('confirmOtherLibraryDialog') confirmOtherLibraryDialog: DialogComponent; + @ViewChild('otherLibraryNoPermissionsAlert') otherLibraryNoPermissionsAlert: DialogComponent; materialsDataSource: GridDataSource; @Input() barcodeInput: string; @Input() relationshipInput: string; @@ -54,6 +56,7 @@ export class CourseAssociateMaterialComponent extends DialogComponent implements @Input() isModifyingCircMod: boolean; @Input() isModifyingCallNumber: boolean; @Input() isModifyingLocation: boolean; + isModifyingLibrary: boolean; bibId: number; itemCircLib: string; @@ -66,6 +69,7 @@ export class CourseAssociateMaterialComponent extends DialogComponent implements private net: NetService, private pcrud: PcrudService, private toast: ToastService, + private perm: PermService, private modal: NgbModal ) { super(modal); @@ -156,7 +160,16 @@ export class CourseAssociateMaterialComponent extends DialogComponent implements } associateItem(barcode, relationship) { - if (barcode) { + if (!barcode || barcode.length === 0) { return; } + this.barcodeInput = null; + + this.pcrud.search('acp', {barcode: barcode.trim()}, { + flesh: 3, flesh_fields: {acp: ['call_number', 'circ_lib']} + }).pipe(switchMap(item => { + this.isModifyingLibrary = item.circ_lib().id() !== this.currentCourse.owning_lib().id(); + return this.isModifyingLibrary ? this.handleItemAtDifferentLibrary$(item) : of(item); + })) + .subscribe((originalItem) => { const args = { barcode: barcode.trim(), relationship: relationship, @@ -164,35 +177,32 @@ export class CourseAssociateMaterialComponent extends DialogComponent implements isModifyingCircMod: this.isModifyingCircMod, isModifyingLocation: this.isModifyingLocation, isModifyingStatus: this.isModifyingStatus, + isModifyingLibrary: this.isModifyingLibrary, tempCircMod: this.tempCircMod, tempLocation: this.tempLocation, + tempLibrary: this.currentCourse.owning_lib().id(), tempStatus: this.tempStatus, currentCourse: this.currentCourse }; - this.barcodeInput = null; - - this.pcrud.search('acp', {barcode: args.barcode}, { - flesh: 3, flesh_fields: {acp: ['call_number', 'circ_lib']} - }).pipe(switchMap(item => this.handleItemAtDifferentLibrary$(item))) - .subscribe((originalItem) => { - const associatedMaterial = this.course.associateMaterials(originalItem, args); - associatedMaterial.material.then(res => { - const item = associatedMaterial.item; - let new_cn = item.call_number().label(); - if (this.tempCallNumber) { new_cn = this.tempCallNumber; } - this.course.updateItem(item, this.currentCourse.owning_lib(), - new_cn, args.isModifyingCallNumber - ).then(resp => { - this.materialsGrid.reload(); - this.materialAddSuccessString.current() - .then(str => this.toast.success(str)); - }); - }, err => { - this.materialAddFailedString.current() - .then(str => this.toast.danger(str)); + + const associatedMaterial = this.course.associateMaterials(originalItem, args); + + associatedMaterial.material.then(res => { + const item = associatedMaterial.item; + let new_cn = item.call_number().label(); + if (this.tempCallNumber) { new_cn = this.tempCallNumber; } + this.course.updateItem(item, this.currentCourse.owning_lib(), + new_cn, args.isModifyingCallNumber + ).then(resp => { + this.materialsGrid.reload(); + this.materialAddSuccessString.current() + .then(str => this.toast.success(str)); }); + }, err => { + this.materialAddFailedString.current() + .then(str => this.toast.danger(str)); }); - } + }); } deleteSelectedMaterials(items) { @@ -210,15 +220,24 @@ export class CourseAssociateMaterialComponent extends DialogComponent implements }); } - handleItemAtDifferentLibrary$(item: IdlObject): Observable { + private handleItemAtDifferentLibrary$(item: IdlObject): Observable { this.itemCircLib = item.circ_lib().shortname(); - if (item.circ_lib().id() !== this.currentCourse.owning_lib().id()) { - return this.confirmOtherLibraryDialog.open() - .pipe(switchMap(confirmed => { - if (!confirmed) { return EMPTY; } - return of(item); - })); - } - return of(item); + const promise = this.perm.hasWorkPermAt(['UPDATE_COPY'], true).then(result => { + return result.UPDATE_COPY as number[]; + }); + return from(promise).pipe(concatMap((editableItemLibs) => { + if (editableItemLibs.indexOf(item.circ_lib().id()) !== -1) { + return this.confirmOtherLibraryDialog.open() + .pipe(switchMap(confirmed => { + // If the user clicked "no", return an empty observable, + // so the subsequent code has nothing to do. + if (!confirmed) { return EMPTY; } + return of(item); + })); + } else { + return this.otherLibraryNoPermissionsAlert.open() + .pipe(switchMap(() => EMPTY)); + } + })); } } diff --git a/Open-ILS/src/eg2/src/app/staff/share/course.service.ts b/Open-ILS/src/eg2/src/app/staff/share/course.service.ts index 3baa871c50..12a030382d 100644 --- a/Open-ILS/src/eg2/src/app/staff/share/course.service.ts +++ b/Open-ILS/src/eg2/src/app/staff/share/course.service.ts @@ -1,5 +1,5 @@ -import {Observable, throwError} from 'rxjs'; -import {switchMap, tap} from 'rxjs/operators'; +import { Observable, merge, throwError } from 'rxjs'; +import { tap, switchMap } from 'rxjs/operators'; import {Injectable} from '@angular/core'; import {AuthService} from '@eg/core/auth.service'; import {EventService} from '@eg/core/event.service'; @@ -140,6 +140,10 @@ export class CourseService { if (args.isModifyingCallNumber) { material.original_callnumber(item.call_number()); } + if (args.isModifyingLibrary && args.tempLibrary && this.org.canHaveVolumes(args.tempLibrary)) { + material.original_circ_lib(item.circ_lib()); + item.circ_lib(args.tempLibrary); + } const response = { item: item, material: this.pcrud.create(material).toPromise() @@ -234,10 +238,11 @@ export class CourseService { ).pipe(switchMap(res => { const event = this.evt.parse(res); if (event) { return throwError(event); } - return this.net.request( - 'open-ils.cat', 'open-ils.cat.transfer_copies_to_volume', - this.auth.token(), res.acn_id, [item.id()] - ); + // Not using open-ils.cat.transfer_copies_to_volume, + // because we don't necessarily want acp.circ_lib and + // acn.owning_lib to match in this scenario + item.call_number(res.acn_id) + return this.pcrud.update(item); })); return updatingVolume ? itemObservable.pipe(switchMap(() => callNumberObservable)).toPromise() : diff --git a/Open-ILS/src/eg2/src/app/staff/share/course.spec.ts b/Open-ILS/src/eg2/src/app/staff/share/course.spec.ts new file mode 100644 index 0000000000..049ada3faa --- /dev/null +++ b/Open-ILS/src/eg2/src/app/staff/share/course.spec.ts @@ -0,0 +1,116 @@ +import { waitForAsync } from '@angular/core/testing'; +import { CourseService } from './course.service'; +import { AuthService } from '@eg/core/auth.service'; +import { EventService } from '@eg/core/event.service'; +import { IdlService, IdlObject } from '@eg/core/idl.service'; +import { NetService } from '@eg/core/net.service'; +import { OrgService } from '@eg/core/org.service'; +import { PcrudService } from '@eg/core/pcrud.service'; +import { of } from 'rxjs'; + +describe('CourseService', () => { + let service: CourseService; + let circLib = 5; + let originalCircLib: number; + + const mockCallNumber = { + label_class: () => 1, + prefix: () => 2, + suffix: () => null , + owning_lib: () => 5, + record: () => 123 + }; + + const materialSpy = jasmine.createSpyObj(['item', 'record', 'course', 'original_circ_lib']); + materialSpy.original_circ_lib.and.callFake((newValue?: number) => { + if (newValue) { + originalCircLib = newValue; + } else { + return originalCircLib; + } + }) + const itemSpy = jasmine.createSpyObj(['call_number', 'circ_lib', 'id']); + itemSpy.circ_lib.and.callFake((newValue?: number) => { // this will return 5 unless set otherwise + if (newValue) { + circLib = newValue; + } else { + return circLib; + } + }); + itemSpy.call_number.and.returnValue(mockCallNumber); + const authServiceSpy = jasmine.createSpyObj(['token']); + authServiceSpy.token.and.returnValue('myToken'); + const evtServiceSpy = jasmine.createSpyObj(['parse']); + const idlServiceSpy = jasmine.createSpyObj(['create']); + idlServiceSpy.create.and.returnValue(materialSpy); + const netServiceSpy = jasmine.createSpyObj(['request']); + netServiceSpy.request.and.returnValue(of()); + const orgServiceSpy = jasmine.createSpyObj(['settings', 'canHaveVolumes']); + const pcrudServiceSpy = jasmine.createSpyObj(['retrieveAll', 'search', 'update', 'create']); + pcrudServiceSpy.update.and.returnValue(of(1)); + pcrudServiceSpy.create.and.returnValue(of(materialSpy)); + + const mockOrg = { + a: [], + classname: 'aou', + _isfieldmapper: true, + id: () => 5 + }; + + const mockConsortium = { + id: () => 1 + }; + + const mockCourse = { + id: () => 20 + }; + + beforeEach(() => { + service = new CourseService(authServiceSpy, evtServiceSpy, + idlServiceSpy, netServiceSpy, + orgServiceSpy, pcrudServiceSpy); + orgServiceSpy.canHaveVolumes.and.returnValue(true); + circLib = 5; // set the item's circ lib to 5 + }); + + afterEach(() => { + pcrudServiceSpy.update.calls.reset(); + itemSpy.circ_lib.calls.reset(); + materialSpy.original_circ_lib.calls.reset(); + }) + + it('updateItem() passes the expected parameters to open-ils.cat', () => { + service.updateItem(itemSpy, mockOrg, 'ABC 123', true); + expect(netServiceSpy.request).toHaveBeenCalledWith( + 'open-ils.cat', 'open-ils.cat.call_number.find_or_create', + 'myToken', 'ABC 123', 123, 5, 2, null, 1 + ); + }); + + it('updateItem() calls pcrud only once when modifying call number', () => { + service.updateItem(itemSpy, mockOrg, 'ABC 123', true); + expect(pcrudServiceSpy.update).toHaveBeenCalledTimes(1); + }); + + it('updateItem() calls pcrud only once when not modifying call number', () => { + service.updateItem(itemSpy, mockOrg, 'ABC 123', false); + expect(pcrudServiceSpy.update).toHaveBeenCalledTimes(1); + }); + + it('#associateMaterials can temporarily change the item circ_lib', waitForAsync(() => { + const results = service.associateMaterials(itemSpy, {tempLibrary: 4, isModifyingLibrary: true, currentCourse: mockCourse}); + expect(results.item.circ_lib()).toBe(4); + results.material.then((material) => { + expect(material.original_circ_lib()).toBe(5); + }); + })); + + it("#associateMaterials does not change the item circ_lib if the requested lib can't have items", () => { + orgServiceSpy.canHaveVolumes.and.returnValue(false); + const results = service.associateMaterials(itemSpy, {tempLibrary: 1, isModifyingLibrary: true, currentCourse: mockCourse}); + expect(itemSpy.circ_lib).not.toHaveBeenCalled(); + expect(results.item.circ_lib()).toBe(5); + expect(materialSpy.original_circ_lib).not.toHaveBeenCalled(); + }); + +}); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Courses.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Courses.pm index af55705065..ac6d874b29 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Courses.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Courses.pm @@ -265,6 +265,9 @@ sub _resetItemFields { if ($acmcm->original_location) { $acp->location($acmcm->original_location); } + if ($acmcm->original_circ_lib) { + $acp->circ_lib($acmcm->original_circ_lib); + } $e->update_asset_copy($acp); if ($acmcm->original_callnumber) { my $existing_acn = $e->retrieve_asset_call_number($acp->call_number); diff --git a/Open-ILS/src/perlmods/live_t/31-courses.t b/Open-ILS/src/perlmods/live_t/31-courses.t index eafee305bf..af9884ea3e 100644 --- a/Open-ILS/src/perlmods/live_t/31-courses.t +++ b/Open-ILS/src/perlmods/live_t/31-courses.t @@ -1,7 +1,7 @@ #!perl use strict; use warnings; -use Test::More tests => 7; +use Test::More tests => 8; use OpenILS::Utils::TestUtils; use OpenILS::Utils::CStoreEditor qw/:funcs/; use OpenILS::Application::AppUtils; @@ -84,7 +84,7 @@ is(scalar(@$results), 0, 'Successfully deleted bre'); # 3. Let's attach an existing item record entry to course #1, then detach it # -------------------------------------------------------------------------- -# Create an item with temporary location, so that we can confirm its fields revert on course material detach +# Create an item with temporary location and library, so that we can confirm its fields revert on course material detach my $acp = Fieldmapper::asset::copy->new; my $item_id = int (rand (1_000_000) ); my $acmcm_id = int (rand (1_000_000) ); @@ -92,7 +92,7 @@ $acp->deleted(0); $acp->call_number(1); $acp->creator(1); $acp->editor(1); -$acp->circ_lib(5); +$acp->circ_lib(6); # temporary value $acp->age_protect(1); $acp->barcode( $bre->id . '-1' ); $acp->create_date('now'); @@ -122,6 +122,7 @@ $acmcm->record(55); $acmcm->item($item_id); $acmcm->original_status(0); $acmcm->original_location(1); +$acmcm->original_circ_lib(5); $acmcm->temporary_record(0); $e->xact_begin; $e->create_asset_course_module_course_materials( $acmcm ); # associated this bib record with a course @@ -134,4 +135,5 @@ is(scalar(@$results), 0, 'Successfully deleted acmcm'); # Re-load the acp into memory from the db $acp = $e->retrieve_asset_copy($item_id); -is($acp->location, 1, "Successfully reverted item's shelving location"); \ No newline at end of file +is($acp->location, 1, "Successfully reverted item's shelving location"); +is($acp->circ_lib, 5, "Successfully reverted item's circ_lib"); \ No newline at end of file diff --git a/Open-ILS/src/sql/Pg/040.schema.asset.sql b/Open-ILS/src/sql/Pg/040.schema.asset.sql index b62dc761e4..0c3866b99b 100644 --- a/Open-ILS/src/sql/Pg/040.schema.asset.sql +++ b/Open-ILS/src/sql/Pg/040.schema.asset.sql @@ -1142,6 +1142,7 @@ CREATE TABLE asset.course_module_course_materials ( original_status INT REFERENCES config.copy_status, original_circ_modifier TEXT, --REFERENCES config.circ_modifier original_callnumber INT REFERENCES asset.call_number, + original_circ_lib INT REFERENCES actor.org_unit (id), unique (course, item, record) ); diff --git a/Open-ILS/src/sql/Pg/upgrade/xxxx.schema.course_original_circ_lib.sql b/Open-ILS/src/sql/Pg/upgrade/xxxx.schema.course_original_circ_lib.sql new file mode 100644 index 0000000000..c9e0473352 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/xxxx.schema.course_original_circ_lib.sql @@ -0,0 +1,8 @@ +BEGIN; + +-- SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version); + +ALTER TABLE asset.course_module_course_materials + ADD COLUMN original_circ_lib INT REFERENCES actor.org_unit (id); + +COMMIT; \ No newline at end of file -- 2.11.0