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

AG-1173 added TEP logic to gene hero #1229

Merged
merged 3 commits into from
Aug 24, 2023
Merged

AG-1173 added TEP logic to gene hero #1229

merged 3 commits into from
Aug 24, 2023

Conversation

sagely1
Copy link
Contributor

@sagely1 sagely1 commented Aug 22, 2023

No description provided.

Copy link
Contributor

@hallieswan hallieswan left a comment

Choose a reason for hiding this comment

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

Looks good! Just looks like the new mock may need to be tweaked

Comment on lines 49 to 57
it('should only add TEP text if nominations is null and either is_tep or is_adi is true', () => {
component.gene = geneMock3;
fixture.detectChanges();

const el = element.querySelector('.gene-hero-nominated') as HTMLElement;

const expected = 'Selected for Target Enabling Resource Development';
expect(el.textContent).toBe(expected);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice tests! You could also consider adding tests to show that 1) only "Nominated Target" is shown when is_tep and is_adi are both false, and 2) nothing is displayed when nominations is null and both is_tep and is_adi are false. Up to you though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My latest commit should have these missing tests. thanks!

is_tep: true
};

export const geneMock3: Gene = {
Copy link
Contributor

Choose a reason for hiding this comment

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

From the build log, it looks like this mock might be missing nominatedtarget and nominations fields:

ERROR in src/app/testing/gene-mocks.ts:3153:14 - error TS2739: Type '{ _id: string; ensembl_gene_id: string; name: string; 
summary: string; hgnc_symbol: string; alias: string[]; isIGAP: false; haseqtl: true; isAnyRNAChangedInADBrain: true; 
rna_brain_change_studied: true; ... 5 more ...; is_tep: true; }' is missing the following properties from type 'Gene':
nominatedtarget, nominations

3153 export const geneMock3: Gene = {
                  ~~~~~~~~~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I didn't commit all my changes. Good catch!

Copy link
Contributor

@hallieswan hallieswan left a comment

Choose a reason for hiding this comment

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

Looks good! Could also add a test to check when nominations is null and both is_adi and is_tep are false, but up to you!

@sagely1
Copy link
Contributor Author

sagely1 commented Aug 24, 2023

Looks good! Could also add a test to check when nominations is null and both is_adi and is_tep are false, but up to you!

Added border condition unit test as per @hallieswan

@sagely1 sagely1 merged commit fff3352 into Sage-Bionetworks:develop Aug 24, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants