Update dropdown conditions on column rename #1038

SleepyLeslie · 2024-06-12T20:36:22Z

This PR adds the ability to automatically update the dropdown condition formula on a reference column when a column it refers to has been renamed.

berhalak

Can you add some tests for this functionality?

SleepyLeslie · 2024-06-13T22:29:52Z

Refactored the entire logic. Now ACL formulas and dropdown condition formulas use the same logic to handle renames. Previously $ notations in ACL formulas are not auto-renamed. This should also be fixed now.

Comments and tests to come later.

Currently translated at 100.0% (1340 of 1340 strings) Translation: Grist/client Translate-URL: https://hosted.weblate.org/projects/grist/client/de/

Each time the a Grist page is reload the `last_connection_at` of the user is updated resolve [#924](#924)

We need this directory for building the image, but not for running the tests outside of it.

While the intent was to run tests with it, we don't need it. Instead, this caused problems because the stubs overrode the intended `ext` directory and therefore disabled the ext features.

Currently translated at 99.1% (1329 of 1340 strings) Translation: Grist/client Translate-URL: https://hosted.weblate.org/projects/grist/client/fr/

Currently translated at 100.0% (1340 of 1340 strings) Translation: Grist/client Translate-URL: https://hosted.weblate.org/projects/grist/client/sk/

Summary: - Adding confirmation dialog when user doesn't want to cancel site - Changing `Cancel subscription` to `Cancel plan` - Removing `Pro` from upgrade header on pricing modal - Better handling situation when there is no default price - Removing mentions about sprouts program - Removing cache for stripe plans Test Plan: Updated tests Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D4273

Summary: Some editors do some async work before saving the value (Ref column can add new records). Those actions were send without bundling, so it wasn't possible to undo those actions with togheter. Test Plan: Added new test Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D4285

This is to facilitate alerting to detect if snapshot generation were to stall for a document.

Reorganize preview workflows so that previews can be made for PRs from outside contributors.

Summary: Adding authorization header support for webhooks. Issue: #827 --------- Co-authored-by: Florent <florent.git@zeteo.me>

dsagal

Thank you! Looks good, just one lingering question about syntaxerrors.

dsagal · 2024-07-09T05:41:56Z

sandbox/grist/dropdown_condition.py

-    widget_options["dropdownCondition"] = {"text": new_dc_formula}
-    try:
-      # Parse the new dropdown condition formula if it is syntactically correct.
+    # The data engine stops processing remaining formulas when it hits an exception. Parsing a


This sentence sounds wrong. Generally Grist formulas can have exceptions and that doesn't normally interfere with other formulas. So probably you mean something more specific -- it would be good to clarify.

I rephrased it a bit. Hope it's clearer now.

sandbox/grist/predicate_formula.py

sandbox/grist/test_dropdown_condition_renames.py

dsagal · 2024-07-11T22:17:37Z

I think one remaining comment is about making public a UserActions member that seems better to keep non-public: #1038 (comment)

dsagal

I have two more comments but really minor now. Approving. Thank you very much! I appreciate in particular unifying quite a bit of the logic and fixing the renaming of acl formulas with "$" signs along the way!

sandbox/grist/acl.py

sandbox/grist/dropdown_condition.py

berhalak · 2024-07-16T13:51:45Z

sandbox/grist/useractions.py

      # Build up a dictionary mapping col_ref of each affected formula to the new formula text.
      formula_updates = self._prepare_formula_renames(renames)

      # For any affected columns, include the formula into the update.
      for col_rec, new_formula in sorted(six.iteritems(formula_updates)):
        col_updates.setdefault(col_rec, {}).setdefault('formula', new_formula)

+      acl.perform_acl_rule_renames(self, renames)


@dsagal @SleepyLeslie Can I move those two method calls down below (where it were before)?
The new order of things generates acl rename actions before the column is actually renamed. Before the order was reversed, first RenameAction then UpdateRecord (for ACL)

After discussing it offline, I will update it in another PR.

Update dropdown conditions on column rename

88a857b

SleepyLeslie force-pushed the sleepyleslie/dropdown-cond-update branch from c564153 to 88a857b Compare June 12, 2024 20:38

SleepyLeslie requested review from paulfitz, dsagal, berhalak and georgegevoian June 12, 2024 20:39

berhalak reviewed Jun 13, 2024

View reviewed changes

SleepyLeslie force-pushed the sleepyleslie/dropdown-cond-update branch 5 times, most recently from 3e14132 to 101f4e9 Compare June 15, 2024 01:00

SleepyLeslie requested a review from berhalak June 15, 2024 01:00

SleepyLeslie force-pushed the sleepyleslie/dropdown-cond-update branch 2 times, most recently from 2ead64c to 4eeb928 Compare June 15, 2024 01:10

SleepyLeslie added 8 commits June 14, 2024 20:55

Handle rec. and $ column refs

3014237

Reuse the same logic for ACL formulas

a773272

Add comments and cleanup

df10e29

Add test for dollar signs in ACL formulas

6c9477e

Add tests for dropdown condition formula renames

1c37047

Add comments to new test case

d9d5100

Use ModifyColumn instead of UpdateRecord

270c7a9

Improve the test cases

b9ea716

SleepyLeslie force-pushed the sleepyleslie/dropdown-cond-update branch 2 times, most recently from e538e20 to a1b537b Compare June 15, 2024 02:17

Handle wrong/unsupported syntax in pred formulas

9847b84

SleepyLeslie force-pushed the sleepyleslie/dropdown-cond-update branch from a1b537b to 9847b84 Compare June 15, 2024 02:20

jordigh self-assigned this Jun 18, 2024

SleepyLeslie requested a review from jordigh June 18, 2024 15:58

Paul Janzen and others added 21 commits July 8, 2024 13:30

Translated using Weblate (German)

ba95ef4

Currently translated at 100.0% (1340 of 1340 strings) Translation: Grist/client Translate-URL: https://hosted.weblate.org/projects/grist/client/de/

Create user last connection datetime (#935)

28a22ca

Each time the a Grist page is reload the `last_connection_at` of the user is updated resolve [#924](#924)

workflows: Do not use ext/ director to run tests

18a39cb

We need this directory for building the image, but not for running the tests outside of it.

tsconfig-ext: revert bc52f65

55d13bd

While the intent was to run tests with it, we don't need it. Instead, this caused problems because the stubs overrode the intended `ext` directory and therefore disabled the ext features.

Translated using Weblate (French)

511a4a3

Currently translated at 99.1% (1329 of 1340 strings) Translation: Grist/client Translate-URL: https://hosted.weblate.org/projects/grist/client/fr/

Translated using Weblate (Slovak)

419403a

Currently translated at 100.0% (1340 of 1340 strings) Translation: Grist/client Translate-URL: https://hosted.weblate.org/projects/grist/client/sk/

log periodic per-document statistics about snapshot generation

e1d0224

This is to facilitate alerting to detect if snapshot generation were to stall for a document.

add a getSnapshotProgress implementation to DocStorageManager

af8c261

docstrings, moment import, fix log format

3f6f0e9

Enable external contributors to create previews (#1068)

5865134

Reorganize preview workflows so that previews can be made for PRs from outside contributors.

Add authorization header in webhooks stored in secrets table (#941)

89f5fc0

Summary: Adding authorization header support for webhooks. Issue: #827 --------- Co-authored-by: Florent <florent.git@zeteo.me>

Move HomeDBManager to gen-server/lib/homedb (#1076)

530ff8f

Fix linting issues

d1e70e3

Merge main

5c6eece

Remove redundant error checking

49b55c2

Replace ValueError with more accurate SyntaxError

78ace36

Revise error handling for invalid formulas

421eafb

Separate column type parsing into usertypes

e73f0dc

Use map_back_patch to replace map_back_offset

ca67ec2

dsagal reviewed Jul 9, 2024

View reviewed changes

Address various PR remarks

b4f415f

Make docmodel private and add a getter for it

06d50a4

dsagal approved these changes Jul 12, 2024

View reviewed changes

sandbox/grist/acl.py Outdated Show resolved Hide resolved

sandbox/grist/dropdown_condition.py Show resolved Hide resolved

Address more PR remarks

94263cf

SleepyLeslie merged commit 6326205 into main Jul 12, 2024
11 checks passed

SleepyLeslie deleted the sleepyleslie/dropdown-cond-update branch July 12, 2024 14:59

berhalak reviewed Jul 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dropdown conditions on column rename #1038

Update dropdown conditions on column rename #1038

SleepyLeslie commented Jun 12, 2024

berhalak left a comment

SleepyLeslie commented Jun 13, 2024 •

edited

Loading

dsagal left a comment

dsagal Jul 9, 2024

SleepyLeslie Jul 9, 2024

dsagal commented Jul 11, 2024

dsagal left a comment

berhalak Jul 16, 2024 •

edited

Loading

berhalak Jul 16, 2024

Update dropdown conditions on column rename #1038

Update dropdown conditions on column rename #1038

Conversation

SleepyLeslie commented Jun 12, 2024

berhalak left a comment

Choose a reason for hiding this comment

SleepyLeslie commented Jun 13, 2024 • edited Loading

dsagal left a comment

Choose a reason for hiding this comment

dsagal Jul 9, 2024

Choose a reason for hiding this comment

SleepyLeslie Jul 9, 2024

Choose a reason for hiding this comment

dsagal commented Jul 11, 2024

dsagal left a comment

Choose a reason for hiding this comment

berhalak Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

berhalak Jul 16, 2024

Choose a reason for hiding this comment

SleepyLeslie commented Jun 13, 2024 •

edited

Loading

berhalak Jul 16, 2024 •

edited

Loading