From be019f33777e51edaf70880ab375de5d9bd6d525 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 24 Sep 2024 16:00:35 -0400 Subject: [PATCH] [ALS-6937] Support open access users for actions (#188) --- src/lib/assets/configuration.json | 29 +++- src/lib/components/Stats.svelte | 73 +++++++++-- src/lib/stores/Stats.ts | 43 ------ src/lib/types.ts | 3 + src/routes/(picsure)/+page.svelte | 20 ++- tests/mock-data.ts | 28 +++- tests/routes/api/test.ts | 6 +- .../explorer/genomic-filter/gene-test.ts | 2 +- tests/routes/help/test.ts | 6 +- tests/routes/landing/test.ts | 124 ++++++++++++------ tests/routes/login/test.ts | 3 +- 11 files changed, 232 insertions(+), 105 deletions(-) delete mode 100644 src/lib/stores/Stats.ts diff --git a/src/lib/assets/configuration.json b/src/lib/assets/configuration.json index 4ffc3a57..abb5741d 100644 --- a/src/lib/assets/configuration.json +++ b/src/lib/assets/configuration.json @@ -39,7 +39,7 @@ "footer": { "showTerms": true, "showSitemap": true, - "excludeSitemapOn": ["/explorer"], + "excludeSitemapOn": ["/explorer", "/discover"], "links": [ { "title": "Privacy Policy", @@ -71,20 +71,43 @@ "landing": { "searchPlaceholder": "Search terms or variables of interest…", "explanation": "The National Health and Nutrition Examination Survey (NHANES) dataset is designed to assess the health and nutritional status of adults and children in the United States", + "authExplanation": "The National Health and Nutrition Examination Survey (NHANES) dataset is designed to assess the health and nutritional status of adults and children in the United States", "actions": [ + { + "title": "Discover", + "description": "Discover studies in BioData Catalyst by searching for terms, applying filters, and retrieving aggregate counts", + "icon": "fa-regular fa-compass", + "url": "/discover", + "btnText": "Start Discovering", + "isOpen": true, + "showIfLoggedIn": true + }, { "title": "Explore", "description": "Explore data, apply filters, and build cohorts", "icon": "fa-solid fa-magnifying-glass", "url": "/explorer", - "btnText": "Start Exploring" + "btnText": "Start Exploring", + "isOpen": false, + "showIfLoggedIn": true }, { "title": "Prepare for Analysis", "description": "Use the PIC-SURE API to prepare data for analysis", "icon": "fa-solid fa-chart-line", "url": "/analyze", - "btnText": "Prepare for Analysis" + "btnText": "Prepare for Analysis", + "isOpen": false, + "showIfLoggedIn": true + }, + { + "title": "Login", + "description": "Log In to explore participant-level data, save datasets, and kickstart your research analysis", + "icon": "fa-solid fa-user", + "url": "/login", + "btnText": "Go to Login", + "isOpen": true, + "showIfLoggedIn": false } ], "stats": ["Variables", "Participants", "Data Sources"] diff --git a/src/lib/components/Stats.svelte b/src/lib/components/Stats.svelte index 389eef4b..f2dc18e8 100644 --- a/src/lib/components/Stats.svelte +++ b/src/lib/components/Stats.svelte @@ -1,23 +1,74 @@ @@ -48,7 +99,7 @@ {/each}
- {@html branding?.landing?.explanation} + {@html isUserLoggedIn() ? branding?.landing?.authExplanation : branding?.landing?.explanation}
{#if hasError}
Promise }; - -const isUserLoggedIn = () => { - if (browser) { - return !!localStorage.getItem('token'); - } - return false; -}; - -const apiMap: ApiMap = { - 'Data Sources': () => - getStudiesCount(!isUserLoggedIn()).then((response) => { - if (response) { - return response.toLocaleString(); - } - return ERROR_VALUE; - }), - Participants: () => - api - .post( - 'picsure/query/sync', - getQueryRequest(isUserLoggedIn(), isUserLoggedIn() ? resources.hpds : resources.openHPDS), - ) - .then((response) => response.toLocaleString()), - Variables: () => - getConceptCount(!isUserLoggedIn()).then((response) => { - if (response) { - return response.toLocaleString(); - } - return ERROR_VALUE; - }), -}; - -export async function getOrApi(key: string): Promise { - return apiMap[key](); -} diff --git a/src/lib/types.ts b/src/lib/types.ts index 88b17ef3..63f64f02 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -47,12 +47,15 @@ export interface ExplorePageConfig { export interface LandingConfig { searchPlaceholder: string; explanation: string; + authExplanation: string; actions: Array<{ title: string; description: string; icon: string; url: string; btnText: string; + isOpen: boolean; + showIfLoggedIn: boolean; }>; stats: string[]; } diff --git a/src/routes/(picsure)/+page.svelte b/src/routes/(picsure)/+page.svelte index cf74bda1..3ec9f27b 100644 --- a/src/routes/(picsure)/+page.svelte +++ b/src/routes/(picsure)/+page.svelte @@ -3,11 +3,27 @@ import { goto } from '$app/navigation'; import Searchbox from '$lib/components/Searchbox.svelte'; import Stats from '$lib/components/Stats.svelte'; + import { browser } from '$app/environment'; let searchTerm = ''; + const isUserLoggedIn = () => { + if (browser) { + return !!localStorage.getItem('token'); + } + return false; + }; + function search() { goto(`/explorer?search=${searchTerm}`); } + + const actionsToDisplay = branding?.landing?.actions.filter((action) => { + if (isUserLoggedIn()) { + return action.showIfLoggedIn; + } else { + return action.isOpen || !action.showIfLoggedIn; + } + }); @@ -25,11 +41,11 @@ id="actions-section" class="flex flex-row justify-evenly items-center w-2/3 mt-auto mb-8" > - {#each branding?.landing?.actions as { title, description, icon, url, btnText }} + {#each actionsToDisplay as { title, description, icon, url, btnText }}
{title}
-
{description}
+
{description}
{btnText} diff --git a/tests/mock-data.ts b/tests/mock-data.ts index 3f5c78a4..d9d9fa6e 100644 --- a/tests/mock-data.ts +++ b/tests/mock-data.ts @@ -117,7 +117,7 @@ export const searchRequest = { facets: [], search: 'age' }; export const searchResults = { totalPages: 1, - totalElements: 4, + totalElements: 7, pageable: { pageNumber: 0, pageSize: 10, @@ -456,7 +456,6 @@ export const detailResponseNum = { description: 'Age', }, }; - export const facetsResponse = [ { name: 'study_ids_dataset_ids', @@ -573,6 +572,31 @@ export const facetsResponse = [ }, ], }, + { + name: 'dataset_id', + display: 'Study IDs/Dataset IDs', + description: '', + facets: [ + { + name: 'Test For Landing', + display: 'Test', + description: 'dataset_id study', + count: 1, + children: null, + category: 'dataset_id', + meta: null, + }, + { + name: 'Test For Landing 2', + display: 'Test 2', + description: 'dataset_id study', + count: 1, + children: null, + category: 'dataset_id', + meta: null, + }, + ], + }, { name: 'another_category_name', display: 'Another Category Name', diff --git a/tests/routes/api/test.ts b/tests/routes/api/test.ts index 61161e29..38d54cf3 100644 --- a/tests/routes/api/test.ts +++ b/tests/routes/api/test.ts @@ -1,7 +1,11 @@ import { expect } from '@playwright/test'; import { test, mockApiFail, mockApiSuccess } from '../../custom-context'; -import { branding } from '../../../src/lib/configuration'; import { picsureUser, roles as mockRoles, mockExpiredToken } from '../../../tests/mock-data'; +import type { Branding } from '../../../src/lib/configuration'; +import * as config from '../../../src/lib/assets/configuration.json' assert { type: 'json' }; +//TypeScript is confused by the JSON import so I am fxing it here +/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ +const branding: Branding = JSON.parse(JSON.stringify((config as any).default)); const placeHolderDots = '••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••'; diff --git a/tests/routes/explorer/genomic-filter/gene-test.ts b/tests/routes/explorer/genomic-filter/gene-test.ts index b1098530..959a83fa 100644 --- a/tests/routes/explorer/genomic-filter/gene-test.ts +++ b/tests/routes/explorer/genomic-filter/gene-test.ts @@ -4,7 +4,7 @@ import { geneValues } from '../../../mock-data'; import * as config from '../../../../src/lib/assets/configuration.json' assert { type: 'json' }; import type { Branding } from '$lib/configuration'; //TypeScript is confused by the JSON import so I am fxing it here -/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ const branding: Branding = JSON.parse(JSON.stringify((config as any).default)); const HPDS = process.env.VITE_RESOURCE_HPDS; diff --git a/tests/routes/help/test.ts b/tests/routes/help/test.ts index 07c5f179..aea3fcd6 100644 --- a/tests/routes/help/test.ts +++ b/tests/routes/help/test.ts @@ -1,5 +1,9 @@ import { expect, test } from '@playwright/test'; -import { branding } from '../../../src/lib/configuration'; +import type { Branding } from '../../../src/lib/configuration'; +import * as config from '../../../src/lib/assets/configuration.json' assert { type: 'json' }; +//TypeScript is confused by the JSON import so I am fxing it here +/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ +const branding: Branding = JSON.parse(JSON.stringify((config as any).default)); test.describe('Help page', () => { branding?.help?.links?.forEach(({ title }) => { diff --git a/tests/routes/landing/test.ts b/tests/routes/landing/test.ts index 5a609bdc..ea6e932c 100644 --- a/tests/routes/landing/test.ts +++ b/tests/routes/landing/test.ts @@ -1,47 +1,43 @@ import { expect, type Route, type Page } from '@playwright/test'; -import { test, mockApiSuccess } from '../../custom-context'; -import { branding } from '../../../src/lib/configuration'; +import { test, mockApiSuccess, unauthedTest } from '../../custom-context'; import { searchResults as mockSearchResults, searchResultPath, datasets as mockDatasets, + facetsResponse, + searchResults, } from '../../mock-data'; +import type { Branding } from '../../../src/lib/configuration'; +import * as config from '../../../src/lib/assets/configuration.json' assert { type: 'json' }; +//TypeScript is confused by the JSON import so I am fxing it here +/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ +const branding: Branding = JSON.parse(JSON.stringify((config as any).default)); +const loggedInActions = branding?.landing?.actions?.filter((action) => action.showIfLoggedIn); +const loggedOutActions = branding?.landing?.actions?.filter( + (action) => !action.showIfLoggedIn || action.isOpen, +); const HPDS = process.env.VITE_RESOURCE_HPDS; -const mockStatsRoutesSuccess: { - [key: string]: { mock: (context: Page) => Promise; expected: string }; -} = { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - 'Data Sources': { mock: (_context: Page) => Promise.resolve(), expected: '1' }, - Participants: { - mock: (context: Page) => - context.route('*/**/picsure/query/sync', (route: Route) => route.fulfill({ json: '53' })), - expected: '53', - }, - Variables: { - mock: (context: Page) => - context.route(`*/**/picsure/search/${HPDS}`, (route: Route) => - route.fulfill({ json: { results: { phenotypes: { pheno1: {}, peno2: {} } } } }), - ), - expected: '2', - }, -}; - const mockStatsRoutesFail: { [key: string]: (context: Page) => Promise } = { Participants: (context: Page) => context.route('*/**/picsure/query/sync', (route: Route) => route.abort('failed')), Variables: (context: Page) => context.route(`*/**/picsure/search/${HPDS}`, (route: Route) => route.abort('failed')), + 'Data Sources': (context: Page) => + context.route('*/**/picsure/proxy/dictionary-api/facets/', (route: Route) => + route.abort('failed'), + ), }; -function mockAllStats(context: Page) { - return Promise.all(Object.values(mockStatsRoutesSuccess).map((route) => route.mock(context))); -} +const mockStatsRoutesSuccess = new Map([ + ['Participants', '1000'], + ['Variables', searchResults.totalElements.toLocaleString()], + ['Data Sources', facetsResponse[1].facets.length.toLocaleString()], +]); test.describe('Landing page', () => { test.describe('Search', () => { - test.beforeEach(({ page }) => mockAllStats(page)); test('Has expected search to go to explorer', async ({ page }) => { // Given await mockApiSuccess(page, searchResultPath, mockSearchResults); @@ -67,7 +63,13 @@ test.describe('Landing page', () => { branding?.landing?.stats?.forEach((stat) => { test(`Has expected stat of ${stat}`, async ({ page }) => { // Given - await mockAllStats(page); + mockApiSuccess(page, '*/**/picsure/proxy/dictionary-api/facets/', facetsResponse); + mockApiSuccess( + page, + '*/**/picsure/proxy/dictionary-api/concepts?page_number=1&page_size=1', + searchResults, + ); + mockApiSuccess(page, '*/**/picsure/query/sync', '1000'); await page.goto('/'); // Then @@ -75,22 +77,22 @@ test.describe('Landing page', () => { }); test(`Has expected stat value for ${stat}`, async ({ page }) => { // Given - await mockAllStats(page); + mockApiSuccess(page, '*/**/picsure/proxy/dictionary-api/facets/', facetsResponse); + mockApiSuccess( + page, + '*/**/picsure/proxy/dictionary-api/concepts?page_number=1&page_size=1', + searchResults, + ); + mockApiSuccess(page, '*/**/picsure/query/sync', '1000'); await page.goto('/'); // Then await expect(page.getByTestId(`value-${stat}`)).toHaveText( - mockStatsRoutesSuccess[stat].expected, + mockStatsRoutesSuccess.get(stat) || '-', ); }); test(`Should display error message if api returns error for ${stat}`, async ({ page }) => { - if (stat === 'Data Sources') { - test.skip(); - // Right now, data sources resolves to 1 always - // TODO: update when data sources is not always 1 - } // Given - await mockAllStats(page); await mockStatsRoutesFail[stat](page); await page.goto('/'); @@ -103,15 +105,16 @@ test.describe('Landing page', () => { }); }); test.describe('Actions', () => { - test.beforeEach(({ page }) => mockAllStats(page)); - branding?.landing?.actions?.forEach(({ description, icon, url, title }) => { - test(`Has expected action of description: ${description}`, async ({ page }) => { + loggedInActions.forEach(({ description, icon, url, title }) => { + test(`Logged In user Has expected action of description: ${description}`, async ({ + page, + }) => { // Given await page.goto('/'); // Then await expect(page.getByText(description, { exact: true })).toBeVisible(); }); - test(`Has expected icon of: ${icon}`, async ({ page }) => { + test(`Logged In user Has expected icon of: ${icon}`, async ({ page }) => { // Given await page.goto('/'); // Then @@ -120,7 +123,7 @@ test.describe('Landing page', () => { await expect(iconElement).toBeVisible(); await expect(iconElement).toHaveClass(pattern); }); - test(`Card "${description}"'s click leads to ${url}`, async ({ page }) => { + test(`Action button "${description}"'s click leads to ${url}`, async ({ page }) => { // Given await mockApiSuccess(page, '*/**/picsure/dataset/named', mockDatasets); await page.goto('/'); @@ -146,3 +149,46 @@ test.describe('Landing page', () => { }); }); }); + +unauthedTest.describe('Logged Out Landing', () => { + loggedOutActions.forEach(({ description, icon, url, title }) => { + unauthedTest(`Has expected action of description: ${description}`, async ({ page }) => { + // Given + await page.goto('/'); + // Then + await expect(page.getByText(description, { exact: true })).toBeVisible(); + }); + unauthedTest(`Has expected icon of: ${icon}`, async ({ page }) => { + // Given + await page.goto('/'); + // Then + const iconElement = page.locator(`.${icon.replaceAll(' ', '.')}`); + const pattern = new RegExp(`.*${icon}.*`); + await expect(iconElement).toBeVisible(); + await expect(iconElement).toHaveClass(pattern); + }); + unauthedTest(`Button "${description}"'s click leads to ${url}`, async ({ page }) => { + // Given + await mockApiSuccess(page, '*/**/picsure/dataset/named', mockDatasets); + await page.goto('/'); + + // When + const action = page.getByTestId(`landing-action-${title}-btn`); + await action.isVisible(); + + // Then + if ((await action.getAttribute('target')) !== '_blank') { + await action.click(); + await expect(page).toHaveURL(`${url}`); + } else { + //check if new tab opened + const newTabPromise = page.waitForEvent('popup'); + await action.click(); + const newPage = await newTabPromise; + await newPage.waitForLoadState(); + await expect(newPage).not.toBeNull(); + await expect(newPage).toHaveURL(`${url}`); + } + }); + }); +}); diff --git a/tests/routes/login/test.ts b/tests/routes/login/test.ts index 3c5698df..2c3484bd 100644 --- a/tests/routes/login/test.ts +++ b/tests/routes/login/test.ts @@ -2,11 +2,10 @@ import { expect } from '@playwright/test'; import { unauthedTest } from '../../custom-context'; import * as config from '../../../src/lib/assets/configuration.json' assert { type: 'json' }; import type { Branding } from '$lib/configuration'; -const PROVIDER_PREFIX = 'VITE_AUTH_PROVIDER_MODULE_'; - //TypeScript is confused by the JSON import so I am fxing it here /* eslint-disable @typescript-eslint/no-explicit-any */ const branding: Branding = JSON.parse(JSON.stringify((config as any).default)); +const PROVIDER_PREFIX = 'VITE_AUTH_PROVIDER_MODULE_'; //TODO: Tests for login dropdown