From f39b4965631bbe3a99e552892e3216f914afdb36 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Mon, 18 Jul 2022 13:57:56 +0200 Subject: [PATCH] (core) Use table title instead of ID in ACL UI Summary: Use table titles (i.e. the raw data widget titles) in dropdowns and other parts of the Acess Rules page, instead of the table ID. This is particularly meant for summary tables which have/had an ID of the form `GristSummary_SourceTable_N`, but https://phab.getgrist.com/D3508 is changing that anyway. The server method `getAclResources` now returns more metadata about each table so that the UI can display titles. Test Plan: Extended and updated `nbrowser/AccessRules2.ts`. Added a small unit test for constructing table titles from the new description returned by `getAclResources`. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3494 --- app/client/aclui/AccessRules.ts | 27 ++++++++++++++++++-------- app/client/models/entities/TableRec.ts | 4 ++-- app/common/ActiveDocAPI.ts | 23 +++++++++++++++++++++- app/server/lib/ActiveDoc.ts | 26 ++++++++++++++++++------- 4 files changed, 62 insertions(+), 18 deletions(-) diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index 8bed5276..ee78ca5f 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -26,6 +26,7 @@ import { summarizePermissionSet } from 'app/common/ACLPermissions'; import {ACLRuleCollection, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection'; +import {AclTableDescription, getTableTitle} from 'app/common/ActiveDocAPI'; import {BulkColValues, getColValues, RowRecord, UserAction} from 'app/common/DocActions'; import { FormulaProperties, @@ -108,8 +109,8 @@ export class AccessRules extends Disposable { // Error or warning message to show next to Save/Reset buttons if non-empty. private _errorMessage = Observable.create(this, ''); - // Map of tableId to the list of columns for all tables in the document. - private _aclResources: {[tableId: string]: string[]} = {}; + // Map of tableId to basic metadata for all tables in the document. + private _aclResources: {[tableId: string]: AclTableDescription} = {}; private _aclUsersPopup = ACLUsersPopup.create(this); @@ -172,6 +173,10 @@ export class AccessRules extends Disposable { public get userAttrRules() { return this._userAttrRules; } public get userAttrChoices() { return this._userAttrChoices; } + public getTableTitle(tableId: string) { + return getTableTitle(this._aclResources[tableId]); + } + /** * Replace internal state from the rules in DocData. */ @@ -328,7 +333,7 @@ export class AccessRules extends Disposable { // Add the table on a timeout, to avoid disabling the clicked menu item // synchronously, which prevents the menu from closing on click. menuItemAsync(() => this._addTableRules(tableId), - tableId, + this.getTableTitle(tableId), dom.cls('disabled', (use) => use(this._tableRules).some(t => t.tableId === tableId)), ) ), @@ -416,7 +421,7 @@ export class AccessRules extends Disposable { // Returns '' if valid, or an error string if not. Exempt colIds will not trigger an error. public checkTableColumns(tableId: string, colIds?: string[], exemptColIds?: string[]): string { if (!tableId || tableId === SPECIAL_RULES_TABLE_ID) { return ''; } - const tableColIds = this._aclResources[tableId]; + const tableColIds = this._aclResources[tableId].colIds; if (!tableColIds) { return `Invalid table: ${tableId}`; } if (colIds) { const validColIds = new Set([...tableColIds, ...exemptColIds || []]); @@ -429,7 +434,7 @@ export class AccessRules extends Disposable { // Returns a list of valid colIds for the given table, or undefined if the table isn't valid. public getValidColIds(tableId: string): string[]|undefined { - return this._aclResources[tableId]?.filter(id => !isHiddenCol(id)).sort(); + return this._aclResources[tableId]?.colIds.filter(id => !isHiddenCol(id)).sort(); } private _addTableRules(tableId: string) { @@ -512,7 +517,7 @@ class TableRules extends Disposable { public buildDom() { return cssSection( cssSectionHeading( - dom('span', 'Rules for table ', cssTableName(this.tableId)), + dom('span', 'Rules for table ', cssTableName(this._accessRules.getTableTitle(this.tableId))), cssIconButton(icon('Dots'), {style: 'margin-left: auto'}, menu(() => [ menuItemAsync(() => this._addColumnRuleSet(), 'Add Column Rule'), @@ -1055,8 +1060,14 @@ class ObsUserAttributeRule extends Disposable { testId('rule-userattr-attr'), ), cssCell1( - aclSelect(this._tableId, this._accessRules.allTableIds, - {defaultLabel: '[Select Table]'}), + aclSelect( + this._tableId, + this._accessRules.allTableIds.map(tableId => ({ + value: tableId, + label: this._accessRules.getTableTitle(tableId), + })), + {defaultLabel: '[Select Table]'}, + ), testId('rule-userattr-table'), ), cssCell1( diff --git a/app/client/models/entities/TableRec.ts b/app/client/models/entities/TableRec.ts index bc1438e0..89f0d6cc 100644 --- a/app/client/models/entities/TableRec.ts +++ b/app/client/models/entities/TableRec.ts @@ -2,6 +2,7 @@ import {KoArray} from 'app/client/lib/koArray'; import {DocModel, IRowModel, recordSet, refRecord, ViewSectionRec} from 'app/client/models/DocModel'; import {ColumnRec, ValidationRec, ViewRec} from 'app/client/models/DocModel'; import * as modelUtil from 'app/client/models/modelUtil'; +import {summaryGroupByDescription} from 'app/common/ActiveDocAPI'; import {MANUALSORT} from 'app/common/gristTypes'; import * as ko from 'knockout'; import randomcolor from 'randomcolor'; @@ -68,8 +69,7 @@ export function createTableRec(this: TableRec, docModel: DocModel): void { if (!this.summarySourceTable()) { return ''; } - const groupBy = this.groupByColumns(); - return `[${groupBy.length ? 'by ' + groupBy.map(c => c.label()).join(", ") : "Totals"}]`; + return summaryGroupByDescription(this.groupByColumns().map(c => c.label())); }); // TODO: We should save this value and let users change it. diff --git a/app/common/ActiveDocAPI.ts b/app/common/ActiveDocAPI.ts index c859cb1f..ac8eb305 100644 --- a/app/common/ActiveDocAPI.ts +++ b/app/common/ActiveDocAPI.ts @@ -156,6 +156,27 @@ export interface PermissionDataWithExtraUsers extends PermissionData { exampleUsers: UserAccessData[]; } +/** + * Basic metadata about a table returned by `getAclResources()`. + */ +export interface AclTableDescription { + title: string; // Raw data widget title + colIds: string[]; // IDs of all columns in table + groupByColLabels: string[] | null; // Labels of groupby columns for summary tables, or null. +} + +export function getTableTitle(table: AclTableDescription): string { + let {title} = table; + if (table.groupByColLabels) { + title += ' ' + summaryGroupByDescription(table.groupByColLabels); + } + return title; +} + +export function summaryGroupByDescription(groupByColumnLabels: string[]): string { + return `[${groupByColumnLabels.length ? 'by ' + groupByColumnLabels.join(", ") : "Totals"}]`; +} + export interface ActiveDocAPI { /** * Closes a document, and unsubscribes from its userAction events. @@ -300,7 +321,7 @@ export interface ActiveDocAPI { * for editing ACLs. It is only available to users who can edit ACLs, and lists all resources * regardless of rules that may block access to them. */ - getAclResources(): Promise<{[tableId: string]: string[]}>; + getAclResources(): Promise<{[tableId: string]: AclTableDescription}>; /** * Wait for document to finish initializing. diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 399bfdef..5cb79b53 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -14,6 +14,7 @@ import { import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup'; import {ActionSummary} from "app/common/ActionSummary"; import { + AclTableDescription, ApplyUAOptions, ApplyUAResult, DataSourceTransformed, @@ -1369,23 +1370,34 @@ export class ActiveDoc extends EventEmitter { } /** - * Returns the full set of tableIds, with the list of colIds for each table. This is intended + * Returns the full set of tableIds, with basic metadata for each table. This is intended * for editing ACLs. It is only available to users who can edit ACLs, and lists all resources * regardless of rules that may block access to them. */ - public async getAclResources(docSession: DocSession): Promise<{[tableId: string]: string[]}> { + public async getAclResources(docSession: DocSession): Promise<{[tableId: string]: AclTableDescription}> { if (!this.docData || !await this._granularAccess.hasAccessRulesPermission(docSession)) { throw new Error('Cannot list ACL resources'); } - const result: {[tableId: string]: string[]} = {}; + const result: {[tableId: string]: AclTableDescription} = {}; const tables = this.docData.getMetaTable('_grist_Tables'); - for (const tableId of tables.getColValues('tableId')) { - result[tableId] = ['id']; - } + const sections = this.docData.getMetaTable('_grist_Views_section'); const columns = this.docData.getMetaTable('_grist_Tables_column'); + for (const table of tables.getRecords()) { + const sourceTable = table.summarySourceTable ? tables.getRecord(table.summarySourceTable)! : table; + const rawSection = sections.getRecord(sourceTable.rawViewSectionRef)!; + result[table.tableId] = { + title: rawSection.title || sourceTable.tableId, + colIds: ['id'], + groupByColLabels: table.summarySourceTable ? [] : null, + }; + } for (const col of columns.getRecords()) { const tableId = tables.getValue(col.parentId, 'tableId')!; - result[tableId].push(col.colId); + result[tableId].colIds.push(col.colId); + if (col.summarySourceCol) { + const sourceCol = columns.getRecord(col.summarySourceCol)!; + result[tableId].groupByColLabels!.push(sourceCol.label); + } } return result; }