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:
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:
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:
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:
- 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:
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:
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:
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
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:
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: 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:
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:
- 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:
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:
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 /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
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:
- 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:
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:
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
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: 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: `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:
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:
When a user requests to read the contents of an attachment, only allow the request if there exists a cell in an attachment column that contains the attachment and which they have read access to.
This does not cover:
* Granular write access for attachments. In particular, a user who can write to any attachment column should be considered to have full read access to all attachment columns, currently.
* Access control of attachment metadata such as name and format.
The implementation uses a sql query that requires a scan, and some notes on how this could be optimized in future. The web client was updated to specify the cell to check for access, and performance seemed fine in casual testing on a doc with 1000s of attachments. I'm not sure how performance would hold up as the set of access rules grows as well.
Test Plan: added tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3490
