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: share link and test #6385

Merged
merged 1 commit into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,11 @@ function getNewSelectedFilters({
Object.entries(tissues ?? {}).map(([id, tissue]) => [tissue.name, id])
);

const allTissueNames = Object.keys(tissueIdsByName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

allTissueNames = Object.keys(tissueIdsByName); was returning an empty array, since Object.keys() doesn't work with Map! TIL πŸ’‘

This fixes the bug where share link tissue with tissue name won't get populated in the app state

/**
* (thuang): Warning - Map doesn't work with Object.keys(), so we need to use
* Map.keys() instead
*/
const allTissueNames = Array.from(tissueIdsByName.keys());
const allTissueIds = Object.keys(tissues ?? {});

Object.keys(selectedFilters).forEach((key) => {
Expand All @@ -235,6 +239,7 @@ function getNewSelectedFilters({
const tissueParams = value.split(delimiter);
const tissueIds = [];
const tissueNames = [];

for (const tissueParam of tissueParams) {
if (
tissueParam.includes("UBERON:") &&
Expand Down
51 changes: 26 additions & 25 deletions frontend/tests/features/wheresMyGene/shareLink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,11 @@

const GENES = ["DPM1", "TNMD", "TSPAN6"];

const DATASETS = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we've removed datasets as a filter option in the app

const PUBLICATIONS = [
// (thuang): This publication has blood and lung tissues
{
id: "d8da613f-e681-4c69-b463-e94f5e66847f",
text: "A molecular single-cell lung atlas of lethal COVID-19",
},
{
id: "de2c780c-1747-40bd-9ccf-9588ec186cee",
text: "Immunophenotyping of COVID-19 and influenza highlights the role of type I interferons in development of severe COVID-19",
id: "Ren et al. Cell 2021",
text: "Ren et al. (2021) Cell",
},
];

Expand All @@ -53,8 +50,8 @@
const SHARE_LINK_SEARCH_PARAMS = new URLSearchParams();
SHARE_LINK_SEARCH_PARAMS.set("compare", COMPARE);
SHARE_LINK_SEARCH_PARAMS.set(
"datasets",
DATASETS.map((dataset) => dataset.id).join()
"publications",
PUBLICATIONS.map((publication) => publication.id).join()
);
SHARE_LINK_SEARCH_PARAMS.set(
"diseases",
Expand Down Expand Up @@ -113,7 +110,7 @@
});
});

test.skip("Should generate share link with correct format for all query param types", async ({
test("Should generate share link with correct format for all query param types", async ({
page,
browserName,
}) => {
Expand All @@ -130,8 +127,7 @@
linkVersion: LATEST_SHARE_LINK_VERSION,
tissueIds,
genes: GENES,
// TODO(seve): #6131 test is currently failing on dataset param, should investigate and reenable
// datasets: DATASETS,
publications: PUBLICATIONS,
sexes: SEXES,
diseases: DISEASES,
ethnicities: ETHNICITIES,
Expand Down Expand Up @@ -168,7 +164,7 @@
linkVersion,
tissueIds,
genes,
datasets,
publications,
sexes,
diseases,
ethnicities,
Expand All @@ -179,7 +175,7 @@
linkVersion: string;
tissueIds?: string[];
genes?: string[];
datasets?: ExpectedParam[];
publications?: ExpectedParam[];
sexes?: ExpectedParam[];
diseases?: ExpectedParam[];
ethnicities?: string[];
Expand All @@ -195,6 +191,11 @@
"navigator.clipboard.readText()"
);

/**
* (thuang): The param order below needs to match the order from the ShareButton
* component
*/

// split parameters
const urlParams = new URLSearchParams(
// (thuang): We only want the query params part of the URL, so we split by "?"
Expand All @@ -210,15 +211,6 @@
searchParams.set(param, compare);
}

// datasets
if (datasets !== undefined) {
const param = "datasets";

const data = await verifyParameter(page, urlParams, param, datasets);

searchParams.set(param, String(data));
}

// diseases
if (diseases !== undefined) {
const param = "diseases";
Expand All @@ -237,6 +229,15 @@
searchParams.set(param, String(data));
}

// publications
if (publications !== undefined) {
const param = "publications";

const data = await verifyParameter(page, urlParams, param, publications);

searchParams.set(param, String(data));
}

// sexes
if (sexes !== undefined) {
const param = "sexes";
Expand Down Expand Up @@ -299,17 +300,17 @@
const expectedIds = expectedParams.map((expectedParam) => expectedParam.id);

switch (param) {
case "datasets": {
case "publications": {
const paramValues = getParamValues(param);

// verify datasets have been selected
// verify publications have been selected
paramValues.forEach(async (_id: string) => {
const item = expectedParams.find(
(expectedParam) => expectedParam.id === _id
);

if (item) {
await expect(page.getByText(item.text)).toBeVisible();

Check failure on line 313 in frontend/tests/features/wheresMyGene/shareLink.test.ts

View workflow job for this annotation

GitHub Actions / e2e-tests chromium 9 of 10

[chromium] β€Ί tests/features/wheresMyGene/shareLink.test.ts:113:7 β€Ί Share link tests β€Ί Should generate share link with correct format for all query param types

1) [chromium] β€Ί tests/features/wheresMyGene/shareLink.test.ts:113:7 β€Ί Share link tests β€Ί Should generate share link with correct format for all query param types Error: expect(locator).toBeVisible() Locator: getByText('Ren et al. (2021) Cell') Expected: visible Received: hidden Call log: - expect.toBeVisible with timeout 10000ms - waiting for getByText('Ren et al. (2021) Cell') 311 | 312 | if (item) { > 313 | await expect(page.getByText(item.text)).toBeVisible(); | ^ 314 | } 315 | }); 316 | at forEach (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:313:51) at verifyParameter (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:307:19) at verifyShareLink (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:236:24) at page.page (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:125:9) at tryUntil (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/utils/helpers.ts:163:7) at /home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:123:5

Check failure on line 313 in frontend/tests/features/wheresMyGene/shareLink.test.ts

View workflow job for this annotation

GitHub Actions / e2e-tests chromium 9 of 10

[chromium] β€Ί tests/features/wheresMyGene/shareLink.test.ts:113:7 β€Ί Share link tests β€Ί Should generate share link with correct format for all query param types

1) [chromium] β€Ί tests/features/wheresMyGene/shareLink.test.ts:113:7 β€Ί Share link tests β€Ί Should generate share link with correct format for all query param types Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() Locator: getByText('Ren et al. (2021) Cell') Expected: visible Received: hidden Call log: - expect.toBeVisible with timeout 10000ms - waiting for getByText('Ren et al. (2021) Cell') 311 | 312 | if (item) { > 313 | await expect(page.getByText(item.text)).toBeVisible(); | ^ 314 | } 315 | }); 316 | at forEach (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:313:51) at verifyParameter (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:307:19) at verifyShareLink (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:236:24) at page.page (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:125:9) at tryUntil (/home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/utils/helpers.ts:163:7) at /home/runner/work/single-cell-data-portal/single-cell-data-portal/frontend/tests/features/wheresMyGene/shareLink.test.ts:123:5
}
});

Expand Down
Loading