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

Fix the allele frequency issue in multi-sample patient view #3950

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

zhx828
Copy link
Member

@zhx828 zhx828 commented Aug 26, 2021

This is to fix an issue introduced by #3931

The allele frequency is not shown when the mutation only exists for one sample in the multi-sample patient view.

@zhx828 zhx828 marked this pull request as ready for review August 26, 2021 19:45
@@ -34,4 +35,24 @@ describe('patient page', function() {
'show all 4 samples'.toLowerCase()
);
});

it('should show number under allele frequency column for multiple samples patient in one sample view', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it would be better to make these tests not screenshot tests. screenshots can be so annoying to deal with and update, and flakey, and this is a situation that is really suited to just checking whether an element appears and if its value is what we expect it is.

src/pages/patientView/SampleManager.tsx Show resolved Hide resolved
Signed-off-by: Hongxin <5400599+zhx828@users.noreply.github.com>
@zhx828
Copy link
Member Author

zhx828 commented Aug 30, 2021

The failing tests are irrelevant.

Copy link
Contributor

@adamabeshouse adamabeshouse left a comment

Choose a reason for hiding this comment

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

perfect, thanks!!

@zhx828 zhx828 merged commit b79781b into cBioPortal:master Sep 1, 2021
@zhx828 zhx828 deleted the update-allele-freq branch September 1, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants