* allow Grist front-end to function when location history is unavailable
When the Grist front-end is embedded in an iframe, using a srcdoc
attribute, history.pushState and similar methods are unavailable.
Currently, that makes it impossible to navigate between Grist pages,
since an access error is thrown (behavior may be browser dependent).
With this change, navigation succeeds.
* give unrelated possibly slow test a little more time
Summary:
The changes are intended to smooth over some sharp edges when a signed-out user
is using Grist (particularly while on the templates site).
Test Plan: Browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3957
Summary:
Fixes a problem reported here: https://community.getgrist.com/t/exporting-the-records-in-a-linked-view/2556/4
The download CSV/Excel link now contains an additional `linkingFilter` URL parameter containing JSON-encoded `filters` and `operations`. This object is originally created in the frontend in `LinkingState`, and previously it was only used internally in the frontend. It would make its way via `QuerySetManager` to `QuerySet.getFilterFunc` where the actual filtering logic happened. Now most of that logic has been moved to a similar function in `common`. The new function works with a new interface `ColumnGettersByColId` which abstract over the different ways data is accessed in the client and server in this context. There's no significant new logic in the diff, just refactoring and wiring.
Test Plan: Expanded two `nbrowser/SelectBy*.ts` test suites to also check the contents of a downloaded CSV in different linking scenarios.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3961
A date test was noted to fail, with a formula intended for
a cell ending up in the column header. This may help.
Entering formulas reliably requires waiting for a particular
focus state.
This test appears to fail if toasts aren't issued fast enough
(perhaps because an empty toast list may count as truthy?).
I might be missing something since I don't understand the
purpose of waitForOverflownMessageToDisappear apart from
lengthening a timeout.
Summary:
Replaces https://phab.getgrist.com/D3940, particularly to avoid doing potentially unwanted things automatically.
Adds optional fields `evaluateCurrentFormula?: boolean; rowId?: number` to `FormulaAssistanceContext` (part of `AssistanceRequest`). When `evaluateCurrentFormula` is `true`, calls a new function `evaluate_formula` in the sandbox which computes the existing formula in the column (regardless of anything the AI may have suggested) and uses that to generate an additional system message which is added before the user's message. In theory this could be used in an interface where users ask why a formula doesn't work, including possibly a formula suggested by the AI. For now, it's only used in `runCompletion_impl.ts` for experimenting.
Also cleaned up a bit, removing `_chatMode` which is always `true` now, and uses of `regenerate` which is always `false`.
Test Plan: Updated `runCompletion_impl` to optionally use the new feature, in which case it now scores 51/68 instead of 49/68.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3970
Summary:
WindowDimensions from gristUtils needs exporting
after it got moved to another file.
Test Plan: existing tests should pass
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D3971
It would be useful to write browser tests that use Grist for some
of our other repositories (e.g. grist-widget, grist-static).
This is a first baby step to factor out some useful code that has
no code dependencies beyond mocha-webdriver, for use in grist-widget.
Summary: Also fixes a few bugs with some telemetry events not being recorded.
Test Plan: Manual.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3960
* Fix support of Scaleway S3 bucket #359
While MinIO and AWS return versionId as strings, other S3 API
implementations return versionId as integers.
We must carefully convert the versionId as string in order to cover
these various behaviors.
Also ensure that docStorage is initialized before attempting to
calculate the data size in order to avoid an exception.
* Add unit tests for MinIOExternalStorage#versions() #359
Introduced some unit tests to :
- ensure listObjects is called with the right arguments;
- cover the case when a S3 bucket implementation does not return the
versionId as a string but rather as an integer (like Scaleway):
in such a case, ensure that the returned snapshotId is a string;
- cover the case when the listObjects function emits an error, ensure the
versions() call rejets with the error emitted;
- that the deleteMarkers are only returned when the
includeDeleteMarkers is passed;
---------
Co-authored-by: Florent FAYOLLE <florent.fayolle@beta.gouv.fr>
Summary: Using standard tost notification, message about webhook queue being overflown was added. message is permanent as long as queue is full. Message contains linkt to the webhook setings
Test Plan: two nbrowser test was added - one to check if message is show when queue is full, and second to check if message is dismiss when queue was cleaned.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3929
Summary:
introduces POST /api/docs/{docId}/webhooks and DELETE /api/docs/{docId}/webhooks/{webhookId} on place of old _subscribe and _unsubscribe endpoints.
Remove checking for unsubscribeKey while deleting webhook - only owner can delete webhook using DELETE endpoint. subscription key is still needed for _unsubscribe endpoint.
old _unsubscribe and _subscribe endpoints are still active and work as before - no changes there.
Posting schema:
```
POST /api/docs/[docId]/webhooks
```
Request Body:
```
{
"webhooks": [
{
"fields": {
"url": "https://webhook.site/3bd02246-f122-445e-ba7f-bf5ea5bb6eb1",
"eventTypes": [
"add",
"update"
],
"enabled": true,
"name": "WebhookName",
"memo": "just a text",
"tableId": "Table1"
}
},
{
"fields": {
"url": "https://webhook.site/3bd02246-f122-445e-ba7f-bf5ea5bb6eb2",
"eventTypes": [
"add",
],
"enabled": true,
"name": "OtherWebhookName",
"memo": "just a text",
"tableId": "Table1"
}
}
]
}
```
Expected response: WebhookId for each webhook posted:
```
{
"webhooks": [
{
"id": "85c77108-f1e1-4217-a50d-acd1c5996da2"
},
{
"id": "d87a6402-cfd7-4822-878c-657308fcc8c3"
}
]
}
```
Deleting webhooks:
```
DELETE api/docs/[docId]/webhooks/[webhookId]
```
there is no payload in DELETE request. Therefore only one webhook can be deleted at once
Response:
```
{
"success": true
}
```
Test Plan: Old unit test improved to handle new endpoints, and one more added to check if endpoints are in fact created/removed
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: paulfitz, alexmojaki
Differential Revision: https://phab.getgrist.com/D3916
Test Plan: Added a check to the emoji test.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D3951
Summary:
Adding limits for AI calls and connecting those limits with a Stripe Account.
- New table in homedb called `limits`
- All calls to the AI are not routed through DocApi and measured.
- All products now contain a special key `assistantLimit`, with a default value 0
- Limit is reset every time the subscription has changed its period
- The billing page is updated with two new options that describe the AI plan
- There is a new popup that allows the user to upgrade to a higher plan
- Tiers are read directly from the Stripe product with a volume pricing model
Test Plan: Updated and added
Reviewers: georgegevoian, paulfitz
Reviewed By: georgegevoian
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3907
Summary:
In a selector table, when a selected row is filtered out of view, linked widgets should update based on the newly selected row.
There were a few bugs that contributed to this wrong behavior:
- Gridview wasn't subscribing to the current row id, and the row with id 'new' was being converted to the first row
- Cursor was keeping track of the currently selected row id, it was hiding a problem behind the proper rowIndex
- Undo/redo somehow leveraged the wrong rowId from the cursor during the position restore.
The `No data` text was also changed to be more meaningful.
Test Plan: Added and updated.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3937
Summary:
Adds a new Support Grist page (accessible only in grist-core), containing
options to opt in to telemetry and sponsor Grist Labs on GitHub.
A nudge is also shown in the doc menu, which can be collapsed or permanently
dismissed.
Test Plan: Browser and server tests.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Subscribers: jarek, dsagal
Differential Revision: https://phab.getgrist.com/D3926
Summary:
- Detecting emoji is surprisingly tricky; we use a fancy regex as a decent heuristic.
- Icons are a little larger than before.
- Styling tweaked for light and dark modes
- In case the OS doesn't render the emoji as one character, truncate what's
shown in the icon box.
Test Plan: Added a test case.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3904
A small test harness bundle was recently added that is breaking the docker image build. It could be added to the docker image, but that would introduce a bunch of extraneous test file dependencies. So this tweaks the build to simply skip the test bundle if its primary source file is not found.
Also added some other test fixes along the way:
* make a custom widget test more reliable
* update a localization test now that `pt` exists
* store more log info in artifact on error
There's a little nest of SelectBy tests that sometimes fail.
They are the only tests with an import of a helper function from
an other file that contains tests. Such imports have caused trouble
with mocha in the past. I'm not sure if that is the case now, but
I'd like to eliminate it as a possibility.
Summary:
This tweaks the prompting so that the user's message is given on its own instead of as a docstring within Python. This is so that the prompt makes sense when:
- the user asks a question such as "Can you write me a formula which does ...?" rather than describing their formula as a docstring would, or
- the user sends a message that doesn't ask for a formula at all (https://grist.slack.com/archives/C0234CPPXPA/p1687699944315069?thread_ts=1687698078.832209&cid=C0234CPPXPA)
Also added wording for the model to refuse when the user asks for something that the model cannot do.
Because the code (and maybe in some cases the model) for non-ChatGPT models relies on the prompt consisting entirely of Python code produced by the data engine (which no longer contains the user's message) those code paths have been disabled for now. Updating them now seems like undesirable drag, I think it'd be better to revisit this when iteration/experimentation has slowed down and stabilised.
Test Plan:
Added entries to the formula dataset where the response shouldn't contain a formula, indicated by the value `1` for the new column `no_formula`.
This is somewhat successful, as the model does refuse to help in some of the new test cases, but not all. Performance on existing entries also seems a bit worse, but it's hard to distinguish this from random noise. Hopefully this can be remedied in the future with more work, e.g. automatic followup messages containing example inputs and outputs.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3936
Summary:
This uses a newer version of mocha in grist-core so that tests can be run in parallel. That allows more tests to be moved without slowing things down overall. Tests moved are venerable browser tests; only the ones that "just work" or worked without too much trouble to are moved, in order to keep the diff from growing too large. Will wrestle with more in follow up.
Parallelism is at the file level, rather than the individual test.
The newer version of mocha isn't needed for grist-saas repo; tests are parallelized in our internal CI by other means. I've chosen to allocate files to workers in a cruder way than our internal CI, based on initial characters rather than an automated process. The automated process would need some reworking to be compatible with mocha running in parallel mode.
Test Plan: this diff was tested first on grist-core, then ported to grist-saas so saas repo history will correctly track history of moved files.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3927
Summary:
Some edits to virtual tables (such as webhook lists) happen
via a route that was not yet handled. Actually Cyprien (the
original author) had handled this case but it got removed
because I didn't know what it was for :-). This brings back
support for edits by this route.
Test Plan: added a test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3924
Summary: I looked through the template documents mentioned in `formula-dataset-index.csv` and selected formulas involving lookups to add to the CSV, particularly nontrivial formulas.
Test Plan: Running the test script on the new dataset gives a score of 47/61 compared to the previous 45/47, i.e. it scores 2/14 on the new entries. Lookups are clearly challenging and we'll need to add more information to the prompt, maybe even consider a more complicated strategy than a single prompt. This diff is purely for expanding the dataset, improving performance will come later.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3931
Summary:
The GristDocTutorial table is now always visible to users with edit
access to the trunk, and the Share menu is now available within
tutorial forks, making it easier for editors to replace the original
tutorial trunk with changes made in the fork, and for viewers to export
their copy of the tutorial.
Also, changes to the GristDocTutorial table are now immediately reflected
in the tutorial popup.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3930
Summary:
- Cleaning stripe data after billing tests
- Better stripe webhook test integration, that should fix test interference
- Not importing why-is-node-running when its not needed, which improves dev experience.
Test Plan: Modified
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3932
Summary:
1. Introduces another highlight for link-selector rows, with the same color as
regular selection, and allowing to overlap with regular selection.
2. Don't show "secondary" cursors (those in inactive sections), to keep a single
cursor on the screen, since having multiple (which different in color) could
cause confusion.
3. An unrelated improvement (prompted by a new fixture doc) is to default the
active section to the top-left one (rather than the one with smallest rowId).
4. Another unrelated improvement (prompted by a test affected by the previous unrelated improvement) is to skip chart widgets when searching (previously search would step through those with an invisible "cursor").
Includes also tweaks for better testing on Arm-based Macs:
- Add support for TEST_CHROME_BINARY_PATH environment variable (helpful for a Mac arm64 architecture workaround)
- Remove unsetting of SELENIUM_REMOTE_URL when running headless (unlikely to affect anyone, and can be done outside the script, but interferes with the Mac workaround)
Test Plan: Added a new test case that cursor and linking-selector CSS classes are present or absent appropriately. Fixed test affected by the fix to default active section.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3891
Summary:
The previous code for extracting a Python formula from the LLM completion involved some shaky string manipulation which this improves on.
Overall the 'test results' from `runCompletion` went from 37/47 to 45/47 for `gpt-3.5-turbo-0613`.
The biggest problem that motivated these changes was that it assumed that code was always inside a markdown code block
(i.e. triple backticks) and so if there was no block there was no code. But the completion often consists of *only* code
with no accompanying explanation or markdown. By parsing the completion in Python instead of JS,
we can easily check if the entire completion is valid Python syntax and accept it if it is.
I also noticed one failure resulting from the completion containing the full function (instead of just the body)
and necessary imports before that function instead of inside. The new parsing moves import inside.
Test Plan: Added a Python unit test
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3922
Not only GREP_TESTS can be assigned a single word like:
GREP_TESTS=DocApi yarn test
But also can be assigned a whole sentence part:
GREP_TESTS="supports ascending sort" yarn test
That's especially useful to run a single test (and not a whole test
suit)
Co-authored-by: Florent FAYOLLE <florent.fayolle@beta.gouv.fr>
Summary:
On Firefox and Safari, setting scrollLeft to a max safe integer was
causing it to be treated as 0. It's not clear why - for now, the
scrollWidth is used instead.
Also fixes a bug where the column title popup wouldn't appear for a
new column if tab was previously used to close the same popup for
the last column.
Test Plan: Browser test.
Reviewers: JakubSerafin
Reviewed By: JakubSerafin
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3911
Summary: Also fixes a few small bugs with telemetry collection.
Test Plan: Server and manual tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3915
Summary:
Adds support for optional telemetry to grist-core.
A new environment variable, GRIST_TELEMETRY_LEVEL, controls the level of telemetry collected.
Test Plan: Server and unit tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, anaisconce
Differential Revision: https://phab.getgrist.com/D3880
Summary:
sanitazing errors output in webhooks to protect users data (not show them in logs and other places).
Because redis is returing whole payload when error occur, best approach is to hijack exception as close to redis operation as posible and sanitize the data.
We need to know data structure do do this corretly tho. Currently I decided to just censore everything that has "payload" key.
Test Plan: Because logs that need to be sanitized come from redis, to be valid tested we should force redis to crash. It's hard to do in our integration test setup. In this moment, unit test is all we got.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3905
Summary:
Adding a way to detach an editor. Initially only implemented for the formula editor, includes redesign for the AI part.
- Initially, the detached editor is tight with the formula assistant and both are behind GRIST_FORMULA_ASSISTANT flag, but this can be relaxed
later on, as the detached editor can be used on its own.
- Detached editor is only supported in regular fields and on the creator panel. It is not supported yet for conditional styles, due to preview limitations.
- Old code for the assistant was removed completely, as it was only a temporary solution, but the AI conversation part was copied to the new one.
- Prompting was not modified in this diff, it will be included in the follow-up with more test cases.
Test Plan: Added only new tests; existing tests should pass.
Reviewers: JakubSerafin
Reviewed By: JakubSerafin
Differential Revision: https://phab.getgrist.com/D3863
Summary:
- Excel exports were awfully memory-inefficient, causing occasional docWorker
crashes. The fix is to use the "streaming writer" option of ExcelJS
https://github.com/exceljs/exceljs#streaming-xlsx-writercontents. (Empirically
on one example, max memory went down from 3G to 100M)
- It's also CPU intensive and synchronous, and can block node for tens of
seconds. The fix is to use a worker-thread. This diff uses "piscina" library
for a pool of threads.
- Additionally, adds ProcessMonitor that logs memory and cpu usage,
particularly when those change significantly.
- Also introduces request cancellation, so that a long download cancelled by
the user will cancel the work being done in the worker thread.
Test Plan:
Updated previous export tests; memory and CPU performance tested
manually by watching output of ProcessMonitor.
Difference visible in these log excerpts:
Before (total time to serve request 22 sec):
```
Telemetry processMonitor heapUsedMB=2187, heapTotalMB=2234, cpuAverage=1.13, intervalMs=17911
Telemetry processMonitor heapUsedMB=2188, heapTotalMB=2234, cpuAverage=0.66, intervalMs=5005
Telemetry processMonitor heapUsedMB=2188, heapTotalMB=2234, cpuAverage=0, intervalMs=5005
Telemetry processMonitor heapUsedMB=71, heapTotalMB=75, cpuAverage=0.13, intervalMs=5002
```
After (total time to server request 18 sec):
```
Telemetry processMonitor heapUsedMB=109, heapTotalMB=144, cpuAverage=0.5, intervalMs=5001
Telemetry processMonitor heapUsedMB=109, heapTotalMB=144, cpuAverage=1.39, intervalMs=5002
Telemetry processMonitor heapUsedMB=94, heapTotalMB=131, cpuAverage=1.13, intervalMs=5000
Telemetry processMonitor heapUsedMB=94, heapTotalMB=131, cpuAverage=1.35, intervalMs=5001
```
Note in "Before" that heapTotalMB goes up to 2GB in the first case, and "intervalMs" of 17 seconds indicates that node was unresponsive for that long. In the second case, heapTotalMB stays low, and the main thread remains responsive the whole time.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3906
Summary:
Choice/Reference List editor wasn't clearing itself when it received an empty string. It led
to a bug on the Card widget where pressing those keys resulted in the same behavior as
pressing Enter - it just opened the editor.
Grid view has it's own implementation for those keys, so it wasn't affected.
Test Plan: Added new test.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3908
Summary:
DateTime columns had a blank timezone after xlsx imports because the
timezone was not included in the column type. We now append the
document's timezone to the type of all imported DateTime columns.
Test Plan: Server test.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3896
Summary:
Migrations were failing in snapshots due to the sandbox no longer
being started in snapshots. We now start up an instance of the
sandbox whenever there are migrations to run, and immediately shut
it down on completion.
Test Plan: Server test.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3898
Summary:
Now that webhook payload delivery can be done using a proxy,
it may be desirable to no longer require a set of `ALLOWED_WEBHOOK_DOMAINS`.
This diff allows this variable to be set to `*`. With this setting,
any domain, and both `http` and `https` protocols will now be accepted.
Another possibility would be to default to unchecked
behavior if `ALLOWED_WEBHOOK_DOMAINS` is not set. But this would
introduce a new kind of vulnerability to unconfigured Grist
installations.
Test Plan: switched a test from naming a domain to using `*`
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3903
Summary:
Grist recently stopped working on Firefox on iOS. The cause turns out an uncaught error, which is reported as an unhelpful "Script Error", but the act of reporting it causes additional errors, leading to an infinite loop and an unusable browser tab.
Firefox-iOS is to blame, but a workaround is preventing a flood of "Script Error" messages. Specifically, we report only the first of these, and only to the server, suppressing the user-visible toast.
Test Plan: Tested manually on Firefox on iOS. Added a test case, and improve other tests of uncaught errors.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3902
Summary:
Tutorials are now hidden by default in grist-core and grist-ee, and can
be re-enabled via a new env variable, GRIST_UI_FEATURES, which accepts
a comma-separated list of UI features to enable.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3885
Summary:
Also:
- Move ProxyAgent to from app/server/utils to app/server/lib, which is
the more usual place for such classes.
- Refactor a helper (delayAbort) that node was reporting a leak in.
Test Plan: Added a test case, and tested manually.
Reviewers: JakubSerafin
Reviewed By: JakubSerafin
Subscribers: JakubSerafin, paulfitz
Differential Revision: https://phab.getgrist.com/D3897
Summary:
Due to a mishap, two distinct migrations with the same migration
number were introduced into Grist. This diff reconciles them as
best we can, by adding another migration to make sure both desired
changes have run (and running them if not).
Test Plan:
updated a test; checked manually that documents
with different 38 migrations are handled as expected.
Reviewers: georgegevoian, jarek
Reviewed By: georgegevoian, jarek
Differential Revision: https://phab.getgrist.com/D3895
Summary:
On supported browsers, the new context menu commands work exactly as they do
via keyboard shortcuts. On unsupported browsers, an unavailable command
modal is shown with a suggestion to use keyboard shortcuts instead.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3867
Summary:
This adds a UI panel for managing webhooks. Work started by Cyprien Pindat. You can find the UI on a document's settings page. Main changes relative to Cyprien's demo:
* Changed behavior of virtual table to be more consistent with the rest of Grist, by factoring out part of the implementation of on-demand tables.
* Cell values that would create an error can now be denied and reverted (as for the rest of Grist).
* Changes made by other users are integrated in a sane way.
* Basic undo/redo support is added using the regular undo/redo stack.
* The table list in the drop-down is now updated if schema changes.
* Added a notification from back-end when webhook status is updated so constant polling isn't needed to support multi-user operation.
* Factored out webhook specific logic from general virtual table support.
* Made a bunch of fixes to various broken behavior.
* Added tests.
The code remains somewhat unpolished, and behavior in the presence of errors is imperfect in general but may be adequate for this case.
I assume that we'll soon be lifting the restriction on the set of domains that are supported for webhooks - otherwise we'd want to provide some friendly way to discover that list of supported domains rather than just throwing an error.
I don't actually know a lot about how the front-end works - it looks like tables/columns/fields/sections can be safely added if they have string ids that won't collide with bone fide numeric ids from the back end. Sneaky.
Contains a migration, so needs an extra reviewer for that.
Test Plan: added tests
Reviewers: jarek, dsagal
Reviewed By: jarek, dsagal
Differential Revision: https://phab.getgrist.com/D3856
Summary: Changes to choices are now saved whenever focus leaves the editor.
Test Plan: Browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3879
* add support for conversational state to assistance endpoint
This refactors the assistance code somewhat, to allow carrying
along some conversational state. It extends the OpenAI-flavored
assistant to make use of that state to have a conversation.
The front-end is tweaked a little bit to allow for replies that
don't have any code in them (though I didn't get into formatting
such replies nicely).
Currently tested primarily through the runCompletion script,
which has been extended a bit to allow testing simulated
conversations (where an error is pasted in follow-up, or
an expected-vs-actual comparison).
Co-authored-by: George Gevoian <85144792+georgegevoian@users.noreply.github.com>
Summary:
- Webhooks form Triggers.ts should now use proxy if it's configured
- Proxy handling code separated to ProxyAgent.ts
- Tests for ProxyAgent
- Integration/API Tests for using Proxy in webhooks
- a bit of refactor - proxy test uses mostly the same codebase as DocApi.ts, but because last one if over 4000 lines long, I've put it into separated file, and extract some common parts (there is some duplicates tho)
- some cleanup in files that I've touched
Test Plan:
Manual test to check if proxy is used on the staging env
Automatic test checking if (fake) proxy was called
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3860
Summary:
- when grist table is exported, currency is check and introduced in cell format in the form of "[currency symbol] [value]" (for example: zł 10000, $ 5000) . It's not what some cultures should display currences, but it's close enought
- when no symbol is defined for the currency, currency 3 letters code is used instead
- when currency is unknown, we are falling back to "$"
Test Plan: - nbrowser test scenario added for that purpose, please check Currences.xlsx to see output format exported.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3886
Summary: Rules where removed when data in column was transformed.
Test Plan: Added new test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3883
Summary:
This ports the useful parts of the test/home tests to test/nbrowser (a chunk of the DocMenu tests were already covered).
I ripped out a chunk of test/browser code that is now no longer used.
I made a few changes to unrelated tests that happened to fail.
Test Plan: ported tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3888
Summary:
When a column is added the rename popup had a disabled save button.
Now we always show either:
Just "Close" if there are no changes.
"Save" and "Cancel" if there are changes.
Also, the description is trimmed when saved through the creator panel.
Test Plan: Added
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3872
Summary:
When clicking the logo in the top-left corner, or finishing a tutorial, we
now direct users to the site they last visited, if possible. If unknown, a
new redirect endpoint, /welcome/home, is used instead, which directs users
to a sensible location based on the number of sites they have.
Test Plan: Browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3878
Summary:
- When importing into a Ref column, use lookupOne() formula for correct previews.
- When selecting columns to import into a Ref column, now a Numeric column like
'Order' will produce two options: "Order" and "Order (as row ID)".
- Fixes exports to correct the formatting of visible columns. This addresses multiple bugs:
1. Formatting wasn't used, e.g. a Ref showing a custom-formatted date was still presented as YYYY-MM-DD in CSVs.
2. Ref showing a Numeric column was formatted as if a row ID (e.g. `Table1[1.5]`), which is very wrong.
- If importing into a table that doesn't have a primary view, don't switch page after import.
Refactorings:
- Generalize GenImporterView to be usable in more cases; removed near-duplicated logic from node side
- Some other refactoring in importing code.
- Fix field/column option selection in ValueParser
- Add NUM() helper to turn integer-valued floats into ints, useful for "as row ID" lookups.
Test Plan: Added test cases for imports into reference columns, updated Exports test fixtures.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3875
Summary:
This addresses two issues, differently:
- For a formula with leading whitespace, like " 1+1", it is stored as is, but
is fixed to work (it should be valid Python, and whitespace is only stripped out
at parsing time to avoid intentation errors caused by the way it gets parsed)
- For a formula with a leading equals-sign ("="), it is stripped out on the
client side before the formula is stored. Grist documentation uses leading
"=" to indicate formulas (because UI shows an "=" icon), and Excel formulas
actually contain the leading "=", so it is a common mistake to include it.
Test Plan: Added new test cases
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3873
Summary:
This finishes porting the bulk of tests relying on the old selenium pseudo-promise manager. Here the remaining ones that I know of:
* There are some tests written in typescript at test/browser/*.test.ts (I've olny been looking at the older *.test.js ones so far)
* There are some tests in test/home that run a slightly different way
First version of this diff just had renames, so for reviewing it is likely easiest to switch to a comparison of that version with the final version.
I switched from using test.(describe/it/...) to straight mocha since the indirection was confusing mocha-webdriver's debug functions (e.g. it was misnaming screenshots). That resulted in a lot of noise in the diff, sorry!
Some tests fail in headless operation in modern chrome for what looks to be the same reason as https://github.com/gristlabs/grist-core/pull/490 (some copy/paste nuance or change).
Test Plan: test porting
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3862
Summary: Column description and new renaming popup for the GridView.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3838
Summary:
Includes the following changes:
* Adds "Click to expand" hover tooltip to all images
* Adds support for minimize/maximize by double clicking tutorial popup header
* Add New menu (and all other popups) should now persist when user moves tutorial popup
* Preserves scrollbar position when minimizing and maximizing tutorial popup
* Formula cell editor (and other elements) should now be stacked under tutorial
Test Plan: Browser and manual tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3864
Summary: Extending the time tests wait for the confirmation popup in Fork tests.
Test Plan: Existing
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3870
Summary:
Support height adjustment for tutorial popup, also change the way we calculate
delta for the movement to make it follow the cursor more smoothly.
Test Plan: Added
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: JakubSerafin
Differential Revision: https://phab.getgrist.com/D3858
Summary:
History wasn't being replaced in some cases, which was
causing a bug where trying to leave a tutorial fork via the
browser's back button would navigate back to the trunk, and
trigger forking again. This effectively made it impossible to
leave a tutorial.
Also adds support for specifying custom CSS classes for
tutorial Markdown images.
Test Plan: Browser test.
Reviewers: JakubSerafin
Reviewed By: JakubSerafin
Differential Revision: https://phab.getgrist.com/D3866
Summary: Stop opening the creator panel on chart or custom widgets. Only switch tabs when panel is open
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3868
* feat: allow simple click on certain cases for editing field in cardView
* remove empty line
* test fix : add trick to limit the context of event
* desable the simple click behavior on widget card in mobile
* Delete timings.txt
* Delete xunit.xml
* clean-feat(Simple click on Card): use field type and id
* ignore local testing files
* codeStyle(single click on card): rename var + move util function in DetailView.prototype
* remove unused var
* CIFix(SingleClickOnCard): escape of field editor before close test
Summary:
When a chart page/section is added and the creator panel is already open, focus is set to a "table" tab.
When a custom view is added as a page/section, the same thing happens, but there is also a behavioral tooltip shown
for the custom URL.
Test Plan: Updated and added.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3857
Summary:
Ports more test/browser tests from *.test.js (run using an old selenium setup) to *.ntest.js (run using newer setup).
Weird test failures happened due to a change in timing. Eventually tracked in down to billing changes in one test suite resulting in reloads in another test suite, since it turns out redis pub/sub channels are not scoped to the redis database specified in REDIS_URL, but are global:
https://redis.io/docs/manual/pubsub/#database--scoping.
Test Plan: Ported tests should run and pass
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3844
Summary:
The substring for the anchor link row id was missing a leading ".", which was
a problem because tutorial fork ids would sometimes contain the substring.
Test Plan: N/A
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3853
Summary: Fixing the bug by setting viewInstance on the temp viewsection record
Test Plan: Added
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3848
This updates the expected count of languages in a localization test
to omit translations that have been started but are not yet ready
to be offered to users by default.
Summary:
Adds a new endpoint to update webhook.
Perform some refactoring to allow code reuse from endpoint allowing to _subscribe and _unsubscribe webhooks.
One aspect of webhook is that url are stored in the home db while the rest of the fields (tableRef, isReadyColRef, ...) are stored in sqlite. So care must be taken when updating fields, to properly rollback if anything should fail.
Follow up diff will bring UI to edit webhook list
Test Plan: Updated doc api server tests
Reviewers: jarek
Reviewed By: jarek
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3821
Summary:
In certain scenarios, it's possible for a Grist tour and a tip that kicks off
an easter egg to be shown on screen at the same time. To deconflict, we
now check for the presence of one or the other and avoid showing both.
Test Plan: Browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3840
Summary:
This is to prevent too many popups from being shown on screen at any
given moment.
Test Plan: Tested manually.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3841
Summary:
For now only html stub and docList adjustement for showing a tutorial card.
It will be used in future diffs after tutorial implementation.
Test Plan: Manual
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3750
Summary:
We have an important batch of old browser tests that depend on a pseudo-promise manager selenium's node client used to have. That manager was dropped in v4 (see changelog https://github.com/SeleniumHQ/selenium/blob/trunk/javascript/node/selenium-webdriver/CHANGES.md).
I tried porting some wholesale to our newer style of writing tests, and it is a bit of a bear (and would also be hard work to review). So instead I tried something else: remapping the `webdriverjq` implementation to work with mocha-webdriver. This works pretty well. Some API differences are hard to reconcile, but so far most test code just needs async/await changes to port, meaning a lot less thinking, and probably easier to review overall.
The important property of the ports tests is that they no longer import or require `selenium-webdriver`. mocha-webdriver depends on selenium-webdriver, but a newer version.
I haven't tried dealing with types, since that doesn't matter much - these tests aren't under active development, they are just important for preventing regressions.
Follow up work would be porting the remainder of the tests which, while a slog, I'm hoping is no longer a quagmire. Once the tests are ported, I'd propose moving them to `core`.
Test Plan: Test porting
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3825
Summary:
The feature is behind a flag GRIST_FORMULA_ASSISTANT (must be "true"). But can be enabled in the
developer console by invoking GRIST_FORMULA_ASSISTANT.set(true).
Keys can be overriden in the document settings page.
Test Plan: For now just a stub test that checks if this feature is disabled by default.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3815
Summary:
The previous implementation for collapsing sections involved disposing of a view instance (Grid or Chart component). This caused numerous bugs with
linking sections as the implementation is located in the BaseView.js. Now the view instance is kept and attached to a dom in a hidden div, so it can respond
and function as a normal rendered section. It is also passed from between collapsed and main layout, when sections are dragged or moved using section's
menu commands (`collapse` and `add to main page`)
It also implies that the ViewLayout must still be rendered when a section is maximized (as it is responsible for the view instance), so the dom, and
some logic for rendering it, had to be changed.
Test Plan: New and updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3826
Summary:
Documents can now be flagged as tutorials, which causes them to display
Markdown-formatted slides from a special GristDocTutorial table. Tutorial
documents are forked on open, and remember the last slide a user was on.
They can be restarted too, which prepares a new fork of the tutorial.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3813
Summary:
Porting script that run an evaluation against our formula dataset.
To test you need an openai key (see here: https://platform.openai.com/)
or hugging face (it should work as well), then checkout the branch and run
`OPENAI_API_KEY=<my_openai_api_key> node core/test/formula-dataset/runCompletion.js`
Test Plan:
Needs manually testing: so far there is no plan to make it part of CI.
The current score is somewhere around 34 successful prompts over a total of 47.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3816
Summary:
Usage is simply to call `overflowTooltip()` with no arguments, as an argument
to an element whose text may overflow. On 'mouseenter', it'll check for
overflow and show the element's .textContent in a tooltip.
- Added for long table names in the widget picker (Add Page, Add Widget to Page).
- Added for long page names in the left-panel list of pages.
Test Plan: Added test cases for the new overflow tooltips
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3814
Summary:
Fixes a bug when in a linked widget, the automatic reference wasn't being set
for a new record if attachment is the first thing that gets added to the
record.
- Move handling of 'setCursorPos' pseudo-command to GristDoc to support cross-section switching (relevant when moving attachment into a cell of a non-active page widget)
- Modernize code for AttachmentsWidget slightly (better typings, css conventions)
- Change the fix in https://phab.getgrist.com/D3796 from using isolate to using different z-index values, to avoid a change in the look of the cursor on Attachment cells.
Test Plan: Added a test case for what's possible to test with webdriver.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3811
This sets up a framework for running tests in parallel.
It increases the total time taken (since some steps are
repeated) but reduces the turn-around time significantly
overall.
The main objective is to make it possible to release more
test batches to grist-core without bringing CI to a crawl.
The clever little test/split-test.js script is from the
Grist Labs mono-repo and is Dmitry's work.
I considered doing the build in one job, and copying
it to test jobs, since it feels wasteful to repeat it.
That may be worth trying, especially if we start getting
jobs backing up (total concurrent Linux jobs on free plan
is quoted at 20).
It might also be worth looking at doing some tests in
parallel on the same worker, perhaps using the relatively
new MOCHA_WORKER_ID feature, since the tests are often not
actually CPU or I/O bound.
Summary:
A feature that allows minimizing widgets on the ViewLayout.
- Code in ViewLayout and Layout hasn't been changed. Only some methods or variables were made public, and some events are now triggered when a section is dragged.
- Widgets can be collapsed or expanded (added back to the main area)
- Collapsed widgets can be expanded and shown as a popup
- Collapsed widgets support drugging, reordering, and transferring between the main and collapsed areas.
Test Plan: New test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3779
Summary:
Fixes a test util that broke after a sync with grist-core, and upgrades
fixtures after a migration was added.
Test Plan: N/A
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3810
Summary:
Shows a placeholder flag icon for the language picker button when a country flag
isn't available.
The country flag icon is displayed on top of the placeholder icon. For countries
where an icon isn't available, the placeholder will then become visible.
This fixes a bug where no icon was shown for languages that didn't have a flag
icon available.
Test Plan: Tested manually.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3805
Summary:
Adds information about forks to the home db. This will be used
later by the UI to list forks of documents.
Test Plan: Browser and server tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3772
Summary:
- Preserving cursor position when linked state is removed.
- Moving linking tests to grist core.
- Disabling yarn offline mirror for grist-core. This helps testing grist-core when it is imported as a submodule.
- Moving one test for linked section from ReferenceColumns.ts to RightPanelSelectBy.ts.
Test Plan: Updated
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3795
Summary:
The coaching call popup wasn't being dismissed correctly in the
HomeIntro tests.
Test Plan: N/A
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3788
Summary:
The "Empty values last" options was breaking sort when multiple
columns are involved in the sort spec. Problem comes from wrong
handling of equals.
Diff fixes that issue and update test.
Test Plan: Updates nbrowser test
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3774
Summary:
New language selector on the Account page for logged-in users.
New icon for switching language for an anonymous user.
For anonymous users, language is stored in a cookie grist_user_locale.
Language is stored in user settings for authenticated users and takes
precedence over what is stored in the cookie.
Test Plan: New tests
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3766
Summary:
This diff implement new page for document settings which replaces the old modal settings.
Diff also adds a new `Settings + API` page item below tools in the left panel that link to that very doc settings page.
Test Plan: Updated existing tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3761
Summary:
New icon to expand an active section and show it as a popup (just like raw data views).
"Show raw data" popup couldn't be reused (as it is basically a different page), so now
we have two kinds of popups that look the same.
1. Raw data popup - to show an alien section on a page (a section from a different view). This is used by "Show raw data" button, it is basically a different page that shows an arbitrary section.
2. Layout popup - a popup generated by Layout.ts that basically hides every other section and adds an overlay effect to itself.
Other changes
- Layout.js was migrated to typescript
- "Show raw data" menu item was converted to link
Test Plan: new tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3764
Summary:
Fixes deployment test failures that were caused by a test user
being deleted and recreated, which consequently reset their
preferences and made popups and tips always appear.
Test Plan: N/A
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3768
Summary:
- Introduces a fictitious "*SPECIAL:SchemaEdit" resource in UI only.
- Hides "S" bit for the default rule section.
- Shows a checkbox UI similar to other checkboxes, with an additional
dismissable warning.
Test Plan: Added a browser test
Reviewers: paulfitz, georgegevoian
Reviewed By: paulfitz, georgegevoian
Differential Revision: https://phab.getgrist.com/D3765
Summary:
Date filter was not taking timezone correclty into account, which was
causing to wrong-inclusion and wrong-exclusion of dates near the
bounds.
Diff fixes that, it also bring little refactoring that hopefully clarifies things a little.
Test Plan: Includes brand new test for `app/common/ColumnFilterFunc`.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3763
There was no script for updating typescript schema information after
a python-based document migration. Moving one in here, along with its
test. Tweaked the code slightly to work with grist-core's directory
structure. Also fixed a formatting error in mocha calls that was resulting
in some root tests not running.
Summary: In Access rules page item, adds “…” buttons that shows a menu of users to view-as:
Test Plan: Include new nbrowser test
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3751
Summary:
By default editor inherits permission for a new document or workspace.
Now editor is added explicitly as an owner of a new doc or workspace.
Test Plan: Updated
Reviewers: georgegevoian, paulfitz
Reviewed By: georgegevoian, paulfitz
Subscribers: dsagal, paulfitz
Differential Revision: https://phab.getgrist.com/D3734
Summary:
By default the fallback contains the prefix, which doesn't
work well with a key based translations. Now makeT helper will fallback
to the passed key (without a prefix).
Test Plan: Added new client test
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3758
Summary:
On mobile view not all rows are rendered when a section is expanded.
Scrolly component calculates height of the GridView too soon (before animation is
completed). With this change on mobile view we always take the screen height for
calculation.
A similar bug was on Card List, where cards were squeezed and their height was
calculated to soon.
Test Plan: Added test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3748
Summary: Adds a search input at the top of columns dropdown. Start typing in the search bar filters the list of column (matching occurences should work similarly as the autocomplete dropdown on Choice column).
Test Plan: Include tests.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3738
Summary: This test has been added to core. This diff also updates some storage-related code in monorepo to match changes in core.
Test Plan: moving test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3746
Summary:
for users who don't automatically have deep rights
to the document, provide them with attachment metadata only
for rows they have access to. This is a little tricky to
do efficiently. We provide attachment metadata when an
individual table is fetched, rather than on initial document
load, so we don't block that load on a full document scan.
We provide attachment metadata to a client when we see that
we are shipping rows mentioning particular attachments,
without making any effort to keep track of the metadata they
already have.
Test Plan: updated tests
Reviewers: dsagal, jarek
Reviewed By: dsagal, jarek
Differential Revision: https://phab.getgrist.com/D3722
Summary:
Staging tests were failing because a popup tip was shown when
anonymous; these tips aren't dismissed by testing hooks during
deployment tests, and so they need to be explicitly dismissed by
tests (in this case, Forks.ts).
grist-core tests were failing because a test suite was accidentally
included that shouldn't have been (BehavioralPrompts.ts).
Test Plan: N/A
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3741
Summary:
Also fixes a deployment test failure in staging due to a test util
throwing a (harmless) error.
Test Plan: Tested manually.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3740
Summary:
Adds a new category of popups that are shown dynamically when
certain parts of the UI are first rendered, and a free coaching
call popup that's shown to users on their site home page.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3706
Summary:
Checking SCHEMA_EDIT permission when user wants to
update document's name.
Test Plan: New test
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3733
Summary:
Implements the new date filtering panel. Design results from long
discussion between: Alex, Anais, Cyprien and Dmitry.
Test environment: https://grist-new-date-range-filter.fly.dev/
Test Plan: Include various new tests.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3720
Summary:
- New /webhooks event that lists all webhooks in a document (available for owners),
- Monitoring webhooks usage and saving it in memory or Redis,
- Loosening _usubscribe API endpoint, so that the information returned from the /webhook endpoint is enough to unsubscribe,
- Owners can remove webhook without the unsubscribe key.
The endpoint lists all webhooks that are registered in a document, not just webhooks from a single table.
There are two status fields. First for the webhook, second for the last request attempt.
Webhook can have 5 statuses: 'idle', 'sending', 'retrying', 'postponed', 'error', which roughly describes what the
sendLoop is currently doing. The 'error' status describes a situation when all request attempts failed and the queue needs
to be drained, so some requests were dropped.
The last request status can only be: 'success', 'failure' or 'rejected'. Rejected means that the last batch was dropped because the
queue was too long.
Test Plan: New and updated tests
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3727
Summary:
Adds a new UI for writing access rule memos.
Migrates old memos (written as Python comments) to the new UI.
Test Plan: Browser and migration tests.
Reviewers: jarek, dsagal
Reviewed By: jarek
Subscribers: dsagal, paulfitz
Differential Revision: https://phab.getgrist.com/D3726
Summary:
Creating an API endpoint to cancel any queued webhook messages from
a document.
Test Plan: Updated
Reviewers: paulfitz, georgegevoian
Reviewed By: paulfitz, georgegevoian
Differential Revision: https://phab.getgrist.com/D3713
Summary:
- Handle the possibility that any access to localStorage causes error.
- Move getStorage() and getSessionStorage() safe functions to a separate file.
- Use these safe functions in more places.
Test Plan:
Added a test case, using a webdriver instance that blocks third-party cookies,
to enforce third-party restrictions. Added to gristUtil a way to override the
webdriver instance.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3719
Summary:
Guest editors added to a document were able to remove it. This limits this permission
by allowing only owners of a doc to delete it.
Test Plan: Updated
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, anaisconce
Differential Revision: https://phab.getgrist.com/D3708
Summary:
The sort and filter UI now has a more unified UI, with similar
capabilities that are accessible from different parts of Grist.
It's now also possible to pin individual filters to the filter bar,
which replaces the old toggle for showing all filters in the
filter bar.
Test Plan: Various tests (browser, migration, project).
Reviewers: jarek, dsagal
Reviewed By: jarek, dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3669
Summary:
View as feature uses example.com emails for simulated users. This can break
when such a user already exists in the home db. Here we pretend that these users
don't exist during ACL checks.
Test Plan: Updated and existing
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3705
Summary:
Fix for layout editor when fields are removed using the creator panel. Layout editor
wasn't updated properly when "layoutSpecs" were changed.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3700
Summary:
When a grid is scrolled, and then data is changed (due to click in a linked section), some
records are not rendered, or position of the scroll container is corrupted
Test Plan: Added
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3661
Summary:
The /replace endpoint was built with home-level access control in mind. Updates needed:
* Only an owner can now replace a document. Only owners are permitted to change granular access rules, and a document replacement could change granular access rules.
* For the document being substituted in: the user must have complete access to view all material within it.
Test Plan: extended test
Reviewers: georgegevoian, dsagal
Reviewed By: georgegevoian, dsagal
Differential Revision: https://phab.getgrist.com/D3694
Summary:
Allow requests from untrusted origins instead of returning an error, but don't allow credentials (Cookie header) or API keys (Authorization header).
Allow setting the header `Content-type: application/json` as an alternative to `X-Requested-With: XMLHttpRequest` to make it easier for clients to make POST/PUT/PATCH/DELETE requests without authentication.
Discussion: https://grist.slack.com/archives/C0234CPPXPA/p1666355281535479
Test Plan: Added and updated DocApi tests. Tested manually how this affects requests made from a browser.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz, dsagal
Differential Revision: https://phab.getgrist.com/D3678
The server.restart functionality in tests uses `TEST_CLEAN_DATABASE`. Make sure the way it wants to set this variable takes precedence over the environment.
Add some more `waitForServer`s near clicks.
Co-authored-by: George Gevoian <george@gevoian.com>
Summary:
When a page is hidden, all its nested pages are shown as children of
a different page that happens to be before (as in pagePos) that page.
This diff shows those pages as CENSORED.
Test Plan: Updated
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3670
Summary:
The renaming is to clarify that the operation is more of a schema change, than
a data update. In particular, this is to reduce confusion why it is allowed to
anyone having Structure permission in Access Rules.
The resetting of type is a separate but related cleanup. Changing type to Any
returns the column to initial state, letting it guess type from new data, and
making it easy to enter a formula. It applies also to the "Clear and reset"
option in the Creator Panel.
Test Plan: Updated tests, added a check for type changing to Any.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3685
Summary: Background for frozen columns was set to transparent in recent PR, this diff is reverting it.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3680
Summary:
Moving bulk of nbrowser tests to core. Some tests were split and only part of them were moved.
Tests that are left are either: not suitable for grist-core (like billing) or are failing during browser tests (are not reliable).
Four fixtures directory (uploads, docs, exports-csv/excel) where completely moved to grist-core and are linked as folders.
Those changes allows to add an nbrowser test in grist-core or in the main test folder without any need to link it or link a fixture document.
Other changes:
- testrun.sh has been modified, now it runs tests from both folders (test and core/test),
- TestServer used in grist-core is now adding sample orgs and users (kiwi and others),
Test modified
- SelectionSummary: now it is run on a bigScreen, it was failing randomly
- Billing.ts: relative paths were used
- DateEditor: added waitForServer - it was failing in browser mode
- FrozenColumns, ImportFromGDrive, Printing: updated import paths
- UserManager.ts: was split into two parts (it assumed limited products)
- ViewLayoutResize.ts: this test is still in main repo, it is still failing in browser mode tests
Test Plan: Existing
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, paulfitz
Differential Revision: https://phab.getgrist.com/D3664
Summary:
After introducing multi columns operation, color picker
could save a cell style for a wrong column, if the save operation
was triggered by user clicking on one of the cells.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3668
Summary:
New shortcuts for removing and adding rows.
For adding a row we now have Mod+(Shift)+Enter
For removing rows we now have Mod+Delete/Mod+Backspace
Before removing rows, the user is prompted to confirm, this prompt
can be dismissed and this setting can be remembered. User needs
to confirm only when using shortcut.
Old shortcuts are still active and shows information about this change.
This information is shown only once, after this shortcuts have default
behavior (zooming).
New users don't see this explanation.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3655
Summary:
Editing data selection would sometimes cause columns to be hidden in the updated view. A
missing conditional was the culprit: generally, field visibility shouldn't be modified after the view is
updated, but we make an exception for charts to keep certain fields visible or hidden between
updates, so that chart configuration doesn't change too significantly and cause unexpected
data to be displayed. This special behavior for charts was erroneously being applied to non-charts
as well.
Also, when no columns were visible in a view, opening the row menu would cause an error to be
thrown. A loop was inadvertently using null control variables - an explicit check for non-null loop
variables was added, which skips the loop when no columns are visible.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3650
Summary:
Fixes bug that prevented the table of a reference list from being
changed in the column transform UI.
Test Plan: Browser test.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3666
Summary:
First iteration for comments system for Grist.
- Comments are stored in a generic metatable `_grist_Cells`
- Each comment is connected to a particular cell (hence the generic name of the table)
- Access level works naturally for records stored in this table
-- User can add/read comments for cells he can see
-- User can't update/remove comments that he doesn't own, but he can delete them by removing cells (rows/columns)
-- Anonymous users can't see comments at all.
- Each comment can have replies (but replies can't have more replies)
Comments are hidden by default, they can be enabled by COMMENTS=true env variable.
Some things for follow-up
- Avatars, currently the user's profile image is not shown or retrieved from the server
- Virtual rendering for comments list in creator panel. Currently, there is a limit of 200 comments.
Test Plan: New and existing tests
Reviewers: georgegevoian, paulfitz
Reviewed By: georgegevoian
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3509
Summary:
Creator panel allows now to edit multiple columns at once
for some options that are common for them. Options that
are not common are disabled.
List of options that can be edited for multiple columns:
- Column behavior (but limited to empty/formula columns)
- Alignment and wrapping
- Default style
- Number options (for numeric columns)
- Column types (but only for empty/formula columns)
If multiple columns of the same type are selected, most of
the options are available to change, except formula, trigger formula
and conditional styles.
Editing column label or column id is disabled by default for multiple
selection.
Not related: some tests were fixed due to the change in the column label
and id widget in grist-core (disabled attribute was replaced by readonly).
Test Plan: Updated and new tests.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3598
Summary:
- DocApi test for Allowed Origin was using a home server endpoint
- Fixing waitForServer, as gristApp can be unavailable for a moment when browser is refreshed
- Fixing MergedOrgs tests typing issue
Test Plan: Updated
Reviewers: cyprien, paulfitz
Reviewed By: cyprien, paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3648
Summary:
Adds a cell selection summary to grid view that shows either a count or
sum of all the selected values. Implementation was done by Dmitry.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Subscribers: paulfitz, dsagal, jarek
Differential Revision: https://phab.getgrist.com/D3630
Summary:
This diff adds a new `BulkAddOrUpdateRecord` user action which is what is sounds like:
- A bulk version of the existing `AddOrUpdateRecord` action.
- Much more efficient for operating on many records than applying many individual actions.
- Column values are specified as maps from `colId` to arrays of values as usual.
- Produces bulk versions of `AddRecord` and `UpdateRecord` actions instead of many individual actions.
Examples of users wanting to use something like `AddOrUpdateRecord` with large numbers of records:
- https://grist.slack.com/archives/C0234CPPXPA/p1651789710290879
- https://grist.slack.com/archives/C0234CPPXPA/p1660743493480119
- https://grist.slack.com/archives/C0234CPPXPA/p1660333148491559
- https://grist.slack.com/archives/C0234CPPXPA/p1663069291726159
I tested what made many `AddOrUpdateRecord` actions slow in the first place. It was almost entirely due to producing many individual `AddRecord` user actions. About half of that time was for processing the resulting `AddRecord` doc actions. Lookups and updates were not a problem. With these changes, the slowness is gone.
The Python user action implementation is more complex but there are no surprises. The JS API now groups `records` based on the keys of `require` and `fields` so that `BulkAddOrUpdateRecord` can be applied to each group.
Test Plan: Update and extend Python and DocApi tests.
Reviewers: jarek, paulfitz
Reviewed By: jarek, paulfitz
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3642
Summary:
When running as a deployment test, there are some timing issues
in the DuplicateDocument test. These tweaks appear to resolve
them.
Test Plan: checked with staging_deployment test target
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3638
Summary:
This endpoint has started to fail when called between a pair
of doc workers. The simplest fix is to simply remove it, it serves no
purpose.
Test Plan: added basic deployment test
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3636
Summary:
FreeTeam test was overlapping with DuplicateDocument test.
It was leaving some documents in personal workspace.
Test Plan: Updated tests
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3632
XLSX export of active view / table
Co-authored-by: Louis Delbosc <louis.delbosc.prestataire@anct.gouv.fr>
Co-authored-by: Vincent Viers <vincent.viers@beta.gouv.fr>
Summary: Adding title and buttons for empty workspace screen.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3622
Summary:
The selection was not cleared after removing rows, and it still
kept deleted rows ids. Removing selected rows once again
produced an assertion error.
Also, the selection is now cleared when columns are removed.
Test Plan: Updated tests
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3623
Summary:
upgrade typeorm version, so Grist can run against newer versions of postgres.
Dusted off some old benchmarking code to verify that important queries don't get slower. They don't appear to, unlike for some intermediate versions of typeorm I tried in the past.
Most of the changes are because `findOne` changed how it interprets its arguments, and the value it returns when nothing is found. For the return value, I stuck with limiting its impact by emulating old behavior (returning undefined rather than null) rather than propagating the change out to parts of the code unrelated to the database.
Test Plan: existing tests pass; manual testing with postgres 10 and 14
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3613
Summary:
Highlighting wasn't working correctly for the new
normalized search for autocomplate widgets.
Test Plan: Existing tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3602
Summary:
Fixes some bugs involving intervals, and updates RandomizedTimer to support both fixed and
randomized delays, and to better handle async callbacks.
* Fixed a bug where Throttle would queue up many pidusage calls due to the use of
setInterval, and the async nature of the calls.
* Fixed a but where RandomizedTimer (now just Interval) would not be disabled in
ActiveDoc on doc shutdown if initialization had not yet settled.
Test Plan: Tested manually.
Reviewers: jarek, dsagal
Reviewed By: jarek, dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3604
Summary:
Show raw data will now open a popup with
raw section instead of redirecting to raw data page.
Adding new anchor link type "a2" that is able to open
any section in a popup on a current view.
Not related:
Fixing highlightMatches function, after merging core PR.
Test Plan: Updated tests
Reviewers: alexmojaki, georgegevoian
Reviewed By: alexmojaki, georgegevoian
Subscribers: georgegevoian, alexmojaki
Differential Revision: https://phab.getgrist.com/D3592
Summary:
- Moved /test/client and /test/common to core.
- Moved two files (CircularArray and RecentItems) from app/common to core/app/common.
- Moved resetOrg test to gen-server.
- `testrun.sh` is now invoking common and client test from core.
- Added missing packages to core's package.json (and revealed underscore as it is used in the main app).
- Removed Coord.js as it is not used anywhere.
Test Plan: Existing tests
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3590
Summary: This particular test fails often enough to be annoying but not often enough to be worrying. It's not clear why, but it seems like a race condition involving redis. Fixing the test 'properly' seems hard and not worth the effort. Looking at the past 20 Jenkins builds, I found this test failing once. If we assume that the probability of failing is 1/20 (it's probably less since the test actually runs 3 times with different server configurations) then the probability of failing 3 times independently is (1/20)^3 = 1/8000, so `this.retry(3)` seems like a good enough solution. See also: https://grist.slack.com/archives/C0234CPPXPA/p1652909955773049
Test Plan: Manually made the test fail randomly 90% of the time, in which case `this.retries(3)` was not usually enough to prevent failures, but `this.retries(300)` was.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3595
Summary: Removing duplicated tests that were moved to grist-core.
Test Plan: Existing tests
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3588
Summary:
Conditional formatting can now be used for whole rows.
Related fix:
- Font styles weren't applicable for summary columns.
- Checkbox and slider weren't using colors properly
Test Plan: Existing and new tests
Reviewers: paulfitz, georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3547
Summary:
Removing welcome tests for viewers from HomeIntro tests, as
they are run against external servers.
Test Plan: moved tests to the FreeTeam test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3563
Summary:
Adding intro for a viewer on a teamsite.
Showing upgrade button for owners only.
Test Plan: new test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3557
Summary: `nbrowser/CopyPaste` was failing on Mac. Diff fixes that issue.
Test Plan: Update `test/nbrowser/CopyPaste` and `core/test/server/customUtil.ts`.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3552
Summary:
Adds the new personal plan as a product that will be available
in the future. Can be enabled along with other plan-related via
an environment variable.
Test Plan: Browser tests and existing tests.
Reviewers: jarek
Reviewed By: jarek
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3533
Summary: [Select By] in the creator panel was bugged. It wasn't refreshed in some cases as the observable array that needed to be created seemed too complicated. This Diff recomputes this array when the user wants to change the selection.
Test Plan: added tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3541
Summary:
As reported in https://grist.slack.com/archives/C069RUP71/p1655316194602829, when a table is hidden by ACL, it was still showing as a blank 'ghost' option to select data when adding a widget:
{F55498}
The fix is simply to return `true` from `isHiddenTable` for empty table IDs, which indicate a table hidden by ACL. `TableRec.isHidden` is supposed to match this so I updated it too, and I cleaned up a tiny bit of other related code.
Test Plan: Extended `nbrowser/AccessRules1.ts` to test the data options when adding widgets.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3530
Summary:
Modifies CSS so that text overflow is handled by displaying an
ellipsis when raw table names exceed the width of their container.
Test Plan: Tested manually.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3524
Summary:
When adding a summary table widget to a page and using 'select by' in the add widget config (as opposed to in the right panel for an existing widget):
1. If an equivalent summary table already exists, use its referencing columns (if any) to construct link nodes. Previously the source table columns were being used instead, which could include referencing columns that don't have any equivalent in the summary table, and exclude referencing columns in the summary table.
2. If no such summary table exists yet, then keep using the source table columns, but only the selected groupby columns, and
3. After the summary table is created, correct the `linkTargetColRef` (which points to a source table column) to the corresponding column from the new summary table instead.
This fixes bugs which only appeared recently since 'select by' for a summary table previously involved no target columns.
Test Plan: Added two new tests to `nbrowser/SelectBySummaryRef`, and confirmed that they fail without the fixes to all three points above.
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3527
Summary:
A client error should no longer be thrown when filtering an empty Choice
or Choice List column.
Test Plan: Browser tests.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3528
Summary:
1. Log errors in `ActiveDoc.loadDoc` as errors, not just warnings, except for a common 'Cannot create fork' error caused by deployment tests.
2. Log the method name that had an error in `server/lib/Client.ts`.
Discussion: https://grist.slack.com/archives/CR8HZ4P9V/p1652364998893169
Following up on https://phab.getgrist.com/D3522
Test Plan: tested manually, particularly by running the nbrowser/Fork test that led to the initial noisy errors in Slack.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3525
Summary:
This adds rudimentary support for opening certain SQLite files in Grist.
If you have a file such as `landing.db` in Grist, you can convert it to Grist format by doing (either in monorepo or grist-core):
```
yarn run cli -h
yarn run cli sqlite -h
yarn run cli sqlite gristify landing.db
```
The file is now openable by Grist. To actually do so with the regular Grist server, you'll need to either import it, or convert some doc you don't care about in the `samples/` directory to be a soft link to it (and then force a reload).
This implementation is a rudimentary experiment. Here are some awkwardnesses:
* Only tables that happen to have a column called `id`, and where the column happens to be an integer, can be opened directly with Grist as it is today. That could be generalized, but it looked more than a Gristathon's worth of work, so I instead used SQLite views.
* Grist will handle tables that start with an uncapitalized letter a bit erratically. You can successfully add columns, for example, but removing them will cause sadness - Grist will rename the table in a confused way.
* I didn't attempt to deal with column names with spaces etc (though views could deal with those).
* I haven't tried to do any fancy type mapping.
* Columns with constraints can make adding new rows impossible in Grist, since Grist requires that a row can be added with just a single cell set.
Test Plan: added small test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3502
Summary:
Changes auto-generated summary table IDs from e.g. `GristSummary_6_Table1` to `Table1_summary_A_B` (meaning `Table1` grouped by `A` and `B`). This makes it easier to write formulas involving summary tables, make API requests, understand logs, etc.
Because these don't encode the source table ID as reliably as before, `decode_summary_table_name` now uses the summary table schema info, not just the summary table ID. Specifically, it looks at the type of the `group` column, which is `RefList:<source table id>`.
Renaming a source table renames the summary table as before, and now renaming a groupby column renames the summary table as well.
Conflicting table names are resolved in the usual way by adding a number at the end, e.g. `Table1_summary_A_B2`. These summary tables are not automatically renamed when the disambiguation is no longer needed.
A new migration renames all summary tables to the new scheme, and updates formulas using summary tables with a simple regex.
Test Plan:
Updated many tests to use the new style of name.
Added new Python tests to for resolving conflicts when renaming source tables and groupby columns.
Added a test for the migration, including renames in formulas.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3508
