LP#2098043,2098407,2098117: Angular Bucket updates

This commit builds on several improvements to the open-ils.pcrud
service, allowing for improved performance and removal of some layers of
indirection that additionally provide the opportunity for permission and
correctness bugs.

General changes:

 * Add "count_only" support directly to the pcrud services, a la idlist.
 * Improve open-ils.fielder map generation by only causing joins on
   fields that are used for sorting or filtering; displayed fields come
   from fleshing.
 * Use pcrud to add and remove bucket items via the bucket service

Bucket UI improvements:

 * Use pcrud for all grids on all tabs, rather than open-ils.actor
   methods, leveraging the new permission testing functionality.
 * Deserialize count fetching, and use pcrud
 * Remove unnecessary grid and count refreshing
 * Add user-applied grid filters to the datasource's query for fetching
   buckets to display.  These filters are /not/ included in the count
   call that populates the tab label, that is always the unfiltered
   count.

The definitions for "Shared with {me|others}" and "Visible to me" have changed
such that:

 * Shared with me - all NON-public buckets that I can see. That is, if I
   don't own the bucket, I have one of CREATE_BIB_BUCKET, ADMIN_BIB_BUCKET,
   VIEW_CONTAINER, or UPDATE_CONTAINER permissions at the bucket owning
   lib or one of the share_maps orgs, or have VIEW_CONTAINER or
   UPDATE_CONTAINER object permissions on a bucket assigned by the
   owner.
 * Shared with others - all buckets that I own that have entries in the
   share map table (org sharing) or that have entries in the object
   permission mapping table (user permission grant).
 * Visible to me - All PUBLIC buckets that I don't own.

We actively exclude "temp" btype buckets and add an index to support
that exclusion.  These buckets are only used for transient purposes and
will not accumulate on a normal, production instance.  However, if
(for instance) A/T event definitions used for printing and emailing
records are disabled, the buckets are not cleaned up.  This can happen
on a test instance where most A/T events are disabled to avoid sending
test-data emails to "real" email addresses.  In such cases, there may be
many orders of magnitude more "temp" buckets than actual user-created
ones.

Signed-off-by: Mike Rylander <[email protected]>
Signed-off-by: Galen Charlton <[email protected]>

Author: spaceisntsyntax

Open-ILS/examples/fm_IDL.xml

@@ -9050,19 +9050,19 @@ SELECT  usr,
 		</links>
 		<permacrud xmlns="http://open-ils.org/spec/opensrf/IDL/permacrud/v1">
 			<actions>
-				<retrieve permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET VIEW_CONTAINER">
+				<create permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET CREATE_CONTAINER_ITEM UPDATE_CONTAINER">
 					<context delegate="bucket" field="owning_lib" owning_user="owner"/>
 					<context delegate="bucket" jump="share_maps" field="share_org"/>
-				</retrieve>
-				<create permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET UPDATE_CONTAINER">
+				</create>
+				<retrieve permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET VIEW_CONTAINER UPDATE_CONTAINER">
 					<context delegate="bucket" field="owning_lib" owning_user="owner"/>
 					<context delegate="bucket" jump="share_maps" field="share_org"/>
-				</create>
+				</retrieve>
 				<update permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET UPDATE_CONTAINER">
 					<context delegate="bucket" field="owning_lib" owning_user="owner"/>
 					<context delegate="bucket" jump="share_maps" field="share_org"/>
 				</update>
-				<delete permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET UPDATE_CONTAINER">
+				<delete permission="CREATE_BIB_BUCKET ADMIN_BIB_BUCKET DELETE_CONTAINER_ITEM UPDATE_CONTAINER">
 					<context delegate="bucket" field="owning_lib" owning_user="owner"/>
 					<context delegate="bucket" jump="share_maps" field="share_org"/>
 				</delete>

Open-ILS/src/eg2/src/app/core/pcrud.service.ts

