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

Pages Editor: check if workflow is part of project #7156

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

shaunanoordin
Copy link
Member

@shaunanoordin shaunanoordin commented Aug 21, 2024

PR Overview

Staging branch URL: https://pr-7156.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging

This PR fixes an issue where it's possible to view/edit a Workflow in the incorrect Project.

The Issue:

  • e.g. if you have a WF 1111 that's part of project 22, you can still access that WF at the URL for project 44: /lab/44/workflows/editor/1111 This is clearly incorrect.
  • Note that this issue exists even with the PFE workflow editor, i.e. /lab/44/workflows/1111 still allows you to edit the WF on an incorrect project. This is a separate issue to be fixed.

The Fix:

  • Add checkIsWorkflowPartOfProject() helper that returns true if, and only if, the Workflow is linked to the Project.
  • Add error message if the Workflow isn't part of the higher-level Project.

Testing

For the following tests, staging WF 3711 belongs to project 1982, not 1992.

Status

Ready to go

@shaunanoordin shaunanoordin requested a review from a team August 21, 2024 16:53
@coveralls
Copy link

Coverage Status

coverage: 56.819%. remained the same
when pulling 57d74b7 on pages-editor-check-valid-workflow
into 5184aef on master.

Copy link
Contributor

@kieftrav kieftrav left a comment

Choose a reason for hiding this comment

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

LGTM! Works exactly as expected with straightforward code changes.

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.

3 participants