Skip to content

Commit

Permalink
allow seed rules to refer to columns (#1254)
Browse files Browse the repository at this point in the history
This makes a small change to the UI for entering seed rules,
so that a seed rule that refers to a column may be added.
This is a bit niche, but occasionally handy when all tables
have some common structure.
  • Loading branch information
paulfitz authored Oct 10, 2024
1 parent aa69652 commit e43bb67
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 4 deletions.
28 changes: 26 additions & 2 deletions app/client/aclui/AccessRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,16 @@ abstract class ObsRuleSet extends Disposable {
public getCustomRules(): ObsRulePart[] {
return this._body.get().filter(rule => !rule.isBuiltInOrEmpty());
}

/**
* If the set applies to a special column, return its name.
*/
public getSpecialColumn(): string|undefined {
if (this._ruleSet?.tableId === SPECIAL_RULES_TABLE_ID &&
this._ruleSet.colIds.length === 1) {
return this._ruleSet.colIds[0];
}
}
}

class ColumnObsRuleSet extends ObsRuleSet {
Expand Down Expand Up @@ -1635,6 +1645,14 @@ class ObsRulePart extends Disposable {
!isEqual(use(this._permissions), this._rulePart?.permissions ?? emptyPerms)
);
});
// The formula may be invalid from the beginning. Make sure we show errors in this
// case.
const text = this._aclFormula.get();
if (text) {
this._setAclFormula(text, true).catch(e => {
console.error(e);
});
}
}

