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:
Owners weren't able to access snapshots if access rules
that denied access to non-owners existed. The backend
was lowering snapshot document access to "viewers" as
part of implementing read-only behavior; this is now done
in the client, with document access for snapshots now
accurately reflecting the user's trunk access.
Additionally, sandboxes are no longer created for snapshots,
and background intervals aren't started for snapshots.
Test Plan: Browser test.
Reviewers: jarek, paulfitz
Reviewed By: jarek, paulfitz
Differential Revision: https://phab.getgrist.com/D3849
Summary:
Counts were being sent as strings to the Elasticsearch instance because the
SQL queries were returning strings for the aggregate columns instead of
integers.
Test Plan: N/A
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3859
Summary:
Enhances dark mode support for the formula editor, and adds support to
the color select popup. Also fixes some bugs with dark mode.
Test Plan: Tested manually.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3847
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:
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:
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
* Replace `ormconfig.js` with a newer mechanism of configuring
TypeORM that can be included in the source code properly.
The path to `ormconfig.js` has always been awkward to handle,
and eliminating the file makes building different Grist setups
a bit simpler.
* Remove `electron` package. It is barely used, just for some old
remnants of an older attempt at electron packaging. It was used
for two types, which I left at `any` for now. More code pruning is
no doubt possible here, but I'd rather do it when Electron packaging
has solidified.
* Add a hook for replacing the login system, and for adding some
extra middleware the login system may need.
* Add support for some more possible locations of Python, which
arise when a standalone version of it is included in the Electron
package. This isn't very general purpose, just configurations
that I found useful.
* Support using grist-core within a yarn workspace - the only tweak
needed was webpack related.
* Allow an external ID to be optionally associated with documents.
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:
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:
Diff removes view-as pill in the document breadcrumbs and add new view-as banner.
Note: Banners are still missing mechanism to handle several banners. As of now both doc-usage and view-as banners could show up at the same time.
Test Plan: Refactored existing test.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3732
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: This changes the suggestions in the User Manager autocomplete.
Test Plan: Project tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3656
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:
This makes two small tweaks based on a user's questions about sharing
sites publicly for a self-managed installation:
* The support user `support@getgrist.com` is made configurable with
`GRIST_SUPPORT_EMAIL`. This came up because only the support user
can share material with the special "everyone" user. This restriction
was added to avoid spam.
* Regardless of public sharing settings, for our SaaS we had
decided not to list public sites to anonymous users. That is
somewhat a question of taste, so a `GRIST_LIST_PUBLIC_SITES` flag
is added to override this choice.
Public sharing isn't in a well polished state, and this diff doesn't
advance that, in fact it adds a new wrinkle :-/
Test Plan: existing tests pass; manual testing
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3663
Summary:
There is a new column in users table called ref (user reference).
It holds user's unique reference number that can be used for features
that require some kind of ownership logic (like comments).
Test Plan: Updated tests
Reviewers: georgegevoian, paulfitz
Reviewed By: georgegevoian, paulfitz
Differential Revision: https://phab.getgrist.com/D3641
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:
Adds a CLI command to un-quarantine an active document. Also tweaks the
name of related environment variable to avoid a naming conflict.
Test Plan: Server test.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3583
Summary:
Adds a new CLI command, doc, with a subcommand that quarantines
an active document. Adds a group query param to a housekeeping
endpoint for updating the document group prior to checking if a doc
needs to be reassigned. Both methods require support user credentials.
Test Plan: Server tests. (Additional testing will be done manually on staging.)
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3570
Summary:
Removing NEW_DEAL flag checks and exposing all new deal features as default.
Also removing Pages.ts as it was moved to grist-core.
Test Plan: Existing and updated tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3572
Summary:
Fills in the title and description/thumbnail (for templates) in app.html if the
page being requested is for a document.
Test Plan: Tested manually.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3544
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:
With this, a custom widget can render an attachment by doing:
```
const tokenInfo = await grist.docApi.getAccessToken({readOnly: true});
const img = document.getElementById('the_image');
const id = record.C[0]; // get an id of an attachment
const src = `${tokenInfo.baseUrl}/attachments/${id}/download?auth=${tokenInfo.token}`;
img.setAttribute('src', src)
```
The access token expires after a few mins, so if a user right-clicks on an image
to save it, they may get access denied unless they refresh the page. A little awkward,
but s3 pre-authorized links behave similarly and it generally isn't a deal-breaker.
Test Plan: added tests
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3488
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:
Building:
- Builds no longer wait for tsc for either client, server, or test targets. All use esbuild which is very fast.
- Build still runs tsc, but only to report errors. This may be turned off with `SKIP_TSC=1` env var.
- Grist-core continues to build using tsc.
- Esbuild requires ES6 module semantics. Typescript's esModuleInterop is turned
on, so that tsc accepts and enforces correct usage.
- Client-side code is watched and bundled by webpack as before (using esbuild-loader)
Code changes:
- Imports must now follow ES6 semantics: `import * as X from ...` produces a
module object; to import functions or class instances, use `import X from ...`.
- Everything is now built with isolatedModules flag. Some exports were updated for it.
Packages:
- Upgraded browserify dependency, and related packages (used for the distribution-building step).
- Building the distribution now uses esbuild's minification. babel-minify is no longer used.
Test Plan: Should have no behavior changes, existing tests should pass, and docker image should build too.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3506
Summary:
- Update nudge boxes content and collapsing on personal and free team site
- New confirmation after upgrading from a free team site
- Refactoring ProductUpgrade code, splitting plans / modals and nudges
Test Plan: Manual and updated tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3481
Summary:
- Upgrades to build-related packages:
- Upgrade typescript, related libraries and typings.
- Upgrade webpack, eslint; add tsc-watch, node-dev, eslint_d.
- Build organization changes:
- Build webpack from original typescript, transpiling only; with errors still
reported by a background tsc watching process.
- Typescript-related changes:
- Reduce imports of AWS dependencies (very noticeable speedup)
- Avoid auto-loading global @types
- Client code is now built with isolatedModules flag (for safe transpilation)
- Use allowJs to avoid copying JS files manually.
- Linting changes
- Enhance Arcanist ESLintLinter to run before/after commands, and set up to use eslint_d
- Update eslint config, and include .eslintignore to avoid linting generated files.
- Include a bunch of eslint-prompted and eslint-generated fixes
- Add no-unused-expression rule to eslint, and fix a few warnings about it
- Other items:
- Refactor cssInput to avoid circular dependency
- Remove a bit of unused code, libraries, dependencies
Test Plan: No behavior changes, all existing tests pass. There are 30 tests fewer reported because `test_gpath.py` was removed (it's been unused for years)
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3498
Summary:
- Showing nudge to individual users to sign up for free team plan.
- Implementing billing page to upgrade from free team to pro.
- New modal with upgrade options and free team site signup.
- Integrating Stripe-hosted UI for checkout and plan management.
Test Plan: updated tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3456
Summary:
During testing, and in some dev configurations, we make connections to
the Grist home database from multiple processes, and store it in Sqlite.
This is a situation prone to file locks in Sqlite. We set a busy_timeout
parameter to help, but transactions may still fail immediately. This
diff adds retries to our patch of TypeORM for Sqlite transactions
(which without the patch are already flakey for other reasons
https://github.com/typeorm/typeorm/issues/1884#issuecomment-380767213).
Without this change, I can tickly SQLITE_BUSY problems fairly
reliably with the DocApi tests. With this change, I can't so far.
This change should not affect deployments of our SaaS (which use
Postgres for home db) or grist-core/grist-ee (which access home
db from a single process in default configuration).
Test Plan: repeated running of DocApi tests
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3466
Summary:
grist-ee build was failing since it didn't have a
DocUsageBanner implementation available. Made the implementation
added to monorepo available, since it will be useful to improve
the activation banner.
Test Plan: manaul
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: georgegevoian
Differential Revision: https://phab.getgrist.com/D3452
Summary:
I missed committing a file that is important for editing files comfortably in the ext directory in an IDE. This diff:
* Adds tsconfig-base-ext.json - that was the only intended change
* Unrelated: Forces all creation of connections to the home db through a new `getOrCreateConnection` method which changes the `busy_timeout` if using Sqlite. This was an attempt to fix random "database is locked" test failures. I believe multiple connections to the home db as an sqlite file do not happen in self-hosted Grist (where there is a single node process) or in our SaaS (where the database is in postgres). It does affect Grist started using `devServerMain.ts` (where multiple processes accessing same database are started) or various test configurations when extra database connections are opened.
* Unrelated: I added a `busy_timeout` for session storage, when it uses Sqlite. Again, I don't believe this affects self-hosted Grist or our SaaS.
* Tweaked a `BillingDiscount` test that looked perhaps vulnerable to a stripe request stalling.
I can't be sure my tweaks actually help, since I didn't succeed in replicating the failures. Update: looks like the "locked" error can still happen :(
Test Plan: manual
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3450
Summary:
Also fixes a minor CSS regression in UserManager where the
link to add a team member wasn't shown on a separate row.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3444
Summary:
Instead of showing a blank dialog for users whose access
is limited (e.g. public members), we now show the user's
role and a mention of whether their access is public.
Test Plan: Browser tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3431
Summary: For grist-ee, expect an activation key in environment variable `GRIST_ACTIVATION` or in a file pointed to by `GRIST_ACTIVATION_FILE`. In absence of key, start a 30-day trial, during which a banner is shown. Once trial expires, installation goes into document-read-only mode.
Test Plan: added a test
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3426
Summary:
The summary includes a count of documents that are approaching
limits, in grace period, or delete-only. The endpoint is only accessible
to site owners, and is currently unused. A follow-up diff will add usage
banners to the site home page, which will use the response from the
endpoint to communicate usage information to owners.
Test Plan: Browser and server tests.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3420
Summary: Allow exceeding the daily API usage limit for a doc based on additional allocations for the current hour and minute. See the doc comment on getDocApiUsageKeysToIncr for details. This means that up to 5 redis keys may be relevant at a time for a single document.
Test Plan: Updated and expanded 'Daily API Limit' tests.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3368
Summary:
- Add a new parameter `Features.baseMaxAttachmentsBytesPerDocument` and set it to 1GB for the free team product.
- Add a method to DocStorage to calculate the total size of existing and used attachments.
- Add a migration to DocStorage adding an index to make the query in the above method fast.
- Check in ActiveDoc if uploading attachment(s) would exceed the product limit on that document.
Test Plan: Added test in `limits.ts` testing enforcement of the attachment limit.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3374
Summary:
For users who cannot otherwise change access to a resource, let
them remove themselves. Implemented via the standard endpoints
as a special exception that will process a request from a user
that would otherwise be denied, if the only contents of that
request are a removal of themselves.
Users who can change access are still not permitted to change their
own permissions or to remove themselves, as a precaution against
orphaning resources.
Test Plan: extended and updated tests
Reviewers: cyprien
Reviewed By: cyprien
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3367
Summary:
This also updates Authorizer to link the authSubject
to Grist users if not previously linked. Linked subjects
are now used as the username for password-based logins,
instead of emails, which remain as a fallback.
Test Plan: Existing tests, and tested login flows manually.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3356
Summary:
This adds an optional `isConsultant` flag to `users.options`, and an endpoint that allows the support user to turn it on or off. Users marked as consultants are not counted as billable members. Follows the example of existing `allowGoogleLogin` option.
Billable members are counted when members are added or removed from a site. Changing the `isConsultant` flag has no immediate or retroactive effect on billing. The number of users in stripe is now set unconditionally, rather than only when it has changed.
Notifications to billing managers are not aware of this billing nuance, but continue to report user counts that include consultants. The notifications link users to the billing page.
Test Plan: extended test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: anaisconce, jarek
Differential Revision: https://phab.getgrist.com/D3362
Summary:
Adds a new Grist login page to the login app, and replaces the
server-side Cognito Google Sign-In flow with Google's own OAuth flow.
Test Plan: Browser and server tests.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3332
Summary:
Track 'data size' in ActiveDoc alongside row count. Measure it at most once every 5 minutes after each change as before, or after every change when it becomes high enough to matter.
A document is now considered to be approaching/exceeding 'the data limit' if either the data size or the row count is approaching/exceeding its own limit.
Unrelated: tweaked teamFreeFeatures.snapshotWindow based on Quip comments
Test Plan: Tested manually that data size is now logged after every change once it gets high enough, but only if the row limit isn't also too high. Still too early for automated tests.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3341
Summary:
Builds upon https://phab.getgrist.com/D3328
- Add HomeDB column `Document.gracePeriodStart`
- When the row count moves above the limit, set it to the current date. When it moves below, set it to null.
- Add DataLimitStatus type indicating if the document is approaching the limit, is in a grace period, or is in delete only mode if the grace period started at least 14 days ago. Compute it in ActiveDoc and send it to client when opening.
- Only allow certain user actions when in delete-only mode.
Follow-up tasks related to this diff:
- When DataLimitStatus in the client is non-empty, show a banner to the appropriate users.
- Only send DataLimitStatus to users with the appropriate access. There's no risk landing this now since real users will only see null until free team sites are released.
- Update DataLimitStatus immediately in the client when it changes, e.g. when user actions are applied or the product is changed. Right now it's only sent when the document loads.
- Update row limit, grace period start, and data limit status in ActiveDoc when the product changes, i.e. the user upgrades/downgrades.
- Account for data size when computing data limit status, not just row counts.
See also the tasks mentioned in https://phab.getgrist.com/D3331
Test Plan: Extended FreeTeam nbrowser test, testing the 4 statuses.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3331
Summary:
This adds support for gvisor sandboxing in core. When Grist is run outside of a container, regular gvisor can be used (if on linux), and will run in rootless mode. When Grist is run inside a container, docker's default policy is insufficient for running gvisor, so a fork of gvisor is used that has less defence-in-depth but can run without privileges.
Sandboxing is automatically turned on in the Grist core container. It is not turned on automatically when built from source, since it is operating-system dependent.
This diff may break a complex method of testing Grist with gvisor on macs that I may have been the only person using. If anyone complains I'll find time on a mac to fix it :)
This diff includes a small "easter egg" to force document loads, primarily intended for developer use.
Test Plan: existing tests pass; checked that core and saas docker builds function
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3333
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:
- Add a method `getSnapshotWindow` to `IInventory` and `DocSnapshotInventory`. It returns a `SnapshotWindow`, which represents a duration of time for which we keep backups for a particular document.
- `DocSnapshotPruner` calls this method and passes the window to `shouldKeepSnapshots` to determine which document versions have fallen outside the window and should be pruned.
- The implementation passed to `DocSnapshotInventory` uses a new method `getDocProduct` in `HomeDBManager` which directly returns the `Product` associated with a document, given only the document ID. Other methods in `HomeDBManager` require passing more information, especially about a user, but `DocSnapshotPruner` only knows about document IDs.
Test Plan: Added a test for `getDocProduct` and a test for `DocSnapshotPruner` where `getSnapshotWindow` is specified.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3322
Summary:
Importing a .grist document is implemented in a somewhat clunky way, in a multi-worker setup.
* First a random worker receives the upload, and updates Grist's various stores appropriately (database, redis, s3).
* Then a random worker is assigned to serve the document.
If the worker serving the document fails, there is a chance the it will end up assigned to the worker that handled its upload. Currently the worker will misbehave in this case. This diff:
* Ports a multi-worker test from test/home to run in test/s3, and adds a test simulating a bad scenario seen in the wild.
* Fixes persistence of any existing document checksum in redis when a worker is assigned.
* Adds a check when assigned a document to serve, and finding that document already cached locally. It isn't safe to rely only on the document checksum in redis, since that may have expired.
* Explicitly claims the document on the uploading worker, so this situation becomes even less likely to arise.
Test Plan: added test
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3305
Summary:
This makes many small changes so that Grist is less fussy to run as a single instance behind a reverse proxy. Some users had difficulty with the self-connections Grist would make, due to internal network setup, and since these are unnecessary in any case in this scenario, they are now optimized away. Likewise some users had difficulties related to doc worker urls, which are now also optimized away. With these changes, users should be able to get a lot further on first try, at least far enough to open and edit documents.
The `GRIST_SINGLE_ORG` setting was proving a bit confusing, since it appeared to only work when set to `docs`. This diff
adds a check for whether the specified org exists, and if not, it creates it. This still depends on having a user email to make as the owner of the team, so there could be remaining difficulties there.
Test Plan: tested manually with nginx
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3299
Summary: Removes code that was marked for removal.
Test Plan: Existing tests still pass.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3289
Summary: When user 2FA status is changed, we now send out emails via SendGrid.
Test Plan: Server tests.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3280
Summary:
Enabled by default, the new checkbox is only visible to
users logged in with email/password, and controls whether it is possible
to log in to the same account via a Google account
(with matching email). When disabled, CognitoClient will refuse logins
from Google if a Grist account with the same email exists.
Test Plan:
Server and browser tests for setting flag. Manual tests to verify
Cognito doesn't allow signing in with Google when flag is disabled.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3257
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:
Limits crafted for our SaaS product were getting applied to grist-core
users. This diff removes them. There will be limits on a future
self-managed product.
Test Plan: checked manually
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3255
Summary:
This adds a Feature object that is an approximation of what we
plan for free team sites. It includes restrictions that are not
yet implemented, and an endpoint for testing.
Test Plan: added a test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3243
Summary:
* Tie build and run-time docker base images to a consistent version (buster)
* Extend the test login system activated by GRIST_TEST_LOGIN to ease porting tests that currently rely on cognito (many)
* Make org resets work in absence of billing endpoints
* When in-memory session caches are used, add missing invalidation steps
* Pass org information through sign-ups/sign-ins more carefully
* For CORS, explicitly trust GRIST_HOST origin when set
* Move some fixtures and tests to core, focussing on tests that cover existing failures or are in the set of tests run on deployments
* Retain regular `test` target to run the test suite directly, without docker
* Add a `test:smoke` target to run a single simple test without `GRIST_TEST_LOGIN` activated
* Add a `test:docker` target to run the tests against a grist-core docker image - since tests rely on certain fixture teams/docs, added `TEST_SUPPORT_API_KEY` and `TEST_ADD_SAMPLES` flags to ease porting
The tests ported were `nbrowser` tests: `ActionLog.ts` (the first test I tend to port to anything, out of habit), `Fork.ts` (exercises a lot of doc creation paths), `HomeIntro.ts` (a lot of DocMenu exercise), and `DuplicateDocument.ts` (covers a feature known to be failing prior to this diff, the CORS tweak resolves it).
Test Plan: Manually tested via `buildtools/build_core.sh`. In follow up, I want to add running the `test:docker` target in grist-core's workflows. In jenkins, only the smoke test is run. There'd be an argument for running all tests, but they include particularly slow tests, and are duplicates of tests already run (in different configuration admittedly), so I'd like to try first just using them in grist-core to gate updates to any packaged version of Grist (the docker image currently).
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3176
Summary:
The ./test/grist-admin doc-info command line tool was out of date
and not showing user access correctly anymore. This freshens the tool and
adds a small test for it.
Test Plan: Added test.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3182
Summary:
- Fix base href in HelpScout beacon when showing articles (in particular for Firefox)
- Show the 'Answers' tab normally except when reporting an error.
- Combine the "Give Feedback" and "Help Center" buttons into one that normally
opens the beacon (with a link to Help Center and to Community Forum), and a
smaller one that opens the Help Center site in a new tab.
- Update HELP_SCOUT_* env vars to use _V2 suffix, to allow them to coexist with
code using the previous beacon.
Test Plan: Updated the browser test to check the new behavior.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3170
Summary:
Exposing custom widgets as a dropdown menu in custom section configuration panel.
Adding new environmental variable GRIST_WIDGET_LIST_URL that points to a
json file with an array of available widgets. When not present, custom widget menu is
hidden, exposing only Custom URL option.
Available widget list can be fetched from:
https://github.com/gristlabs/grist-widget/releases/download/latest/manifest.json
Test Plan: New tests, and updated old ones.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3127
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:
Shares of the same role (e.g. viewer) at different levels could interact for a resource (e.g. a doc) shared with everyone@, potentially blocking the listing of that resource. This diff removes the interaction.
The permission of a user on a resource is calculated by finding all acl rules that link that resource to a group to which the user belongs, or to a group that has a subgroup to which the user belongs, etc, and then bitwise-or-ing the permissions on the acl rules. A later wrinkle was to allow public sharing via special users. A still later wrinkle was to avoid listing resources if they were only shared with the special everyone@ user, while allowing access to them if user has their full link. That wrinkle had a bug, where if e.g. a doc were shared with everyone@ as a viewer, and the org the doc was in was shared with someone@ as a viewer, and the doc inherited the org permissions via a workspace, then that doc would end up not being listed.
The fix is straightforward enough, but needs different code for postgres and sqlite, and is a bit verbose because we unwrap subgroups to a few levels rather than doing recursion (which looks cleaner but was slower in benchmarks).
Test Plan: added test that fails without this fix
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3095
Summary:
This gives more guidance to users when editing document shares in the UserManager dialog.
* For a document on a team site, any shares with team members are marked `Team member`.
* Shares that count as external collaborators are marked for documents on a team or personal site as `collaborator` (personal site) or `outside collaborator` (team site).
* Collaborators are marked `1 of 2`, `2 of 2`, and then `limit exceeded`.
* On a team site, links are offered for each collaborator to add them to the team. The links lead to a prefilled dialog for managing team membership which can be confirmed immediately, allowing the user to continue without interruption.
* On a personal site, for the last collaborator and beyond, a link is added for creating a team. This isn't seamless since creating a team involves billing etc.
There's a small unrelated tweak in tests to remove a confusing import from `test/browser` in `test/server`.
One thing I didn't get to is checking if owner of doc is owner of site. If they aren't, they may try to add a member and be denied at that point - it would be more polite to change messaging earlier for them.
Test Plan: added and updated tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3083
Summary:
The users shown by the "View As" button are now drawn from more sources:
* There are users the document is shared with. This has been rationalized, the behavior was somewhat erratic. If the user is not an owner of the document, the only user of this kind that will be listed is themselves.
* There are users mentioned in any user attribute table keyed by Email. If name and access columns are present, those are respected, otherwise name is taken from email and access is set to "editors".
* There are example users provided if there are not many other users available.
Test Plan: added and extended tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3045
Summary:
This adds a `user:delete` target to the `cli.sh` tool. The desired user will be deleted from our database, from sendgrid, and from cognito.
There is code for scrubbing the user from team sites, but it isn't yet activated, I'm leaving finalizing and writing tests for it for follow-up.
Test Plan: tested manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3043
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 tests site deletion with and without a plan.
Test Plan: adding tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3017
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:
Access endpoints were supposed to provide display versions of emails,
but in fact only the org endpoint was doing so. This brings the
workspaces and docs endpoints into line, and adds tests.
Full user information is tweaked slightly to return an anonymous
flag only when anonymous. This was already anticipated in the
FullUser type.
Test Plan: extended test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2999
Summary:
* Remove adjustSession hack, interfering with loading docs under saml.
* Allow the anonymous user to receive an empty list of workspaces for
the merged org.
* Behave better on first page load when org is in path - this used to
fail because of lack of cookie. This is very visible in grist-core,
as a failure to load localhost:8484 on first visit.
* Mark cookie explicitly as SameSite=Lax to remove a warning in firefox.
* Make errorPages available in grist-core.
This changes the default behavior of grist-core to now start off in
anonymous mode, with an explicit sign-in step available. If SAML is not configured,
the sign-in operation will unconditionally sign the user in as a default
user, without any password check or other security. The user email is
taken from GRIST_DEFAULT_EMAIL if set. This is a significant change, but
makes anonymous mode available in grist-core (which is convenient
for testing) and makes behavior with and without SAML much more consistent.
Test Plan: updated test; manual (time to start adding grist-core tests though!)
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2980
Summary:
SAML support had broken due to SameSite changes in browsers. This
makes it work again, and tests it against Auth0 (now owned by Okta).
Logging in and out works. The logged out state is confusing, and may
not be complete. The "Add Account" menu item doesn't work.
But with this, an important part of self-hosting becomes easier.
SAML support works also in grist-core, for site pages, but there
is a glitch on document pages that I'll look into separately.
Test Plan: tested manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2976
Summary: Importing from google drive from home screen (also for anonymous users)
Test Plan: Browser tests
Reviewers: dsagal, paulfitz
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2943
Summary:
- Add showGristTour preference, and trigger tour automatically.
- Tour is only triggered for new and anonymous users on a personal org, with
edit permission.
- Automatically open the right panel at tour start.
- Don't show tours on mobile, since that's not ready (popups are cut off
and can't be dismissed)
- Cancel previous tour if a new one is somehow started.
- Remove #repeat- trigger hash tags from the URL when the tour starts.
- Ensure Help Center popup is positioned even when left panel is collapsed.
- Polish up the content of the last two cards in the tour.
Test Plan: Added test case for triggering and opening right panel.
Reviewers: alexmojaki, paulfitz
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D2955
Summary:
The old Examples and Templates workspace is now
a page that pulls templates from a new public Grist Templates org.
The All Documents view will pull featured templates from that org, where
featured templates are simply pinned documents in Grist Templates. The
Examples and Templates page will also show the featured templates, as
well as the rest of the available templates organized by category. The
categories are equivalent to workspaces in Grist Templates, and are
generated dynamically.
Test Plan: Browser tests.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz, dsagal
Subscribers: dsagal, paulfitz, jarek
Differential Revision: https://phab.getgrist.com/D2930
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:
Bundles some new document options into a JSON column.
The icon option is treated somewhat gingerly. It is intended, at
least initially, to store an image thumbnail for a document as a
url to hand-prepared assets (for examples and templates), so it is
locked down to a particular url prefix to avoid opening the door to
mischief.
Test Plan: added test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D2916
Summary:
This suspends service to a team site for which an AppSumo refund has been made, and nudges users to their free personal account.
I expect that a refund request would fail for a site where user is also paying us for extra seats.
Test Plan: tested manually
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2912
Summary:
This links AppSumo sign-ups with Stripe subscriptions
and our billing pages. Different AppSumo tiers are supported by
different coupons on the standard plan. Configuration of this
is in stripe, and then cached in the database.
The front end is tweaked just enough to make completing a sign-up
possible. It is not yet friendly.
Not covered includes:
* Streamlining landing page.
* Making billing pages git clearer summaries of AppSumo states.
* Making flow through Cognito as graceful as possible - default
probably doesn't meet AppSumo requirements.
* Disabling site on cancellation/refund.
* Downgrades when more seats in use than lower tier allows.
Test Plan: api-level tests added. No front-end tests yet.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2878
Summary:
This adds appsumo /token and /notification endpoints, with some
tests. The stub implementation is sufficient for AppSumo
activation to succeed (when exposed via port forwarding for testing).
It needs fleshing out:
* Implement upgrade/downgrade/refund and stripe subscription.
* Implement custom landing page and flow.
Test Plan: added tests
Reviewers: dsagal, georgegevoian
Reviewed By: dsagal
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D2864
Summary:
A recently added stress test ("deletes documents reasonably quickly" in removedAt.ts) is sporadically failing under postgres. It looks like typeorm's .save() method is in some way unreliable when setting a table with multi-column primary keys, via a ManyToMany relation. This diff replaces the .save() with explicit inserts/deletes.
I modified _repairWorkspaceGuests recently, so thought that change might have been the problem. However under the stress test, failures occur as often in _repairOrgGuests (not changed recently) as in _repairWorkspaceGuests (changed recently).
For reference, see schema diagram at https://grist.quip.com/wWpRAMe058Nl/Home-DB (the table being updated is `group_users`).
Possibly related issue: https://github.com/typeorm/typeorm/issues/4122
Test Plan:
After this change, stress test runs well on postgres locally (no failure 70 iterations); before it would fail on postgres within 3 iterations typically.
Separately: I gave a test that failed a little more time to return, and confirmed it was no slower on average, so I think it was unrelated.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D2848
Summary:
The `_repairWorkspaceGuests` method is slow for workspaces with large numbers of documents. It makes a query that produces a lot of rows. The query itself is tolerable, but TypeORM processing uses enough CPU to be a likely culprit in some production instability. This diff splits the query into two pieces that are logically independent, but which when combined were resulting in the number of rows being the product of the two pieces. Once split, there is also a where clause that can be applied to one of the pieces.
The purpose of the method is to add every user that a document within a workspace is shared with to a "guest" group of the workspace itself. The design of "guest" groups is not ideal, but this diff leaves the design unchanged and is intended only to speed up operation.
Made some small tweaks to the timing of a flakey test, and temporarily recreated the `samples` directory removed in a previous diff (this is currently breaking tests badly on a fresh worker without a `samples` directory lying around)
Test Plan: added test; existing tests pass
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D2844
Summary:
This cleans up a few things about SELF_HYPERLINK urls:
* Use `urlId` rather than `docId`.
* Correctly merge personal org subdomain.
* In dev environment, use clearer port number.
Test Plan: updated test
Reviewers: alexmojaki, dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2841
Summary:
We used tslint earlier, and on switching to eslint, some rules were not
transfered. This moves more rules over, for consistent conventions or helpful
warnings.
- Name private members with a leading underscore.
- Prefer interface over a type alias.
- Use consistent spacing around ':' in type annotations.
- Use consistent spacing around braces of code blocks.
- Use semicolons consistently at the ends of statements.
- Use braces around even one-liner blocks, like conditionals and loops.
- Warn about shadowed variables.
Test Plan: Fixed all new warnings. Should be no behavior changes in code.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2831
Summary:
This diff discounts indirect changes for access control purposes. A UserAction that updates a cell A, which in turn causes changes in other dependent cells, will be considered a change to cell A for access control purposes.
The `engine.apply_user_actions` method now returns a `direct` array, with a boolean for each `stored` action, set to `true` if the action is attributed to the user or `false` if it is attributed to the engine. `GranularAccess` ignores actions attributed to the engine when checking for edit rights.
Subtleties:
* Removal of references to a removed row are considered direct changes.
* Doesn't play well with undos as yet. An action that indirectly modifies a cell the user doesn't have rights to may succeed, but it will not be reversible.
Test Plan: added tests, updated tests
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2806
Summary:
- 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 `*SPECIAL:AccessRules` to give read access to the access rules to more users, and `*SPECIAL:FullCopies` to grant download/copy rights to more users.
This diff also changes forks to be owned by the user who forked them (previously they were an editor), since that feels more natural.
Test Plan: Added and updated tests.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2760
Summary:
* Adds a `SELF_HYPERLINK()` python function, with optional keyword arguments to set a label, the page, and link parameters.
* Adds a `UUID()` python function, since using python's uuid.uuidv4 hits a problem accessing /dev/urandom in the sandbox. UUID makes no particular quality claims since it doesn't use an audited implementation. A difficult to guess code is convenient for some use cases that `SELF_HYPERLINK()` enables.
The canonical URL for a document is mutable, but older versions generally forward. So for implementation simplicity the document url is passed it on sandbox creation and remains fixed throughout the lifetime of the sandbox. This could and should be improved in future.
The URL is passed into the sandbox as a `DOC_URL` environment variable.
The code for creating the URL is factored out of `Notifier.ts`. Since the url is a function of the organization as well as the document, some rejiggering is needed to make that information available to DocManager.
On document imports, the new document is registered in the database slightly earlier now, in order to keep the procedure for constructing the URL in different starting conditions more homogeneous.
Test Plan: updated test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2759
Summary:
Adds POST /api/housekeeping/docs/:docId/force-reload, which allows the support user to force a document to reload (even if they don't otherwise have access to the document).
This could be done without a separate endpoint, but that turned out a bit messy, and there's some advantage to quarantining the exceptional authorization somewhere it could be easily elaborated.
Test Plan: adds test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2713
