-
Notifications
You must be signed in to change notification settings - Fork 0
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
Edit templates for measurement registration #832
Edit templates for measurement registration #832
Conversation
Fixes the edit case and renames instrument to MS device fixes #786
Uses the enum and enhanced switch to make sure all columns are covered and removes complexity.
Excel protected the visible sheet. I do not know why and can only speculate that some reordering was not complete. Creating the hidden sheet second solves the issue.
Not an issue as the Random can be insecure in this case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heya @KochTobi! Awesome work, i see the benefit of a helper class. However i found an issue during registration and editing of NGS measurements and have some questions, for which a quick answer from your side would be awesome 😄
@@ -57,7 +57,10 @@ public class ProteomicsMeasurement { | |||
private MeasurementId measurementId; | |||
|
|||
@Column(name = "instrument", columnDefinition = "longtext CHECK (json_valid(`instrument`))") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this change also be reflected in the db?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but this would break all existing measurements. So not sure yet how to go about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably best to keep as is right now.
...anager/views/projects/project/measurements/download/NGSMeasurementContentProviderTest.groovy
Outdated
Show resolved
Hide resolved
...anager/views/projects/project/measurements/download/NGSMeasurementContentProviderTest.groovy
Outdated
Show resolved
Hide resolved
user-interface/src/main/java/life/qbic/datamanager/parser/MetadataConverter.java
Outdated
Show resolved
Hide resolved
lockSheet(hiddenSheet); | ||
hideSheet(workbook, hiddenSheet); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused, why do we need a seperate hidden sheet now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selection of possible values is stored in this hidden sheet. The values are included in a named area in this hidden sheet. This named area is used as formula for the dropdown.https://thecodeshewrites.com/2020/08/11/apache-poi-excel-java-dropdown-list-dependent/
Co-authored-by: steffengreiner <[email protected]>
Co-authored-by: steffengreiner <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heya @KochTobi nice changes, sadly i think one column header is not correct blocking the ngs edit/registration
@@ -327,6 +342,7 @@ public String propertyName() { | |||
|
|||
enum NGSMeasurementProperty { | |||
MEASUREMENT_ID("measurement id"), | |||
SAMPLE_CODE("sample code"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property does not exist in the template sheets for ngs registration and editing, which leads to the issue that no measurements can be registered or edited.
Did you mean QBiC Sample Id
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I did. The issue with the wrong properties being used seems to have been in the code already. It should be fixed now.
Co-authored-by: steffengreiner <[email protected]>
Quality Gate failedFailed conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done 👍
Introduces class to generate drop-down lists in excel.
Further
addresses #823 for the edit case
addresses #747
addresses #786