(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
This commit is contained in:
Alex Hall 2022-07-18 13:57:56 +02:00
parent a0f405e45f
commit f39b496563
4 changed files with 62 additions and 18 deletions

View File

@ -26,6 +26,7 @@ import {
summarizePermissionSet summarizePermissionSet
} from 'app/common/ACLPermissions'; } from 'app/common/ACLPermissions';
import {ACLRuleCollection, SPECIAL_RULES_TABLE_ID} from 'app/common/ACLRuleCollection'; 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 {BulkColValues, getColValues, RowRecord, UserAction} from 'app/common/DocActions';
import { import {
FormulaProperties, FormulaProperties,
@ -108,8 +109,8 @@ export class AccessRules extends Disposable {
// Error or warning message to show next to Save/Reset buttons if non-empty. // Error or warning message to show next to Save/Reset buttons if non-empty.
private _errorMessage = Observable.create(this, ''); private _errorMessage = Observable.create(this, '');
// Map of tableId to the list of columns for all tables in the document. // Map of tableId to basic metadata for all tables in the document.
private _aclResources: {[tableId: string]: string[]} = {}; private _aclResources: {[tableId: string]: AclTableDescription} = {};
private _aclUsersPopup = ACLUsersPopup.create(this); private _aclUsersPopup = ACLUsersPopup.create(this);
@ -172,6 +173,10 @@ export class AccessRules extends Disposable {
public get userAttrRules() { return this._userAttrRules; } public get userAttrRules() { return this._userAttrRules; }
public get userAttrChoices() { return this._userAttrChoices; } public get userAttrChoices() { return this._userAttrChoices; }
public getTableTitle(tableId: string) {
return getTableTitle(this._aclResources[tableId]);
}
/** /**
* Replace internal state from the rules in DocData. * 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 // Add the table on a timeout, to avoid disabling the clicked menu item
// synchronously, which prevents the menu from closing on click. // synchronously, which prevents the menu from closing on click.
menuItemAsync(() => this._addTableRules(tableId), menuItemAsync(() => this._addTableRules(tableId),
tableId, this.getTableTitle(tableId),
dom.cls('disabled', (use) => use(this._tableRules).some(t => t.tableId === 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. // 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 { public checkTableColumns(tableId: string, colIds?: string[], exemptColIds?: string[]): string {
if (!tableId || tableId === SPECIAL_RULES_TABLE_ID) { return ''; } 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 (!tableColIds) { return `Invalid table: ${tableId}`; }
if (colIds) { if (colIds) {
const validColIds = new Set([...tableColIds, ...exemptColIds || []]); 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. // Returns a list of valid colIds for the given table, or undefined if the table isn't valid.
public getValidColIds(tableId: string): string[]|undefined { 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) { private _addTableRules(tableId: string) {
@ -512,7 +517,7 @@ class TableRules extends Disposable {
public buildDom() { public buildDom() {
return cssSection( return cssSection(
cssSectionHeading( 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'}, cssIconButton(icon('Dots'), {style: 'margin-left: auto'},
menu(() => [ menu(() => [
menuItemAsync(() => this._addColumnRuleSet(), 'Add Column Rule'), menuItemAsync(() => this._addColumnRuleSet(), 'Add Column Rule'),
@ -1055,8 +1060,14 @@ class ObsUserAttributeRule extends Disposable {
testId('rule-userattr-attr'), testId('rule-userattr-attr'),
), ),
cssCell1( cssCell1(
aclSelect(this._tableId, this._accessRules.allTableIds, aclSelect(
{defaultLabel: '[Select Table]'}), this._tableId,
this._accessRules.allTableIds.map(tableId => ({
value: tableId,
label: this._accessRules.getTableTitle(tableId),
})),
{defaultLabel: '[Select Table]'},
),
testId('rule-userattr-table'), testId('rule-userattr-table'),
), ),
cssCell1( cssCell1(

View File

@ -2,6 +2,7 @@ import {KoArray} from 'app/client/lib/koArray';
import {DocModel, IRowModel, recordSet, refRecord, ViewSectionRec} from 'app/client/models/DocModel'; import {DocModel, IRowModel, recordSet, refRecord, ViewSectionRec} from 'app/client/models/DocModel';
import {ColumnRec, ValidationRec, ViewRec} from 'app/client/models/DocModel'; import {ColumnRec, ValidationRec, ViewRec} from 'app/client/models/DocModel';
import * as modelUtil from 'app/client/models/modelUtil'; import * as modelUtil from 'app/client/models/modelUtil';
import {summaryGroupByDescription} from 'app/common/ActiveDocAPI';
import {MANUALSORT} from 'app/common/gristTypes'; import {MANUALSORT} from 'app/common/gristTypes';
import * as ko from 'knockout'; import * as ko from 'knockout';
import randomcolor from 'randomcolor'; import randomcolor from 'randomcolor';
@ -68,8 +69,7 @@ export function createTableRec(this: TableRec, docModel: DocModel): void {
if (!this.summarySourceTable()) { if (!this.summarySourceTable()) {
return ''; return '';
} }
const groupBy = this.groupByColumns(); return summaryGroupByDescription(this.groupByColumns().map(c => c.label()));
return `[${groupBy.length ? 'by ' + groupBy.map(c => c.label()).join(", ") : "Totals"}]`;
}); });
// TODO: We should save this value and let users change it. // TODO: We should save this value and let users change it.

View File

@ -156,6 +156,27 @@ export interface PermissionDataWithExtraUsers extends PermissionData {
exampleUsers: UserAccessData[]; 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 { export interface ActiveDocAPI {
/** /**
* Closes a document, and unsubscribes from its userAction events. * 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 * 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. * 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. * Wait for document to finish initializing.

View File

@ -14,6 +14,7 @@ import {
import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup'; import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup';
import {ActionSummary} from "app/common/ActionSummary"; import {ActionSummary} from "app/common/ActionSummary";
import { import {
AclTableDescription,
ApplyUAOptions, ApplyUAOptions,
ApplyUAResult, ApplyUAResult,
DataSourceTransformed, 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 * 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. * 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)) { if (!this.docData || !await this._granularAccess.hasAccessRulesPermission(docSession)) {
throw new Error('Cannot list ACL resources'); throw new Error('Cannot list ACL resources');
} }
const result: {[tableId: string]: string[]} = {}; const result: {[tableId: string]: AclTableDescription} = {};
const tables = this.docData.getMetaTable('_grist_Tables'); const tables = this.docData.getMetaTable('_grist_Tables');
for (const tableId of tables.getColValues('tableId')) { const sections = this.docData.getMetaTable('_grist_Views_section');
result[tableId] = ['id'];
}
const columns = this.docData.getMetaTable('_grist_Tables_column'); 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()) { for (const col of columns.getRecords()) {
const tableId = tables.getValue(col.parentId, 'tableId')!; 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; return result;
} }