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

QC Analyses listing appears empty in Sample view #1512

Merged
merged 9 commits into from
Jan 28, 2020
Merged

Conversation

xispa
Copy link
Member

@xispa xispa commented Jan 27, 2020

Description of the issue/feature this PR addresses

When QC Analyses (controls, blanks, duplicates) are added in a worksheet, they should be listed in QC Analyses section from inside Sample view if the worksheet contains at least one analysis of the latter.

Current behavior before PR

QC Analyses listing appears empty

Desired behavior after PR is merged

Captura de 2020-01-27 23-46-39

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

@xispa xispa added Bug 🐞 Cleanup 🧹 Code cleanup and refactoring labels Jan 27, 2020
@xispa xispa requested a review from ramonski January 27, 2020 21:50
})

# Add Worksheet and QC Sample ID columns
new_columns = OrderedDict((
Copy link
Contributor

Choose a reason for hiding this comment

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

I would do it the other way around. Put this into the __init__ and flush it eventually when there are no QC analyses in update.

Copy link
Contributor

@ramonski ramonski left a comment

Choose a reason for hiding this comment

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

👍

@ramonski ramonski merged commit 12c5a26 into master Jan 28, 2020
@xispa xispa deleted the qc-analyses-sample branch January 28, 2020 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐞 Cleanup 🧹 Code cleanup and refactoring P1: Urgent
Development

Successfully merging this pull request may close these issues.

2 participants