Skip to content

Fix analyses from partitions are considered in manage analyses view#2598

Draft
xispa wants to merge 11 commits into
2.xfrom
fix-cannot-remove-partitions
Draft

Fix analyses from partitions are considered in manage analyses view#2598
xispa wants to merge 11 commits into
2.xfrom
fix-cannot-remove-partitions

Conversation

@xispa

@xispa xispa commented Jul 29, 2024

Copy link
Copy Markdown
Member

Description of the issue/feature this PR addresses

This Pull Request comes as an answer to #2593 (comment) by ensuring that from "Manage analyses" view is only possible to add/remove analyses that belong to current sample, leaving analyses from partitions out of the equation. Likewise, analyses from parent sample are not being considered when managing analyses from a partition.

Current behavior before PR

Manage analyses view takes analyses from partitions into account

Desired behavior after PR is merged

Manage analyses view does not take analyses from partitions into account

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

@xispa xispa requested a review from ramonski July 29, 2024 09:21
@xispa xispa added the Bug 🐞 Something is broken label Jul 29, 2024
@xispa xispa marked this pull request as draft July 29, 2024 09:37
@xispa xispa marked this pull request as ready for review July 29, 2024 10:32
@xispa xispa marked this pull request as draft July 29, 2024 14:10
@xispa xispa marked this pull request as ready for review November 2, 2024 16:58
@ramonski

Copy link
Copy Markdown
Contributor

I think there was a concern regarding this PR, but I don't see it in the comments here. Can you remember? I'll check it otherwise later.

@ramonski

Copy link
Copy Markdown
Contributor

Ah, probably that you cannot enter anymore results from the root sample, but need to go directly to the partition instead 🤔

@xispa

xispa commented Jan 31, 2025

Copy link
Copy Markdown
Member Author

Ah, probably that you cannot enter anymore results from the root sample, but need to go directly to the partition instead 🤔

It is possible to enter results from the root sample. See below:

Captura de 2025-01-31 20-08-13

I've been playing around and I've realized that if you have a given test (e.g. Ca) in a partition, you cannot longer add another test Ca in the root sample. You can select the checkbox in manage analysis, and press "Save" afterwards, but nothing happens.

@xispa

xispa commented Jan 31, 2025

Copy link
Copy Markdown
Member Author

The default behaviour before this PR when adding a service to a partition via "Manage Analyses" is that the system first checks for a counterpart analysis in the root sample. If found, it moves the analysis to the partition instead of creating a new one.

This is ok for when a partition is being created, but once created, user should be able to add analyses in the partition regardless of the root. Likewise, the addition of analyses in root should not entail changes in the partition. Since this "dependency" logic is encoded in the datamanager, is not possible to make "Manage analyses" work properly for root sample and partitions without breaking the partition creation functionality.

Therefore, I think that the logic for moving analyses from root to partitions should not reside in the datamanager, but explicitly handled on partition creation.

Will revisit and modify this PR and apply the changes in accordance to ensure system works as expected in both scenarios.

Related: #1505

@xispa xispa marked this pull request as draft January 31, 2025 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug 🐞 Something is broken

Development

Successfully merging this pull request may close these issues.

2 participants