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

RSDEV-280: Return the list of Subsamples when cannot delete a Sample #59

Merged
merged 11 commits into from
Jul 22, 2024

Conversation

rs-nicof
Copy link
Contributor

@rs-nicof rs-nicof commented Jul 15, 2024

This change include changes to how the error message when deleting samples is shown. In the screenshot below, a sample with two subsamples in containers and a lone subsample from a different sample have been deleted. The former has failed as the subsamples are in containers (with the main part of this change being that those subsamples are now detailed and linked to), and the latter has been deleted correctly hence the green alert.

Screenshot 2024-07-16 at 11 08 16

@rs-nicof rs-nicof requested a review from mKowalski256 July 15, 2024 13:58
@rs-nicof rs-nicof force-pushed the RSDEV-280-backend branch 3 times, most recently from 79a3ae9 to 033025b Compare July 15, 2024 15:37
@rs-nicof rs-nicof marked this pull request as ready for review July 15, 2024 16:11
@rs-nicof rs-nicof force-pushed the RSDEV-280-backend branch from 033025b to 68b5f1b Compare July 15, 2024 16:15
@rs-nicof rs-nicof force-pushed the RSDEV-280-backend branch from 68b5f1b to 2eac49e Compare July 15, 2024 17:45
@rs-nicof
Copy link
Contributor Author

✅ Build full java testds passed: https://jenkins.researchspace.com/job/rspace-web/job/RSDEV-280-backend/16/

@mKowalski256
Copy link
Contributor

I've looked through the code, and eventually decided to change the way how info about which subsamples are problematic is encoded into API response. @rs-nicof to avoid enlarging sample response object I've only kept canBeDeleted flag on ApiSample level, removed subSamplesInContainer array, and instead added isStoredInContainer flag to ApiSubSampleInfo model, which means it can be read from existing ApiSample.subSamples array.

@rlamacraft that needs adjusting frontend code to new logic, basically in /ui/src/stores/models/Search.js the alert generation code should check canBeDeleted flag on sample level (as before), then (if flag is false) iterate over subSamples array, and only list subSamples with isStoredInContainer === true.

@rlamacraft
Copy link
Contributor

Oh right yeah, each sample might contain a mixture of subsamples that can or cannot be deleted. Ok, made the change

Copy link
Contributor

@mKowalski256 mKowalski256 left a comment

Choose a reason for hiding this comment

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

Looks good to me & tests are passing, @rs-nicof feel free to merge when you're back on Monday.

@rs-nicof rs-nicof merged commit c2cab3c into rspace-os:main Jul 22, 2024
2 checks passed
@rs-nicof rs-nicof deleted the RSDEV-280-backend branch July 22, 2024 14:25
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2024
@mKowalski256 mKowalski256 restored the RSDEV-280-backend branch July 22, 2024 20:47
@mKowalski256 mKowalski256 deleted the RSDEV-280-backend branch July 22, 2024 20:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants