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

Conversation

paulfitz
Copy link
Member

@paulfitz paulfitz commented Oct 9, 2024

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.

Context

This was motivated by a user's experience adding a seed rule related to UUIDs:
https://community.getgrist.com/t/linkkey-access-rule-for-all-tables/6707

Proposed solution

We wait until a rule set is associated with some columns before checking column validity.

Has this been tested?

  • 👍 yes, I added tests to the test suite
  • 💭 no, because this PR is a draft and still needs work
  • 🙅 no, because this is not relevant here
  • 🙋 no, because I need help

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.
@paulfitz paulfitz marked this pull request as ready for review October 9, 2024 20:33
@@ -1809,6 +1817,9 @@ class ObsRulePart extends Disposable {
private _warnInvalidColIds(colIds?: string[]) {
if (!colIds || !colIds.length) { return false; }
const allValid = new Set(this._ruleSet.getValidColIds());
// Don't check column validity if we don't have any.
// For example, the seed rule has no columns.
if (allValid.size === 0) { return false; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are other cases of no-columns:

  • the default rule (fallback for all tables). I think it would be hard to impossible to avoid it being an invalid rule then.
  • the special rules. Using columns there would probably make them invalid too.

Can we maybe target seed rules more specifically here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will bring back some code for that, this just felt simpler but you're right, it is too broad.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@paulfitz paulfitz requested a review from dsagal October 10, 2024 17:33
Copy link
Member

@dsagal dsagal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@paulfitz paulfitz merged commit e43bb67 into main Oct 10, 2024
11 checks passed
@paulfitz paulfitz deleted the paulfitz/seed-rule-with-column branch October 10, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants