Summary:
Fixes a bug introduced in https://phab.getgrist.com/D3416 which exposed a new type of linking option that didn't actually work. Specifically it allowed selecting by a summary table when the target widget has a reference column to the source table of the summary. This diff correctly implements this linking by linking the reference column against the 'group' column of the summary table, so the source table data (which the client may not have access to) isn't involved. But the 'group' column name is hidden from the 'select by' option label to avoid confusion for users, so it just looks like another kind of summary table linking, and indeed it can be thought of purely in terms of matching groupby columns etc.
Discussion here: https://grist.slack.com/archives/C02EGJ1FUCV/p1654039063055499
Also fixes a related old bug that offered similar meaningless options involving summary tables, like selecting the summary table by the source table or linking summary tables with disjoint groupby columns.
Test Plan: Updated SelectBySummaryRef test and fixture doc for the new behaviour. Also updated a couple of older tests which were incorrectly asserting the buggy option to select a summary table by the source table.
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3464
Summary:
Relax the restriction in `selectBy.isValidLink` so that summary tables can be linked by a column like other tables, except the `group` column. See the discussion on https://grist.slack.com/archives/C0234CPPXPA/p1651773623256959 (the replies are on the following message) for more info on this decision.
Tweaked `LinkingState.ts` since linking with summary tables can now involve a column.
Test Plan: Added a new nbrowser test and fixture checking the options to select by given a summary table with a few ref/reflist columns. Manually tested the behaviour of each option.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3416
Summary:
When linking table to a summary, the linking ended up broken after updating the summary group by columns.
This diff fixes that issue.
There were two issues:
1) some subscriptions were missing due to some .peek() calls instead of directly calling the observable.
2) the LinkingState instance was not being disposed.
3) the filterColValues was not updating after source data table has been loaded
Test Plan: Include new test file.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3358
Summary:
Clicking undo/redo after converting a table to a summary table navigated
to the raw data view.
Test Plan: new test
Reviewers: georgegevoian, alexmojaki
Reviewed By: georgegevoian, alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3337
Summary:
Allows selecting by a reflist in another table. This generalises cursor-linking with a ref column, but now it's filter linking.
Added another case to LinkingState where the source column is a reflist to the target table, filtering by the id column.
Updated convertQueryFromRefs and related functions to handle this since the id column has no column ref. In this case the string 'id' is used instead of a number.
LinkingState also checks if the source value is a reflist and uses that as the list of filter values instead of a single-element list of the cell value.
Indirect linking also works, where the source and target columns both are both references to the same table. This was the plan for a source reflist and target ref column.
I was surprised to see it also works perfectly when both columns are reflists, and it filters rows where there's an intersection!
Adding rows to the target section using the selected source record for default values is iffy. When filtering by row ID, there's no column for defaults, so the new row disappears.
For a source reflist and target ref, the first value of the reflist is the default, which is okayish. When both are reflists, the full source reflist is the default for the target column.
This seems like a bit much but just using the first value seems a bit arbitrary when there's room for all of them?
While doing all this I noticed an unrelated bug which I fixed as I was refactoring. Previously cursor linking based on a reference column did not update the cursor in the link target
when the value of the selected reference cell changed. Now cursor linking uses a floating row model like most other cases to observe the value correctly.
Test Plan: Extended SelectByRefList test and fixture, added previously failing test to RightPanelSelectBy.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3004
Summary:
Previously, using "Change Widget" allowed one to change the underlying table,
but would keep the linking settings. This could allow invalid settings which
would sometimes lead to JS errors. These manifested in production as
"UserError: Query error: n is not a function".
- Unset linking settings in this case, to avoid invalid values.
- In case invalid values are encountered (e.g. saved previously), treat them as
unset, to avoid JS errors.
- If an error does occur, report it with a stack trace.
Also, for testing, added 'selectBy' option to gristUtils helpers for using page-widget-picker.
Test Plan: Added test cases for resetting linking, and for ignoring invalid link settings.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D2993
Summary:
Use 'intersects' query operation when linking against a RefList column, otherwise the rest is the same as linking with a Ref column.
Add RefList columns to Select By options along with Ref columns.
Test Plan: Added new test and fixture similar to SelectBySummary
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2986
Summary:
This moves all client code to core, and makes minimal fix-ups to
get grist and grist-core to compile correctly. The client works
in core, but I'm leaving clean-up around the build and bundles to
follow-up.
Test Plan: existing tests pass; server-dev bundle looks sane
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2627
