Summary:
Firstly I just wanted some more consistency and less repetition in places where Documents are retrieved from the DB, so it's more obvious when code differs from the norm. Main changes for that part:
- Let HomeDBManager accept a `Request` directly and convert it to a `Scope`, and use this in a few places.
- `getScope` tries `req.docAuth.docId` if `req.params` doesn't have a docId.
I also refactored how `_createActiveDoc` gets the document URL, separating out getting the document from getting a URL for it. This is because I want to use that document object in a future diff, but I also just find it cleaner. Notable changes for that:
- Extracted a new method `HomeDBManager.getRawDocById` as an alternative to `getDoc` that's explicitly for when you only have a document ID.
- Removed the interface method `GristServer.getDocUrl` and its two implementations because it wasn't used elsewhere and it didn't really add anything on top of getting a doc (now done by `getRawDocById`) and `getResourceUrl`.
- Between `cachedDoc` and `getRawDocById` (which represent previously existing code paths) also try `getDoc(getScope(docSession.req))`, which is new, because it seems better to only `getRawDocById` as a last resort.
Test Plan: Existing tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3328
Summary:
Keep track of the number of API requests made for this document today in redis. Uses local caches of the count and the document so that usually requests can proceed without waiting for redis or the database.
Moved the free standing function apiThrottle to become a method to avoid adding another layer of request handler callbacks.
Test Plan: Added a DocApi test
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3327
Summary: This makes an equivalent of the /records REST endpoint available within custom widgets. For simple operations, it is compatible with https://github.com/airtable/airtable.js/. About half of the diff is refactoring code from DocApi that implements /records using applyUserActions, to make that code available in the plugin api.
Test Plan: added tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3320
Summary:
As designed in https://grist.quip.com/fZSrAnJKgO5j/Add-or-Update-Records-API
Current `POST /records` adds records, and `PATCH /records` updates them by row ID. This adds `PUT /records` to 'upsert' records, applying the AddOrUpdate user action. PUT was chosen because it's idempotent. Using a separate method (instead of inferring based on the request body) also cleanly separates validation, documentation, etc.
The name `require` for the new property was suggested by Paul because `where` isn't very clear when adding records.
Test Plan: New DocApi tests
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3251
Summary: Bug with 'records' endpoint that was treating 0 as null.
Test Plan: Modified tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3262
Summary:
- Adds a function `parseUserAction` for parsing strings in UserActions to `ValueParser.ts`
- Adds a boolean option `parseStrings` to use `parseUserAction` in `ActiveDoc.applyUserActions`, off by default.
- Uses `parseStrings` by default in DocApi (set `?noparse=true` in a request to disable) when adding/updating records through the `/data` or `/records` endpoints or in general with the `/apply` endpoint.
- Uses `parseStrings` for various actions in `ActiveDocImport`. Since most types are parsed in Python before these actions are constructed, this only affects references, which still look like errors in the import preview. Importing references can also easily still run into more complicated problems discussed in https://grist.slack.com/archives/C0234CPPXPA/p1639514844028200
Test Plan:
- Added tests to DocApi to compare behaviour with and without string parsing.
- Added a new browser test, fixture doc, and fixture CSV to test importing a file containing references.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3183
Summary:
Exports used to show generic message on error.
Adding error description to the message.
Test Plan: Updated tests
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3157
Summary:
The /assign endpoint checks if a document is on the desired worker
and moves it if not. This is never done under regular operation, but
is useful when quarantining a misbehaving document.
The endpoint was failing to operate correctly if the requester did
not have access to the document. This diff makes the endpoint
accessible through a /housekeeping route, using the same pattern as
the /force-reload endpoint.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3109
Summary:
Adding sort options for columns.
- Sort menu has a new option "More sort options" that opens up Sort left menu
- Each sort entry has an additional menu with 3 options
-- Order by choice index (for the Choice column, orders by choice position)
-- Empty last (puts empty values last in ascending order, first in descending order)
-- Natural sort (for Text column, compares strings with numbers as numbers)
Updated also CSV/Excel export and api sorting.
Most of the changes in this diff is a sort expression refactoring. Pulling out all the methods
that works on sortExpression array into a single namespace.
Test Plan: Browser tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: dsagal, alexmojaki
Differential Revision: https://phab.getgrist.com/D3077
Summary:
This verifies that all existing tests are capable of running under python3/gvisor, and fixes the small issues that came up. It does not yet activate python3 tests on all diffs, only diffs that specifically request them.
* Adds a suffix in test names and output directories for tests run with PYTHON_VERSION=3, so that results of the same test run with and without the flag can be aggregated cleanly.
* Adds support for checkpointing to the gvisor sandbox adapter.
* Prepares a checkpoint made after grist python code has loaded in the gvisor sandbox.
* Changes how `DOC_URL` is passed to the sandbox, since it can no longer be passed in as an environment variable when using checkpoints.
* Uses the checkpoint to speed up tests using the gvisor sandbox, otherwise a lot of tests need more time (especially on mac under docker).
* Directs jenkins to run all tests with python2 and python3 when a new file `buildtools/changelogs/python.txt` is touched (this diff counts as touching that file).
* Tweaks miscellaneous tests
- some needed fixes exposed by slightly different timing
- a small number actually give different results in py3 (removal of `u` prefixes).
- some needed a little more time
The DOC_URL change is not the ultimate solution we want for DOC_URL. Eventually it should be a variable that gets updated, like the date perhaps. This is just a small pragmatic change to preserve existing behavior.
Tests are run mindlessly as py3, and for some tests it won't change anything (e.g. if they do not use NSandbox). Tests are not run in parallel, doubling overall test time.
Checkpoints could be useful in deployment, though this diff doesn't use them there.
The application of checkpoints doesn't check for other configuration like 3-versus-5-pipe that we don't actually use.
Python2 tests run using pynbox as always for now.
The diff got sufficiently bulky that I didn't tackle running py3 on "regular" diffs in it. My preference, given that most tests don't appear to stress the python side of things, would be to make a selection of the tests that do and a few wild cards, and run those tests on both pythons rather then all of them. For diffs making a significant python change, I'd propose touching buildtools/changelogs/python.txt for full tests. But this is a conversation in progress.
A total of 6886 tests ran on this diff.
Test Plan: this is a step in preparing tests for py3 transition
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3066
Summary:
Adding validation for api /records endpoint, that checks if the json payload is valid.
Modifying POST /records endpoint to allow creating blank or partial records.
Test Plan: Updated tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3061
Summary:
Finishing imports now occurs in Node instead of the
data engine, which makes it possible to import into
on-demand tables. Merging code was also refactored
and now uses a SQL query to diff source and destination
tables in order to determine what to update or add.
Also fixes a bug where incremental imports involving
Excel files with multiple sheets would fail due to the UI
not serializing merge options correctly.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3046
Summary:
Tests DocApi endpoints _subscribe and _unsubscribe, including various bad inputs.
Tests that webhooks are sent to a test express server, with retrying on failure, filtered by event type, and waiting for isReadyColumn.
Test Plan: this
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3042
Summary:
See https://grist.quip.com/VKd3ASF99ezD/Outgoing-Webhooks
- 2 new DocApi endpoints: _subscribe and _unsubscribe, not meant to be user friendly or publicly documented. _unsubscribe should be given the response from _subscribe in the body, e.g:
```
$ curl -X POST -H "Authorization: Bearer 8fd4dc59ecb05ab29ae5a183c03101319b8e6ca9" "http://localhost:8080/api/docs/6WYa23FqWxGNe3AR6DLjCJ/tables/Table2/_subscribe" -H "Content-type: application/json" -d '{"url": "https://webhook.site/a916b526-8afc-46e6-aa8f-a625d0d83ec3", "eventTypes": ["add"], "isReadyColumn": "C"}'
{"unsubscribeKey":"3246f158-55b5-4fc7-baa5-093b75ffa86c","triggerId":2,"webhookId":"853b4bfa-9d39-4639-aa33-7d45354903c0"}
$ curl -X POST -H "Authorization: Bearer 8fd4dc59ecb05ab29ae5a183c03101319b8e6ca9" "http://localhost:8080/api/docs/6WYa23FqWxGNe3AR6DLjCJ/tables/Table2/_unsubscribe" -H "Content-type: application/json" -d '{"unsubscribeKey":"3246f158-55b5-4fc7-baa5-093b75ffa86c","triggerId":2,"webhookId":"853b4bfa-9d39-4639-aa33-7d45354903c0"}'
{"success":true}
```
- New DB entity Secret to hold the webhook URL and unsubscribe key
- New document metatable _grist_Triggers subscribes to table changes and points to a secret to use for a webhook
- New file Triggers.ts processes action summaries and uses the two new tables to send webhooks.
- Also went on a bit of a diversion and made a typesafe subclass of TableData for metatables.
I think this is essentially good enough for a first diff, to keep the diffs manageable and to talk about the overall structure. Future diffs can add tests and more robustness using redis etc. After this diff I can also start building the Zapier integration privately.
Test Plan: Tested manually: see curl commands in summary for an example. Payloads can be seen in https://webhook.site/#!/a916b526-8afc-46e6-aa8f-a625d0d83ec3/0b9fe335-33f7-49fe-b90b-2db5ba53382d/1 . Great site for testing webhooks btw.
Reviewers: dsagal, paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3019
Summary:
This adds a `site:delete` target to `cli.sh` for deleting sites. Sites should be specified by numeric org id, and for confirmation their name also needs to be given.
All the docs in the site are deleted permanently, and the workspaces, and the site, and the stripe customer (if any).
Test Plan: manual
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3015
Summary:
Moves CSV and XLSX export urls under /download/, and
removes the document title query parameter which is now
retrieved from the backend.
Test Plan: No new tests. Existing tests that verify endpoints still function.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3010
Summary:
The endpoints for exporting CSV and Excel are now under
/api/docs/:docId/ and are forwarded to a doc worker for export.
The Share Menu has been updated to use the new endpoints.
Test Plan: No new tests. Existing tests that verify endpoints work correctly.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3007
Summary:
- Grist document has a associated "locale" setting that affects how currency is formatted.
- Currency selector for number format.
Test Plan: not done
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D2977
Summary:
Applies simple data transformations to the existing /data API.
Mimics the Airtable API. Designed in https://grist.quip.com/RZh9AEbPaj8x/Doc-API#FZfACAAZ9a0
Haven't done deletion because it seems like less of a priority and also not fully designed.
Test Plan: Added basic server tests similar to the /data tests. Haven't tested edge cases like bad input.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D2974
Summary:
Implementing export to excel and send to Google Drive feature.
As part of this feature few things were implemented:
- Server side google authentication exposed on url: (docs, docs-s, or localhost:8080)/auth/google
- Exporting grist documents as an excel file (xlsx)
- Storing exported grist document (in excel format) in Google Drive as a spreadsheet document.
Server side google authentication requires one new environmental variables
- GOOGLE_CLIENT_SECRET (required) used by authentication handler
Test Plan: Browser tests for exporting to excel.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2924
Summary:
When a document has an exception to allow copies,
unset that option on any copies of the document.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2794
Summary:
- Update rules to be more like we've had with tslint
- Switch tsserver plugin to eslint (tsserver makes for a much faster way to lint in editors)
- Apply suggested auto-fixes
- Fix all lint errors and warnings in core/, app/, test/
Test Plan: Some behavior may change subtly (e.g. added missing awaits), relying on existing tests to catch problems.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2785
Summary:
This allows a fork to be made by a user if:
* That user is an owner of the document being forked, or
* That user has full read access to the document being forked.
The bulk of the diff is reorganization of how forking is done. ActiveDoc.fork is now responsible for creating a fork, not just a docId/urlId for the fork. Since fork creation should not be limited to the doc worker hosting the trunk, a helper endpoint is added for placing the fork.
The change required sanitizing worker allocation a bit, and allowed session knowledge to be removed from HostedStorageManager.
Test Plan: Added test; existing tests pass.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2700
Summary:
This adds a snapshots/remove and states/remove endpoint, primarily
for maintenance work rather than for the end user. If some secret
gets into document history, it is useful to be able to purge it
in an orderly way.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2694
Summary:
Adds an "enter safe mode" option and explanation in modal that appears when a document fails to load, if user is owner. If "enter safe mode" is selected, document is reloaded on server in a special mode. Currently, the only difference is that if the acl rules fail to load, they are replaced with a fallback that grants full access to owners and no access to anyone else. An extra tag is shown to mark the document as safe mode, with an "x" for cancelling safe mode.
There are other ways a document could fail to load than just acl rules, so this is just a start.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2686
Summary:
This changes how user attributes are loaded. They are now loaded
directly from sqlite, with per-session caching. Optimizations
considered but not addressed yet are (1) adding indexes to user attribute
tables and (2) swapping in a thinner sqlite wrapper.
The main benefit of this diff is that changes to user attribute
tables now work. Clients whose user attributes are not changed
see no effect; clients whose user attributes have changed have
their document reloaded.
For the purposes of testing, the diff includes a tweak to
GristWSConnection to be "sticky" to a specific user when reloading
(and support machinery on the server side to honor that). Until
now, if a GristWSConnection reloads, it uses whatever the current
default user is in the cookie-based session, which can change.
This was complicating a test where multiple users were accessing
the same document via different clients with occasional document
reloads.
Code for updating when schema or rule changes happen is moved
around but not improved in any meaningful way in this diff.
Test Plan: existing tests pass; extended test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2685
Summary:
With recent changes to action history, we can now remove the temporary
`finalRowContent` field from change details, since all the information
we need is now in the ActionSummary.
We also now have more information about the state of the common ancestor,
which previously we could not get either from ActionSummary or from
`finalRowContent`. We take advantage of that to flesh out rendering
differences where there are some changes locally and some changes
remotely.
There's still a lot more to do, this is just one step.
I have added a link to the UI for viewing the comparison. I wouldn't
want to advertise that link until diffs are robust to name changes.
Test Plan: added test, updated tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2658
Summary:
In an emergency, we may want to serve certain documents with "old" workers as we fix problems. This diff adds some support for that.
* Creates duplicate task definitions and services for staging and production doc workers (called grist-docs-staging2 and grist-docs-prod2), pulling from distinct docker tags (staging2 and prod2). The services are set to have zero workers until we need them.
* These new workers are started with a new env variable `GRIST_WORKER_GROUP` set to `secondary`.
* The `GRIST_WORKER_GROUP` variable, if set, makes the worker available to documents in the named group, and only that group.
* An unauthenticated `/assign` endpoint is added to documents which, when POSTed to, checks that the doc is served by a worker in the desired group for that doc (as set manually in redis), and if not frees the doc up for reassignment. This makes it possible to move individual docs between workers without redeployments.
The bash scripts added are a record of how the task definitions + services were created. The services could just have been copied manually, but the task definitions will need to be updated whenever the definitions for the main doc workers are updated, so it is worth scripting that.
For example, if a certain document were to fail on a new deployment of Grist, but rolling back the full deployment wasn't practical:
* Set prod2 tag in docker to desired codebase for that document
* Set desired_count for grist-docs-prod2 service to non-zero
* Set doc-<docid>-group for that doc in redis to secondary
* Hit /api/docs/<docid>/assign to move the doc to grist-docs-prod2
(If the document needs to be reverted to a previous snapshot, that currently would need doing manually - could be made simpler, but not in scope of this diff).
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2649
Summary:
* Extends `/api/docs/docId1/compare/docId2` endpoint with a `detail=1` option to include details of what changed in the document content.
* Adds an `/api/docs/docId/compare?left=HASH&right=HASH` endpoint for comparing two versions of a single document. This is needed to implement the extension to `/api/docs/docId1/compare/docId2`.
* Adds a `HashUtil` class to allow hash aliases like `HEAD` and `HEAD~`.
Everything is a bit crude:
* Changes are expressed as ActionSummary objects, which aren't fully fleshed out.
* Extra data about formula columns is inserted in an inflexible way.
This is extracted and cleaned up from https://phab.getgrist.com/D2600.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2614
Summary:
This makes it possible to serve a table or tables only to owners.
* The _grist_ACLResources table is abused (temporarily) such that rows of the form `{colId: '~o', tableId}` are interpreted as meaning that `tableId` is private to owners.
* Many websocket and api endpoints are updated to preserve the privacy of these tables.
* In a document where some tables are private, a lot of capabilities are turned off for non-owners to avoid leaking info indirectly.
* The client is tweaked minimally, to show '-' where a page with some private material would otherwise go.
No attempt is made to protect data from private tables pulled into non-private tables via formulas.
There are some known leaks remaining:
* Changes to the schema of private tables are still broadcast to all clients (fixable).
* Non-owner may be able to access snapshots or make forks or use other corners of API (fixable).
* Changing name of table makes it public, since tableId in ACLResource is not updated (fixable).
Security will require some work, the attack surface is large.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2604
Summary: This makes the user's role (owner/editor/viewer) available in ActiveDoc methods. No use of that information is made yet, other than to log it. The bulk of the diff is getting a handle on the various ways the methods can be called, and systematizing it a bit more. In passing, access control is added to broadcasts of document changes, so users who no longer have access to a document do not receive changes if they still have the document open.
Test Plan: existing tests pass; test for broadcast access control added
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2599
Summary: This moves enough server material into core to run a home server. The data engine is not yet incorporated (though in manual testing it works when ported).
Test Plan: existing tests pass
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2552
