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.

Sign up for GitHub

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

Jump to bottom

Add minimal workflow support #14

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Add minimal workflow support #14

merged 1 commit into from
Sep 9, 2024

Conversation

streino
Copy link
Contributor

@streino streino commented Sep 5, 2024 •
edited
Loading

Fix #15

On records with workflow enabled:

  • record has no draft (working copy) -> proceed
  • record has draft -> raise error

This is to avoid the flaky draft implementation in Geonetwork, at least for now.

@streino streino requested a review from abulte September 5, 2024 11:28
abulte
abulte reviewed Sep 6, 2024
View reviewed changes
Copy link
Contributor

@abulte abulte left a comment

Choose a reason for hiding this comment

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

Just by reading the code, I don't understand the difference between WorkflowStatus.DRAFT and WorkflowState.draft.

Maybe use a Class with an init logic that can make the difference explicit? Or maybe a comment is enough.

ecospheres_migrator/geonetwork.py Show resolved Hide resolved
ecospheres_migrator/geonetwork.py Outdated Show resolved Hide resolved
@streino
Copy link
Contributor Author

streino commented Sep 6, 2024

Just by reading the code, I don't understand the difference between WorkflowStatus.DRAFT and WorkflowState.draft.

Tried to clarify. What do you think?

@streino streino requested a review from abulte September 6, 2024 10:55
abulte
abulte reviewed Sep 7, 2024
View reviewed changes
Copy link
Contributor

@abulte abulte left a comment

Choose a reason for hiding this comment

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

Clearer since we don't have two drafts status/state anymore 👍 And it feels like more complexity is surfacing, which is probably a business thing on geonetwork's side...

ecospheres_migrator/geonetwork.py Outdated Show resolved Hide resolved
ecospheres_migrator/geonetwork.py Outdated Show resolved Hide resolved
@streino streino force-pushed the feat/minimal-workflow-support branch from 2155ef9 to 7a706cd Compare September 7, 2024 11:08
@streino streino requested a review from abulte September 7, 2024 14:28
abulte
abulte approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@abulte abulte left a comment

Choose a reason for hiding this comment

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

🧑‍🎨

@streino
Add minimal workflow support
258c2d9 
On records with workflow enabled:
- record has no draft (working copy) -> proceed
- record has draft -> raise error

This is to avoid the flaky draft implementation in Geonetwork, at
least for now.
@streino streino force-pushed the feat/minimal-workflow-support branch from 6e147b4 to 258c2d9 Compare September 9, 2024 14:05
@streino streino merged commit b133110 into main Sep 9, 2024
1 check passed
@streino streino deleted the feat/minimal-workflow-support branch September 9, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abulte abulte abulte approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support workflow mode
2 participants
@streino @abulte