Skip to content

BI-1911 fix QA findings#341

Merged
davedrp merged 2 commits intorelease/0.8.1from
bug/BI-1911-2
Oct 30, 2023
Merged

BI-1911 fix QA findings#341
davedrp merged 2 commits intorelease/0.8.1from
bug/BI-1911-2

Conversation

@davedrp
Copy link
Contributor

@davedrp davedrp commented Oct 20, 2023

Description

BI-1911 - in response to Shahana's testing notes.

Dependencies

bi-api: bug/BI-1911-2 branch

Testing

  • Import an Experiment.
  • Look at the confirmation page.

Expected Results

  • The Env Year, Env, and Exp unit ID columns should appear as they did in the import file.

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 mlm483 and timparsons October 20, 2023 16:09
@github-actions github-actions bot added the bug Something isn't working label Oct 20, 2023
Comment on lines +259 to +264
removeUnique(str: string|undefined): string {
if(!str){ return "";}
str = str.trim();
const reg = /\[[^\]]*\]$/;
return str.replace(reg, '').trim();
}
Copy link
Member

Choose a reason for hiding this comment

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

For v0.8.1 I don't see a problem leaving this code in there, but I think it should be handled in bi-api. Could look to fix this in v0.9

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, we have static Utilities methods to remove program keys in bi-api. If it's not too much effort, I would make the change now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new story for this (BI-1973) has been written.

Copy link
Contributor

@mlm483 mlm483 left a comment

Choose a reason for hiding this comment

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

Functionality looks good, I would strip the program key in bi-api.

Comment on lines +259 to +264
removeUnique(str: string|undefined): string {
if(!str){ return "";}
str = str.trim();
const reg = /\[[^\]]*\]$/;
return str.replace(reg, '').trim();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, we have static Utilities methods to remove program keys in bi-api. If it's not too much effort, I would make the change now.

@davedrp davedrp merged commit 2ea6610 into release/0.8.1 Oct 30, 2023
@davedrp davedrp deleted the bug/BI-1911-2 branch October 30, 2023 15:04
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.

3 participants