public getRulePart(): RuleRec {
Expand Down Expand Up @@ -1790,8 +1808,8 @@ class ObsRulePart extends Disposable {
return this.isBuiltIn() && this._ruleSet.getFirstBuiltIn() !== this;
}

private async _setAclFormula(text: string) {
if (text === this._aclFormula.get()) { return; }
private async _setAclFormula(text: string, initial: boolean = false) {
if (text === this._aclFormula.get() && !initial) { return; }
this._aclFormula.set(text);
this._checkPending.set(true);
this._formulaProperties.set({});
Expand All @@ -1809,6 +1827,12 @@ class ObsRulePart extends Disposable {
private _warnInvalidColIds(colIds?: string[]) {
if (!colIds || !colIds.length) { return false; }
const allValid = new Set(this._ruleSet.getValidColIds());
const specialColumn = this._ruleSet.getSpecialColumn();
if (specialColumn === 'SeedRule') {
// We allow seed rules to refer to columns without checking
// them (until the seed rules are used).
return false;
}
const invalid = colIds.filter(c => !allValid.has(c));
if (invalid.length > 0) {
return `Invalid columns: ${invalid.join(', ')}`;
Expand Down
76 changes: 76 additions & 0 deletions test/nbrowser/AccessRules3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,82 @@ describe("AccessRules3", function() {
await assertSaved();
});

it('can have a SeedRule special that refers to columns', async function() {
// Open Access Rules page.
const mainSession = await gu.session().teamSite.user('user1').login();
await mainSession.loadDoc(`/doc/${docId}`);
await driver.find('.test-tools-access-rules').click();
await driver.findWait('.test-rule-set', 2000);

// Check seed rule checkbox is unselected.
const seedRule = await driver.find('div.test-rule-special-SeedRule');
const checkbox = seedRule.find('input[type=checkbox]');
assert.equal(await checkbox.isSelected(), false);

// Now check the box, and see we get the default rule we expect.
await checkbox.click();
await assertChanged();
await driver.find('.test-rule-special-SeedRule .test-rule-special-expand').click();
assert.deepEqual(await getRules(seedRule),
[{ formula: 'user.Access in [OWNER]', perm: '+R+U+C+D' }]);
assert.equal(await hasExtraAdd(seedRule), true);

// Tweak the seed rule to refer to a column.
await seedRule.find('.test-rule-part .test-rule-add').click();
await enterRulePart(seedRule, 1, 'rec.Year == 1', 'Deny All', 'memo1');
assert.equal(await checkbox.getAttribute('disabled'), 'true');

// New table rules should include the seed rule.
await driver.findContentWait('button', /Add Table Rules/, 2000).click();
await driver.findContentWait('.grist-floating-menu li', /FinancialsTable/, 3000).click();
let fin = findTable(/FinancialsTable/);
assert.deepEqual(await getRules(fin),
[{ formula: 'rec.Year == 1', perm: '-R-U-C-D', res: 'All', memo: 'memo1'},
{ formula: 'user.Access in [OWNER]', perm: '+R+U+C+D', res: 'All' },
{ formula: 'Everyone Else', perm: '', res: 'All' }]);
assert.equal(await hasExtraAdd(fin), false);
await removeTable(/FinancialsTable/);

// Tweak the seed rule to refer to a column that won't exist.
await enterRulePart(seedRule, 1, 'rec.Unreal == 1', 'Deny All', 'memo1');
assert.equal(await checkbox.getAttribute('disabled'), 'true');

// New table rules should include the seed rule, and show an error.
await driver.findContentWait('button', /Add Table Rules/, 2000).click();
await driver.findContentWait('.grist-floating-menu li', /FinancialsTable/, 3000).click();
fin = findTable(/FinancialsTable/);
assert.deepEqual(await getRules(fin),
[{ formula: 'rec.Unreal == 1', perm: '-R-U-C-D', res: 'All', memo: 'memo1',
error: 'Invalid columns: Unreal' },
{ formula: 'user.Access in [OWNER]', perm: '+R+U+C+D', res: 'All' },
{ formula: 'Everyone Else', perm: '', res: 'All' }]);
assert.equal(await hasExtraAdd(fin), false);
await removeTable(/FinancialsTable/);

// Check that returning to the single OWNER rule gets us back to an uncomplicated
// selected checkbox.
await assertChanged();
assert.equal(await checkbox.getAttribute('disabled'), 'true');
assert.equal(await checkbox.isSelected(), false);
await seedRule.find('.test-rule-part .test-rule-remove').click();
assert.equal(await checkbox.getAttribute('disabled'), null);
assert.equal(await checkbox.isSelected(), true);

// Check that removing that rule deselected the checkbox and collapses rule list.
await seedRule.find('.test-rule-part .test-rule-remove').click();
assert.equal(await checkbox.getAttribute('disabled'), null);
assert.equal(await checkbox.isSelected(), false);
await assertSaved();
assert.lengthOf(await seedRule.findAll('.test-rule-set'), 0);

// Expand again, and make sure we are back to default.
await driver.find('.test-rule-special-SeedRule .test-rule-special-expand').click();
assert.lengthOf(await seedRule.findAll('.test-rule-set'), 1);
assert.deepEqual(await getRules(seedRule),
[{ formula: 'Everyone', perm: '' }]);
await assertSaved();
});

it('can save and reload SeedRule special', async function() {
const mainSession = await gu.session().teamSite.user('user1').login();
await mainSession.loadDoc(`/doc/${docId}`);
Expand Down
8 changes: 6 additions & 2 deletions test/nbrowser/aclTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ namespace gristUtils {
export async function getRules(el: WebElement): Promise<Array<{
formula: string, perm: string,
res?: string,
memo?: string}>> {
memo?: string,
error?: string}>> {
const ruleSets = await el.findAll('.test-rule-set');
const results: Array<{formula: string, perm: string,
res?: string,
Expand All @@ -162,9 +163,12 @@ namespace gristUtils {
}
const hasMemo = await part.find('.test-rule-memo').isPresent();
const memo = hasMemo ? await part.find('.test-rule-memo input').value() : undefined;
const hasError = await part.find('.test-rule-error').isPresent();
const error = hasError ? await part.find('.test-rule-error').getText() : undefined;
results.push({formula, perm: permParts.join(''),
...(memo ? {memo} : {}),
...(res ? {res} : {})
...(res ? {res} : {}),
...(error ? {error} : {}),
});
}
}
Expand Down

0 comments on commit e43bb67

Please sign in to comment.