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

Create a new workflows endpoint #578

Merged
merged 2 commits into from
Sep 29, 2022
Merged

Create a new workflows endpoint #578

merged 2 commits into from
Sep 29, 2022

Conversation

csc-felipe
Copy link
Contributor

@csc-felipe csc-felipe commented Sep 13, 2022

Description

Workflows define the process for creating and publishing a submission.

Related issues

Type of change

  • New feature
    This PR doesn't have breaking changes

Changes Made

  • Added workflow schema
  • Added initial workflow descriptions
  • Added check for the workflow schema
  • Updated OpenAPI schema with new endpoints

Testing

  • Unit Tests
  • Integration Tests
  • Schema validation

Mention

This change has features that can be used in the frontend, but breaking changes will come in a future PR. @hannyle

@csc-felipe csc-felipe self-assigned this Sep 13, 2022
@csc-felipe csc-felipe changed the title Feature/workflow Create a new workflows endpoint Sep 13, 2022
@csc-felipe
Copy link
Contributor Author

How can we describe the DAC and Policy in a way that works for ENA schema, and also REMS in a more general manner?

@csc-felipe csc-felipe force-pushed the feature/workflow branch 3 times, most recently from fe25deb to 749e2d3 Compare September 28, 2022 09:55
@csc-felipe
Copy link
Contributor Author

Now I'm thinking that the code change that enforces the workflows could come as a follow-up PR. It starts to be quite large.

@csc-felipe csc-felipe marked this pull request as ready for review September 28, 2022 09:55
@blankdots
Copy link
Contributor

Now I'm thinking that the code change that enforces the workflows could come as a follow-up PR. It starts to be quite large.

so does this PR fixes #362 and should be reviewed as such ?
not sure i follow the enforces the workflows could we have more details ?

@csc-felipe
Copy link
Contributor Author

csc-felipe commented Sep 28, 2022

so does this PR fixes #362 and should be reviewed as such ?

#362 was about adding endpoints for the front-end, and that has been done in the current PR.

not sure i follow the enforces the workflows could we have more details ?

Additionally, the backend should check that the

  • submission actually follows the workflow
  • which workflow the submission follows
  • which services the submission has to be published to
  • required schemas and other constraints, as defined in the workflow, have been followed

After all, the service will be used not only by the front-end, but also directly via API.

What do you think, does it make any sense?

@blankdots
Copy link
Contributor

What do you think, does it make any sense?

yes, thank you. Sounds like a new task.

🤔 (brainstorming) we might need to add to the submission.json which workflow was used in this submission, but getting ahead ...

@csc-felipe
Copy link
Contributor Author

Indeed, it was getting ahead of myself, and implementing that functionality already. Created a new issue for this feature #591

Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

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

small notes at a first glance

tox.ini Show resolved Hide resolved
metadata_backend/conf/conf.py Show resolved Hide resolved
metadata_backend/api/handlers/restapi.py Show resolved Hide resolved
Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

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

I am not sure if these should be handled by this PR or another one ?

i think we don't need METAX_ENABLED and REMS_ENABLED given that most things are going to be handled by the workflow.
at the same time, ServiceHandler and its sub-classes DataciteServiceHandler, MetaxServiceHandler and RemsServiceHandler don't need enabled property.

there are also some self.metax_handler.enabled and self.rems_handler.enabled cases

@csc-felipe
Copy link
Contributor Author

I am not sure if these should be handled by this PR or another one ?

I think that to remove the service enabled checks it requires implementing the workflow "enforcements" mentioned before. I wrote the task you mentioned to this issue: #591

blankdots
blankdots previously approved these changes Sep 29, 2022
@csc-felipe
Copy link
Contributor Author

csc-felipe commented Sep 29, 2022

Rebased to fix conflicts

@csc-felipe csc-felipe merged commit 667941a into develop Sep 29, 2022
@csc-felipe csc-felipe deleted the feature/workflow branch September 29, 2022 09:06
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.

Add workflows endpoint to set objects order in front-end
3 participants