Test Plan: Set all columns to hidden and reloaded document.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2832
Summary:
We used tslint earlier, and on switching to eslint, some rules were not
transfered. This moves more rules over, for consistent conventions or helpful
warnings.
- Name private members with a leading underscore.
- Prefer interface over a type alias.
- Use consistent spacing around ':' in type annotations.
- Use consistent spacing around braces of code blocks.
- Use semicolons consistently at the ends of statements.
- Use braces around even one-liner blocks, like conditionals and loops.
- Warn about shadowed variables.
Test Plan: Fixed all new warnings. Should be no behavior changes in code.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2831
Test Plan: Created a user attribute under access rules, set the attribute to look up to user.LinkKey.e, confirmed that setting e_ in the URL modified access.
Reviewers: dsagal, paulfitz
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2824
Summary: Extract out function _onClickHyperlink
Test Plan: Made a table, formatted column as hyperlink, added values including a link to another page in the document, another document, and an external website, clicked on all the links and only the first one didn't open a new tab.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2826
Summary:
This diff implements the search improvement that are mentioned here https://grist.quip.com/j1biAmfGkbzV/Search-Improvements.
CAVEATS:
I've noticed a bit of a usability caveats: the tooltips overlap with the new `search all pages` checkbox, which requires user to move cursor away for a bit in order to be able to click the button.
{F28224}
I've experimented locally with tooltips showing on both sides of the arrows, but it overlaps with the cross icon so could also be an issue. I couldn't think of any clear simple alternative, probably not too big of an issue anyway.
Test Plan: Added new test.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2818
Summary: Editor position wasn't restored on a long list, where the rows haven't been shown yet (the scroll haven't happend yet).
Test Plan: Browser tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2823
Summary:
- The parameter was inadvertently removed by userOverrideParams().
- when passed a function to `urlState().setHref(...)` or `urlState().makeUrl(...)` it is important that the function does not mutate the state that it receives as argument.
Link to the related task: https://gristlabs.getgrist.com/doc/check-ins/p/5#a1.s9.r791.c19
Test Plan: Adds test of the persistence by slightly modifying existing nbrowser/AccessRules2 tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D2820
Summary:
For conversions between Choice and ChoiceList, it makes more sense to preserve
the list of choices than to re-parse it from data.
Reported by Anais. Creating Choices from parsing ChoiceList cell values was
particularly poor, resulting in choices like "L,Foo,Bar".
Test Plan: Added a test case
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2819
Summary: Grist document, when reloaded, is able to restore the latest cursor position and the editor state.
Test Plan: Browser test were created.
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D2808
Summary: Text editor for Integer and Numeric column was showing null or undefined when the underlying value was null.
Test Plan: Browser test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2817
Summary:
Currently, to compute intermediate steps in a bundle, the bundle
is sent to the data engine to process. Then, if the intermediate
steps break a rule, it is reverted. One problem introduced by
checking permissions this late is that the data engine can be
exposed for formulas with python code by users who don't have the
right to change formulas. This diff pre-checks cases that change
formulas.
Test Plan: added a test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2816
Summary:
Auto-complete helps enter correct column names, and when incorrect ones are
entered, we now show an error and prevent saving the rules.
In an unrelated tweak, fix focusing of ACLFormula when clicking into scroll area.
Test Plan: Added a test case for showing invalid columns
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2815
Summary: For row creations and deletions, treat `rec` and `newRec` variables as identical. This simplifies writing a single rule that controls multiple permissions.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2812
Summary:
When scrolling quicly through a column with hyperlinks, null could be passed to
a function that didn't expect it. Added better types would help catch it.
Test Plan: Tested manually
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2813
Summary:
This diff discounts indirect changes for access control purposes. A UserAction that updates a cell A, which in turn causes changes in other dependent cells, will be considered a change to cell A for access control purposes.
The `engine.apply_user_actions` method now returns a `direct` array, with a boolean for each `stored` action, set to `true` if the action is attributed to the user or `false` if it is attributed to the engine. `GranularAccess` ignores actions attributed to the engine when checking for edit rights.
Subtleties:
* Removal of references to a removed row are considered direct changes.
* Doesn't play well with undos as yet. An action that indirectly modifies a cell the user doesn't have rights to may succeed, but it will not be reversible.
Test Plan: added tests, updated tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2806
Summary:
- Adds a new ChoiceList type, and widgets to view and edit it.
- Store in SQLite as a JSON string
- Support conversions between ChoiceList and other types
Test Plan: Added browser tests, and a test for how these values are stored
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2803
Summary: This treats newRec in the same way as rec in access formulas.
Test Plan: updated test for column renames; autocomplete checked manually.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2810
Summary: As practice for upgrading node in regular grist images, this upgrades the grist-core image to v14.
Test Plan: manual
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2807
Summary: Saving sort or filter is not permitted in readonly mode. Hence we remove the button. The diff adds a new unit to
Test Plan: Adds test for behaviour to `nbrowser/ReadOnlyMode`. Also adds tests for other Save buttons related to sort & filter that should be disabled.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2804
Summary:
- close sort&filter menu when clicking Save/Revert buttons
- also closes when clicking Apply/Cancel from a nested filter menu
Test Plan:
- updated existing test to match new spec
- added new test to cover new behaviour
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2799
Summary: - Combination of styling of what's in the dropdown and what's in the sort config
Test Plan: adds new nbrowser test
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2798
Summary:
- Clicking quickly on the small save/revert button was caussing the
tooltip to stay around.
- But if user waited a little bit before clicking the save button,
the tooltip was shown, and then properly removed when the button was removed.
- Code was missing propertly handling of disposal before the tooltip
were shown.
Test Plan: Added test case to the projects/tooltip.ts tests
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2797
Summary:
- If you type into the "Everyone" / "Everyone Else" rule, and it stops being
the default rule, there will now be an extra row with a "+" button to add a
new default rule
- Switch to ACE-supported auto-resizing (for better scrollbars handling)
- Tweak ACE padding styles for better-looking scrolling.
Test Plan: Added a test case for the extra "+" button.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2796
Summary:
- Supports auto-completion
- Supports various keyboard interactions to delete/select/copy-paste tokens
- Supports various mouse interactions to select and move tokens.
- Supports undo/redo while editing tokens.
Test Plan: stand-alone fixture and browser test verify the supported behaviors
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2789
Summary:
- New sort and filter button has several states
- Empty / unsaved / saved
- offers small save/revert button when unsaved
- Fix little issue with hanging tooltip when the refElem is disposed.
- The problem was that if you hover the save (or revert) button
and then click the button, it causes the button to disappear,
but the tooltip was staying.
Test Plan: Updated all tests to match the new UI.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, paulfitz
Differential Revision: https://phab.getgrist.com/D2795
Summary: Select all functionality by clicking the upper left margin corner
Test Plan: Browser test created
Reviewers: dsagal, paulfitz
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2793
Summary:
When a document has an exception to allow copies,
unset that option on any copies of the document.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2794
Summary:
Exceptional document operations (particularly `system` and `nascent`
operations) should never be denied by a granular access rule.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2792
Summary: Access to structural tables currently depends on SchemaEdit permission. We now make an exception for owner access to _grist_ACLResources and _grist_ACLRules, giving them unconditional access. It was too easy for owners to lock themselves out of editing access rules.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2790
Summary:
This helps forcibly end mocha tests when they hang, and print out
something that may help debug the situation.
Also add the generated static/bundle.css file to core/.gitignore.
Also, avoid using npm-packages-offline-cache when building core, by avoiding use of .yarnrc which turns it on.
Test Plan: Tested manually
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2788
Summary:
- Update rules to be more like we've had with tslint
- Switch tsserver plugin to eslint (tsserver makes for a much faster way to lint in editors)
- Apply suggested auto-fixes
- Fix all lint errors and warnings in core/, app/, test/
Test Plan: Some behavior may change subtly (e.g. added missing awaits), relying on existing tests to catch problems.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2785
Summary:
update user.Link to user.LinkKey in autocomplete.
This was a late change on the backend.
Test Plan: checked manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2787
Summary:
attribute ActiveDoc log messages to users regardless of whether
they were triggered via a client or directly via api
Test Plan: log messages checked manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2786
Summary: add user and docId to heartbeat logging
Test Plan: checked manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2784
Summary:
This removes some unintentional repetition of work when there are
no row-level rules (there was a missing `return`).
Test Plan: existing tests pass
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2782
Summary:
- Cache the count by column, factoring out ColumnCache from
ColumnACIndexes, which uses a similar pattern.
- Update error counts in response to column selection and to data changes.
Test Plan: Adds a test case for the new message
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2780
Summary:
- Adds a + button to the filter. Button triggers a menu that allow to
add one of the column that does not already have a filter set.
Caveats:
- for now menu only allows to choose from visible column.
- This diff introduces a slight change of behavior of how filter works:
- Filter used to be automatically removed when user set them to all
inclusive (ie: by clicking the `All` button).
- With this diff, it is no longer the case.
- indeed, when filter are added to the filter bar with the `+` btn they are initially in the `all inclusive` state, hence would have been removed with the above mention behaviour.
Test Plan: Added new test to nbrowser/FilterBar
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2776
Summary:
Fix an error that used to happen on Chrome: `RangeError: Maximum call stack size
exceeded`. Happened when clicking the `Other Values` checkbox when
filtering a large table. Turns out culprit was a function call that
was using a spread operator to pass a large number of argument to a
function.
Spread operator for passing multiple argument must not be used with
too many arguments. Otherwise it could hit the engine's argument
length limit. That limit varies across browser (webkit's
javascriptcore engine has argument limit of 65536).
Some interesting description of the limit can be found here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_and_built-in_functions
In order to get the fix fast, implementing a proper test for it is left for a follow-up commit.
Test Plan:
- Manually tested on Chrome/FF
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D2779
Summary: This addresses a weakness in the following case: rules controlling view access for a column, with a dependency on the values of other columns. We had disabled support for such rules, since the existing implementation worked only on table loads and not on broadcast changes. This diff adds in logic to enrich broadcasts as needed, and allows such rules.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2774
Summary:
When adding robustness to schema changes to granular access control,
a calculation of intermediate row states that was previously done
semi-intelligently on need started happening less intelligently.
This diff separates out the row state calculations from metadata
state calculations so that one can happen without the other.
Test Plan: extended a test. Also did some manual checks.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2773
Summary:
For a long array with removals proportional to that length,
lodash/pullAt becomes slow due to doing one splice per removal.
This diff swaps in an alternate implementation that doesn't become
quadratic. On a 250k-row doc with a row-level access rule, this improves
initial page load for a viewer with access to half the rows from minutes
to seconds.
Test Plan: added test; did manual benchmarking
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2777
Summary:
A client hit a situation where a granular access control "bundle"
was not closed, leaving the document locked until reset. I don't
yet have a replication. This diff is a possible mitigation,
trusting various methods less.
Test Plan: existing tests pass
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2775
Summary:
- add new filterBar option to views section
- add toggle to the section menu
- add filter bar
- shows Save/Revert btn when unsaved change
- shows all filered fields witch edit and delete buttons
Test Plan: Add new FilterBar nbrowser test
Reviewers: paulfitz, dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2769
Summary:
This fixes DocStorage.fetchQuery when the number of parameters
exceeds the maximum that can be passed directly to sqlite.
In this case, parameters are now stored and used from a temporary
table.
Problem first noticed via a use of DocStorage.fetchQuery by
granular access controls. Access control should be optimized
to make fewer such queries, but that is a separate issue.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2772
Summary:
Activate CI for grist-core using github actions. Can be improved
with caching but starting simple.
Test Plan: tested in a fork of grist-core
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2771
Alex Hall
Dmitry S
Cyprien P
Jarosław Sadziński
Paul Fitzpatrick
Timothy John Perisho Eccleston
Jarosław Sadziński