Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revisions for CRUD operations with $full #15467

Merged
merged 8 commits into from
Sep 12, 2022

Conversation

freekrai
Copy link
Contributor

@freekrai freekrai commented Sep 8, 2022

Description

This PR addresses the fact that when no user is set in flows aka system or $full permissions operations, then the app didn't store revisions to track the changes.

Originally, it set the customAccountability to null, now it will set it as follows:

customAccountability = {
  user: null,
  role: null,
  admin: true,
  app: true,
  ip: '',
  userAgent: '',
  origin: '',
  share: undefined,
  share_scope: undefined,
  permissions: [],
};

This leads to revisions being created by the flow so we can track the changes.

Addresses #15345, fixes #15346

Type of Change

  • Bugfix
  • Improvement
  • New Feature
  • Refactor / codestyle updates
  • Other, please describe:

Requirements Checklist

  • New / updated tests are included
  • All tests are passing locally
  • Performed a self-review of the submitted code

If adding a new feature:

  • Documentation was added/updated

Copy link
Member

@licitdev licitdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. It works well for the item update operation. 👍

This change should be propagated to all operations that has permissions === '$full'. Let's also extract the customAccountability conditional code-block into a util such as getOperationCustomAccountability. 🙂

api/src/operations/item-update/index.ts Outdated Show resolved Hide resolved
@freekrai
Copy link
Contributor Author

freekrai commented Sep 8, 2022

Right will push an update shortly

freekrai and others added 2 commits September 8, 2022 10:27
@freekrai
Copy link
Contributor Author

freekrai commented Sep 8, 2022

@licitdev I've moved the codeblock into getAccountabilityForRole as a system role, and updated create, update read, and delete to use it.

So that calling:

customAccountability = await getAccountabilityForRole('system', { database, schema, accountability });

returns the block I had above, this lets it work the same as public, trigger or anything else.

Copy link
Member

@licitdev licitdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@licitdev licitdev self-assigned this Sep 12, 2022
@licitdev licitdev added this to the Next Release milestone Sep 12, 2022
@licitdev licitdev merged commit b1106e1 into directus:main Sep 12, 2022
qborisb pushed a commit to qborisb/directus that referenced this pull request Sep 16, 2022
* revisions for CRUD operations with $full

* Update api/src/operations/item-update/index.ts

Co-authored-by: ian <licitdev@gmail.com>

* add system role to getAccountabilityForRole and add it to create, delete and update items operations

* Whitespace consistency

Co-authored-by: ian <licitdev@gmail.com>
qborisb pushed a commit to qborisb/directus that referenced this pull request Sep 16, 2022
* revisions for CRUD operations with $full

* Update api/src/operations/item-update/index.ts

Co-authored-by: ian <licitdev@gmail.com>

* add system role to getAccountabilityForRole and add it to create, delete and update items operations

* Whitespace consistency

Co-authored-by: ian <licitdev@gmail.com>
@freekrai freekrai deleted the fix/no_revisions_crud_full branch September 23, 2022 14:58
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flows: No revisions tracked when doing CRUD operations with full permission
3 participants