Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow seed rules to refer to columns #1254

Merged
merged 3 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading