This repository was archived by the owner on Aug 23, 2025. It is now read-only.
Query ACL enhancements: Add userEmail and groupId#558
Merged
Conversation
added 10 commits
February 28, 2020 12:26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Expands Query ACL implementation to add support for specifying
userEmailandgroupId. Also adds some decorations to the query objects returned from REST API to indicate the permissions the user has that is calling GET on/queries.userEmailis being added as it is more config-friendly for seed data thanuserId, which isn't really exposed to end users.userEmailis also used as the "foreign key" on queries, as well as the primary lookup for users, so this is somewhat consistent with the rest of SQLPad.groupIdis added to store the special__EVERYONE__value, as it isn't auserId. This is primarily to help bring clarity to database, and potentially get to a place foruserIdcan be a FK to users at some point in the future.This PR adds a lot of migrations to make this happen, all broken up into atomic operations. Sequelize's QueryInterface does not support running operations within a transaction, so a failure could leave a migration half implemented if these operations are not broken up.
This is kind of a pain, so we may want to start using hand-written SQL for migrations and run that within a transaction if SQLite supports it. The downside here is we lose on cross-db support if SQLPad eventually is to support other databases like postgres for a backend, but I don't know how realistic that is at this point. (Maybe just focus on SQLite for now?)