@@ -14,6 +14,7 @@ interface PcrudReqOps {
     authoritative?: boolean;
     anonymous?: boolean;
     idlist?: boolean;
+    count_only?: boolean;
     atomic?: boolean;
     // If true, link-type fields which link to a class that defines a
     // selector will be fleshed with the linked value.  This affects
@@ -165,7 +166,12 @@ export class PcrudContext {
         reqOps = reqOps || {};
         this.authoritative = reqOps.authoritative || false;
 
-        const returnType = reqOps.idlist ? 'id_list' : 'search';
+        let returnType = reqOps.idlist ? 'id_list' : 'search';
+        if (reqOps.count_only) {
+            returnType = 'count';
+            reqOps.atomic = reqOps.fleshSelectors = false;
+        }
+
         let method = `open-ils.pcrud.${returnType}.${fmClass}`;
 
         if (reqOps.atomic) { method += '.atomic'; }

Open-ILS/src/eg2/src/app/share/grid/grid-flat-data.service.ts

@@ -6,7 +6,12 @@ import {GridContext, GridColumnSort} from './grid';
 import {Pager} from '@eg/share/util/pager';
 
 interface FlatQueryFields {
-    [name: string]: string;
+    [name:string] : {
+        path: string,
+        sort?: boolean,
+        filter?: boolean,
+        display?: boolean
+    };
 }
 
 
@@ -27,12 +32,20 @@ export class GridFlatDataService {
         }
 
         const fields = this.compileFields(gridContext);
+
         const flatSort = sort.map(s => {
             const obj: any = {};
             obj[s.name] = s.dir;
+            fields[s.name].sort = true;
             return obj;
         });
 
+        if (query && query['-and']) {
+            query['-and'].forEach(col => {
+                Object.keys(col).forEach(k => fields[k].filter = true)
+            });
+        }
+
         return this.net.request(
             'open-ils.fielder',
             'open-ils.fielder.flattened_search',
@@ -52,7 +65,14 @@ export class GridFlatDataService {
             // Verify the column describes a proper IDL field
             const path = col.path || col.name;
             const info = gridContext.columnSet.idlInfoFromDotpath(path);
-            if (info) { fields[col.name] = path; }
+            if (info) {
+                fields[col.name] = {
+                    path   : path,
+                    display: true,
+                    sort   : false,
+                    filter : false
+                };
+            }
         });
 
         return fields;

Open-ILS/src/eg2/src/app/staff/share/buckets/bucket.service.ts

@@ -67,28 +67,13 @@ export class BucketService {
             item.target_biblio_record_entry(itemId);
             items.push(item);
         });
-        const requestObs = this.net.request(
-            'open-ils.actor',
-            'open-ils.actor.container.item.create',
-            this.auth.token(),
-            'biblio',
-            items
-        );
-
-        return lastValueFrom(requestObs);
+        return this.pcrud.create(items).toPromise().then(l => l.map(i => i.id()));
     }
 
     async removeBibsFromRecordBucket(bucketId: number, bibIds: number[]): Promise<any> {
-        const requestObs = this.net.request(
-            'open-ils.actor',
-            'open-ils.actor.container.item.delete.batch',
-            this.auth.token(),
-            'biblio_record_entry',
-            bucketId,
-            bibIds
-        );
-
-        return lastValueFrom(requestObs);
+        return this.pcrud.search(
+            'cbrebi', { bucket: bucketId, target_biblio_record_entry: bibIds }, {}, {atomic: true}
+        ).toPromise().then(entries => this.pcrud.remove(entries).toPromise());
     }
 
     async retrieveRecordBuckets(bucketIds: number[]): Promise<any[]> {

Open-ILS/src/eg2/src/app/staff/share/buckets/record-bucket.component.ts

@@ -132,12 +132,15 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
     }
 
     search_or_count(justCount, hint, query, pcrudOps, pcrudReqOps): Observable<number | any[]>  {
-        if (justCount) {
-            return this.net.request('open-ils.actor', 'open-ils.actor.count_with_pcrud.authoritative',
-                this.auth.token(), hint, query);
-        } else {
-            return this.pcrud.search(hint, query, pcrudOps, pcrudReqOps);
+        if (!justCount) {
+            let query_filters = this.extractGridFitlers();
+
+            if (query_filters.length > 0) {
+                query['-and'] = query_filters;
+            }
         }
+
+        return this.pcrud.search(hint, {...query, btype: { '!=': 'temp' }}, pcrudOps, {...pcrudReqOps, count_only : justCount});
     }
 
     initViews() {
@@ -152,7 +155,7 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
                     let result: BucketQueryResult;
                     const response  = await lastValueFrom(
                         this.search_or_count(justCount, 'cbreb',
-                            { id: { '!=' : null } },
+                            { owner: { '!=' : this.userId || this.auth.user().id() }, pub: 't' },
                             {
                                 ...(pager?.limit && { limit: pager.limit }),
                                 ...(pager?.offset && { offset: pager.offset }),
@@ -288,11 +291,21 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
                     console.debug('translatedSort', translatedSort);
                     let result: BucketQueryResult;
                     const response  = await lastValueFrom(
-                        justCount
-                            ? this.net.request( 'open-ils.actor',
-                                'open-ils.actor.container.retrieve_biblio_record_entry_buckets_shared_with_others.count', this.auth.token())
-                            : this.net.request( 'open-ils.actor',
-                                'open-ils.actor.container.retrieve_biblio_record_entry_buckets_shared_with_others', this.auth.token())
+                        this.search_or_count(justCount, 'cbreb',
+                            { owner: { '=' : this.userId || this.auth.user().id() }, '-or': [
+                                {'-exists':{from:'cbrebs',where:{bucket:{'=':{'+cbreb':'id'}}}}},
+                                {'-exists':{from:'puopm',where:{
+                                    object_type:"cbreb",
+                                    "+cbreb":{id:{"=":{transform:"text",value:{"+puopm":"object_id"}}}}}}
+                                }
+                            ]},
+                            {
+                                ...(pager?.limit && { limit: pager.limit }),
+                                ...(pager?.offset && { offset: pager.offset }),
+                                ...(translatedSort && translatedSort),
+                            },
+                            { idlist: true, atomic: true }
+                        )
                     );
                     if (justCount) {
                         result = { bucketIds: [], count: response as number };
@@ -314,11 +327,15 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
                     console.debug('translatedSort', translatedSort);
                     let result: BucketQueryResult;
                     const response  = await lastValueFrom(
-                        justCount
-                            ? this.net.request('open-ils.actor',
-                                'open-ils.actor.container.retrieve_biblio_record_entry_buckets_shared_with_user.count', this.auth.token())
-                            : this.net.request('open-ils.actor',
-                                'open-ils.actor.container.retrieve_biblio_record_entry_buckets_shared_with_user', this.auth.token())
+                        this.search_or_count(justCount, 'cbreb',
+                            { owner: { '!=' : this.userId || this.auth.user().id() }, pub: 'f' },
+                            {
+                                ...(pager?.limit && { limit: pager.limit }),
+                                ...(pager?.offset && { offset: pager.offset }),
+                                ...(translatedSort && translatedSort),
+                            },
+                            { idlist: true, atomic: true }
+                        )
                     );
                     if (justCount) {
                         result = { bucketIds: [], count: response as number };
@@ -353,15 +370,17 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
 
         const viewKeys = this.getViewKeys();
 
-        try {
-            viewKeys.forEach( v => { this.views[v].count = -1; } );
-            // Wait for all bucketIdQuery operations for counting to complete
-            await Promise.all(viewKeys.map(v => this.views[v].bucketIdQuery(null, [], true)));
-        } catch (error) {
-            console.error('Error updating counts:', error);
-        }
-        this.countInProgress = false;
-        console.warn('countInProgress = false');
+        viewKeys.forEach( v => { this.views[v].count = -1; } );
+
+        // Wait for all bucketIdQuery operations for counting to complete
+        Promise.all(
+            viewKeys.map(v => this.views[v].bucketIdQuery(null, [], true))
+        ).catch(
+            error => console.error('Error updating counts:', error)
+        ).finally( () => {
+            this.countInProgress = false;
+            console.warn('countInProgress = false');
+        });
     }
 
     getViewKeys(): string[] {
@@ -458,12 +477,16 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
         console.debug('switchTo', source);
         this.currentView = source;
         this.router.navigate([this.mapDatasourceToUrl(source)], { relativeTo: this.route.parent });
-        if (!this.initInProgress) {
-            this.grid.reload();
-            this.updateCounts();
-        }
+        // router navigation causes a grid refresh
     }
 
+    extractGridFitlers() {
+        let query_filters = [];
+        Object.keys(this.dataSource.filters).forEach(key => {
+            query_filters = query_filters.concat(this.dataSource.filters[key]);
+        });
+        return query_filters;
+    }
 
     buildRetrieveByIdsQuery(bucketIds: number[]) {
         console.debug('buildRetrieveByIdsQuery, bucketIds', bucketIds);
@@ -473,10 +496,7 @@ export class RecordBucketComponent implements OnInit, OnDestroy {
         // query something even if no ids to avoid exception
         query['id'] = bucketIds.length === 0 ? [-1] : bucketIds.map( b => this.idl.pkeyValue(b) );
 
-        let query_filters = [];
-        Object.keys(this.dataSource.filters).forEach(key => {
-            query_filters = query_filters.concat(this.dataSource.filters[key]);
-        });
+        let query_filters = this.extractGridFitlers();
 
         if (query_filters.length > 0) {
             query['-and'] = query_filters;

Open-ILS/src/sql/Pg/070.schema.container.sql

@@ -164,6 +164,7 @@ CREATE TABLE container.biblio_record_entry_bucket (
 	create_time	TIMESTAMP WITH TIME ZONE	NOT NULL DEFAULT NOW(),
 	CONSTRAINT breb_name_once_per_owner UNIQUE (owner,name,btype)
 );
+CREATE INDEX cbreb_pub_owner_not_temp_idx ON container.biblio_record_entry_bucket (pub,owner) WHERE btype != 'temp';
 
 CREATE TABLE container.biblio_record_entry_bucket_shares (
     id          SERIAL      PRIMARY KEY,

Open-ILS/src/sql/Pg/upgrade/YYYY.index.faster-nontemp-buckets.sql

@@ -0,0 +1,5 @@
+BEGIN;
+
+CREATE INDEX IF NOT EXISTS cbreb_pub_owner_not_temp_idx ON container.biblio_record_entry_bucket (pub,owner) WHERE btype != 'temp';
+
+COMMIT;