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

[BI-1923] Treatment factor missing from experimental ui #332

Merged
merged 8 commits into from
Oct 3, 2023

Conversation

davedrp
Copy link
Contributor

@davedrp davedrp commented Sep 21, 2023

Description

Story: BI-1923 - Treatment factor missing from experimental ui

Dependencies

bi-api: develop branch

Testing

Import an experiment with treatment factor.
Display it.
There should now be a Treatment Factors column in the Observation Dataset (with data)

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • I have create/modified unit tests to cover this change
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to documentation
  • I have run TAF: <link to TAF run>

@davedrp davedrp requested review from a team, timparsons and dmeidlin and removed request for a team September 21, 2023 19:39
@github-actions github-actions bot added the bug Something isn't working label Sep 21, 2023
@davedrp davedrp marked this pull request as ready for review September 21, 2023 19:39
Comment on lines 411 to 413
if(unit.additionalInfo && unit.additionalInfo.treatments && unit.additionalInfo.treatments && unit.additionalInfo.treatments[0]){
datasetTableRow.treatmentFactors = unit.additionalInfo.treatments[0].factor;
}
Copy link
Member

Choose a reason for hiding this comment

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

While treatment factors are stored for persistence to the brapi storage server, they are translated back to being in unit.treatments in ObservationUnitDAO.java. This code should reference that location in the ObservationUnit object rather than in additionalInfo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@timparsons timparsons changed the base branch from develop to release/0.8.1 September 22, 2023 19:24
@davedrp davedrp requested a review from timparsons September 28, 2023 21:14
package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "bi-web",
"version": "v0.8.1+585",
"version": "v0.9.0+589",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you rebased from the develop branch. Can the two changes in the file be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -34,7 +34,7 @@ export class ObservationUnit {
observationUnitPosition?: ObservationUnitPosition;
externalReferences?: ExternalReferences;
additionalInfo?: any;
treatments?: any;
treatments?: string[];
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed to an array of strings instead of leaving it as any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Changed.
Upon review of BrAPI, 'any' is the preferred type.

@davedrp davedrp requested a review from timparsons September 29, 2023 20:06
@davedrp davedrp closed this Oct 3, 2023
@davedrp
Copy link
Contributor Author

davedrp commented Oct 3, 2023

Re-opening because it "Closed with unmerged commits"

@davedrp davedrp reopened this Oct 3, 2023
@davedrp davedrp merged commit 87faca9 into release/0.8.1 Oct 3, 2023
@davedrp davedrp deleted the bug/BI-1923 branch October 3, 2023 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants