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

Refactor cancel action #3507

Merged
merged 26 commits into from
Aug 5, 2024
Merged

Refactor cancel action #3507

merged 26 commits into from
Aug 5, 2024

Conversation

seallard
Copy link
Contributor

@seallard seallard commented Aug 2, 2024 •
edited
Loading

The existing cancel action is pretty messy, this PR tidies it up a bit.

@seallard seallard changed the base branch from master to hide-cancelled-samples-from-customers August 2, 2024 11:27
@seallard seallard marked this pull request as ready for review August 2, 2024 11:52
@seallard seallard requested a review from a team as a code owner August 2, 2024 11:52
@clingen-sthlm clingen-sthlm temporarily deployed to stage August 2, 2024 11:55 Inactive
islean
islean reviewed Aug 2, 2024
View reviewed changes
Copy link
Contributor

@islean islean left a comment

Choose a reason for hiding this comment

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

I think this action is not reversible and hence, we need to rethink what should happen. Both the Case and CaseSample table contains data which should not be deleted when cancelling a sample.

cg/services/sample_service/sample_service.py Outdated Show resolved Hide resolved
cg/services/sample_service/utils.py Outdated Show resolved Hide resolved
cg/store/crud/delete.py Outdated Show resolved Hide resolved
cg/services/sample_service/sample_service.py Outdated Show resolved Hide resolved
cg/store/crud/read.py Outdated Show resolved Hide resolved
cg/store/crud/update.py Show resolved Hide resolved
cg/store/crud/update.py Show resolved Hide resolved
cg/services/sample_service/sample_service.py Outdated Show resolved Hide resolved
Base automatically changed from hide-cancelled-samples-from-customers to master August 2, 2024 13:14
islean
islean approved these changes Aug 2, 2024
View reviewed changes
Copy link
Contributor

@islean islean left a comment

Choose a reason for hiding this comment

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

Much cleaner 🥳

cg/services/sample_service/sample_service.py Show resolved Hide resolved
cg/services/sample_service/sample_service.py Outdated Show resolved Hide resolved
cg/store/crud/read.py Outdated Show resolved Hide resolved
@clingen-sthlm clingen-sthlm temporarily deployed to stage August 5, 2024 07:43 Inactive
@clingen-sthlm clingen-sthlm temporarily deployed to stage August 5, 2024 07:48 Inactive
@seallard
Copy link
Contributor Author

seallard commented Aug 5, 2024

Tested in stage.

@seallard seallard merged commit 3afd2de into master Aug 5, 2024
6 checks passed
@seallard seallard deleted the refactor-cancel-action branch August 5, 2024 07:59
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Aug 5, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@islean islean islean 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.
3 participants
@seallard @islean @clingen-sthlm