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

feat(wip): Do not allow template flows to be deleted / archived #4234

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Jan 31, 2025

What does this PR do?

  • Removes "delete" button from the frontend UI - to be replaced by "Archive" in this ticket
  • Adds flows.deleted_at column in order to allow a soft delete pattern

Permissions

  • Removes "delete" permissions from all roles for the flows table
  • Allows all roles to set the flows.deleted_at column (within the parameters of their current permission levels)
  • Does not allow any roles to soft delete (set flows.deleted_at) for template flows

Some of this is pretty tricky to understand via the tables.yaml and took a bit of trial and error to figure out. Some e2e test coverage is needed here to explain this and act as a guard rail for future changes. I'm planning on introducing this in a future PR so that I'm not holding up the "archive flows" ticket @RODO94 will be shortly working on.

Copy link

github-actions bot commented Jan 31, 2025

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (1)

  • public.flows permissions:

    insert select update delete
    api / /
    demoUser / /
    platformAdmin / /
    public /
    teamEditor / /
    3 added column permissions
    insert select update
    demoUser ➕ deleted_at
    platformAdmin
    teamEditor

Copy link

github-actions bot commented Jan 31, 2025

Pizza

Deployed 204dae7 to https://4234.planx.pizza.

Useful links:

@DafyddLlyr DafyddLlyr changed the title dp/template flows delete permissions feat(wip): Do not allow template flows to be deleted Jan 31, 2025
@DafyddLlyr DafyddLlyr changed the title feat(wip): Do not allow template flows to be deleted feat(wip): Do not allow template flows to be deleted / archived Jan 31, 2025
@DafyddLlyr DafyddLlyr force-pushed the dp/template-flows-delete-permissions branch 4 times, most recently from d74ce13 to 9de2b6f Compare February 5, 2025 11:19
Comment on lines +652 to +675
filter:
deleted_at:
_is_null: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This repeated pattern means that archived flows cannot be selected.

Comment on lines 822 to 828
check:
_and:
- deleted_at:
_is_null: false
- templated_flows:
id:
_is_null: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This repeated pattern is a post-update check which ensures that flows cannot be archived if they have associated templated flows derived from them.

@DafyddLlyr DafyddLlyr force-pushed the dp/template-flows-delete-permissions branch 5 times, most recently from 9532b5b to db405ff Compare February 7, 2025 11:36
@DafyddLlyr DafyddLlyr force-pushed the dp/template-flows-delete-permissions branch from db405ff to b457494 Compare February 10, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant