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

OC-974: Enable easier searching of publications on author pages #740

Merged
merged 7 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
5 changes: 4 additions & 1 deletion api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ RUN apk add \
curl \
gnupg \
lsb-release \
wget
wget \
openssl \
openssl-dev \
libc6-compat

# Dockerize is needed to sync containers startup
ENV DOCKERIZE_VERSION v0.6.1
Expand Down
2 changes: 1 addition & 1 deletion api/prisma/schema.prisma
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
generator client {
provider = "prisma-client-js"
previewFeatures = ["fullTextSearch"]
binaryTargets = ["native", "rhel-openssl-3.0.x", "linux-arm64-openssl-3.0.x"]
binaryTargets = ["native", "linux-musl", "rhel-openssl-3.0.x", "linux-arm64-openssl-3.0.x"]
}

datasource db {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('Get many publication versions', () => {
});

expect(getPublications.status).toEqual(200);
const publicationDates = getPublications.body.data.map((version) => version.publishedDate);
const publicationDates: Date[] = getPublications.body.data.map((version) => version.publishedDate as Date);
finlay-jisc marked this conversation as resolved.
Show resolved Hide resolved
// Sort a copy of the dates from the results to confirm order.
const sortedPublicationDates = [...publicationDates].sort(
(a, b) => new Date(b).getTime() - new Date(a).getTime()
Expand All @@ -67,9 +67,9 @@ describe('Get many publication versions', () => {
});

expect(getPublications.status).toEqual(200);
const publicationDates = getPublications.body.data.map((version) => version.publishedDate);
const publicationDates = getPublications.body.data.map((version) => version.publishedDate as Date);
finlay-jisc marked this conversation as resolved.
Show resolved Hide resolved
// Sort a copy of the dates from the results to confirm order.
const sortedPublicationDates = [...publicationDates].sort(
const sortedPublicationDates: Date[] = [...publicationDates].sort(
(a, b) => new Date(a).getTime() - new Date(b).getTime()
);
expect(publicationDates).toEqual(sortedPublicationDates);
Expand Down
29 changes: 22 additions & 7 deletions api/src/components/user/__tests__/getUserPublications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
.query({ apiKey: 123456789, offset: 0, limit: 100 });

expect(publications.status).toEqual(200);
expect(publications.body.results.length).toEqual(25);
expect(publications.body.data.length).toEqual(25);
expect(
publications.body.results.some(
publications.body.data.some(
(publication) => publication.versions.some((version) => version.currentStatus === 'DRAFT') as boolean
)
).toEqual(true);
Expand All @@ -24,9 +24,9 @@
const publications = await testUtils.agent.get('/users/test-user-1/publications');

expect(publications.status).toEqual(200);
expect(publications.body.results.length).toEqual(10);
expect(publications.body.data.length).toEqual(10);
expect(
publications.body.results.some(
publications.body.data.some(
(publication) => publication.versions.some((version) => version.currentStatus === 'DRAFT') as boolean
)
).toEqual(false);
Expand All @@ -36,9 +36,9 @@
const publications = await testUtils.agent.get('/users/test-user-1/publications').query({ apiKey: 987654321 });

expect(publications.status).toEqual(200);
expect(publications.body.results.length).toEqual(10);
expect(publications.body.data.length).toEqual(10);
expect(
publications.body.results.some(
publications.body.data.some(
(publication) => publication.versions.some((version) => version.currentStatus === 'DRAFT') as boolean
)
).toEqual(false);
Expand All @@ -49,8 +49,23 @@
.get('/users/user-does-not-exist/publications')
.query({ apiKey: 987654321 });

expect(publications.body.results).toBe(undefined);
expect(publications.body.data).toBe(undefined);
expect(publications.body.message).toBe('User not found');
expect(publications.status).toEqual(400);
});

test('Results can be filtered by a query term', async () => {
const queryTerm = 'interpretation';
const publications = await testUtils.agent.get('/users/test-user-1/publications').query({ query: queryTerm });

expect(publications.status).toEqual(200);
expect(publications.body.data.length).toEqual(1);
expect(
publications.body.data.every((publication) =>
publication.versions.some(

Check warning on line 65 in api/src/components/user/__tests__/getUserPublications.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unsafe return of an `any` typed value
(version) => version.isLatestLiveVersion && version.title.toLowerCase().includes(queryTerm)

Check warning on line 66 in api/src/components/user/__tests__/getUserPublications.test.ts

View workflow job for this annotation

GitHub Actions / eslint

Unsafe return of an `any` typed value
)
)
).toEqual(true);
});
});
4 changes: 4 additions & 0 deletions api/src/components/user/schema/getPublications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ const getPublicationsSchema: I.JSONSchemaType<I.UserPublicationsFilters> = {
minimum: 1,
default: 10
},
query: {
type: 'string',
nullable: true
},
versionStatus: {
type: 'string',
nullable: true
Expand Down
17 changes: 15 additions & 2 deletions api/src/components/user/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,20 @@ export const getPublications = async (
}
: {}
: // But if the user is not the owner, get only publications that have a published version
{ versions: { some: { isLatestLiveVersion: true } } })
{ versions: { some: { isLatestLiveVersion: true } } }),
// And, if a query is supplied, where the query matches the latest live title.
...(params.query
? {
versions: {
some: {
isLatestLiveVersion: true,
title: {
search: params.query + ':*'
}
}
}
}
: {})
};

const userPublications = await client.prisma.publication.findMany({
Expand Down Expand Up @@ -391,7 +404,7 @@ export const getPublications = async (
}
});

return { offset, limit, total: totalUserPublications, results: sortedPublications };
return { data: sortedPublications, metadata: { offset, limit, total: totalUserPublications } };
};

export const getUserList = async () => {
Expand Down
1 change: 1 addition & 0 deletions api/src/lib/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ export interface UpdateAffiliationsBody {
export interface UserPublicationsFilters {
offset: number;
limit: number;
query?: string;
versionStatus?: string;
}

Expand Down
22 changes: 0 additions & 22 deletions e2e/tests/LoggedIn/livePublication.e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,28 +102,6 @@ test.describe('Live Publication', () => {
await testFlagging(page, 'cl3fz14dr0001es6i5ji51rq4', 'testing the flagging functionality');
});

test('Author profile', async ({ browser }) => {
finlay-jisc marked this conversation as resolved.
Show resolved Hide resolved
const page = await Helpers.users.getPageAsUser(browser);
await page.goto(`/publications/publication-user-6-hypothesis-1-live`, { waitUntil: 'domcontentloaded' });

// Check and click author link
await page.getByRole('link', { name: 'G. Murphy' }).click();
await page.waitForURL(`/authors/test-user-6-grace-murphy`);

// Check name
await expect(page.locator(PageModel.authorInfo.name)).toBeVisible();

// Check ORCID data sections
for await (const orcidDataSection of PageModel.profilePage.orcidDataSections) {
await expect(page.locator(orcidDataSection)).toBeVisible();
}

// Check Author publications section
await page.locator(PageModel.profilePage.showAll).click();
await page.waitForSelector(PageModel.profilePage.result);
await expect(page.locator(PageModel.profilePage.result)).toBeVisible();
});

test('Download pdf/json', async ({ browser, headless }) => {
const page = await Helpers.users.getPageAsUser(browser);
await page.goto(`/publications/cl3fz14dr0001es6i5ji51rq4`, { waitUntil: 'domcontentloaded' });
Expand Down
101 changes: 56 additions & 45 deletions e2e/tests/LoggedOut/profile.e2e.spec.ts
Original file line number Diff line number Diff line change
@@ -1,65 +1,76 @@
import { expect, Page, test } from '@playwright/test';
import { expect, test } from '@playwright/test';
import { PageModel } from '../PageModel';

test.describe('Octopus profile', () => {
let page: Page;
test.describe('User profiles', () => {
test('Visit an author profile', async ({ browser }) => {
const page = await browser.newPage();
await page.goto(`/publications/publication-user-6-hypothesis-1-live`, { waitUntil: 'domcontentloaded' });

test.beforeAll(async ({ browser }) => {
page = await browser.newPage();
// Check and click author link
await page.getByRole('link', { name: 'G. Murphy' }).click();
await page.waitForURL(`/authors/test-user-6-grace-murphy`);

// Check name
await expect(page.locator(PageModel.authorInfo.name)).toBeVisible();

// Check ORCID data sections
for await (const orcidDataSection of PageModel.profilePage.orcidDataSections) {
await expect(page.locator(orcidDataSection)).toBeVisible();
}

// Check publications section
await expect(page.locator(PageModel.organisationalUserInfo.octopusPublications)).toBeVisible();
});

test("Explore a user's publications", async ({ browser }) => {
const page = await browser.newPage();
// navigate to Octopus profile page
await page.goto(`/authors/octopus`, {
waitUntil: 'domcontentloaded'
});
await expect(page).toHaveTitle('Author: Octopus - Octopus | Built for Researchers');
});

test.afterAll(async () => {
await page.close();
});

test('Octopus publications pagination', async () => {
// check user name
// Check user name.
await expect(page.locator('h1')).toHaveText('Octopus');

// check Octopus publications section
const octopusPublicationsHeader = page.locator(PageModel.organisationalUserInfo.octopusPublications);

await expect(octopusPublicationsHeader).toBeVisible();

await page.waitForLoadState('networkidle');

const octopusPublicationsSection = octopusPublicationsHeader.locator('xpath=..');

// initially, only 10 publications should be visible
expect(await octopusPublicationsSection.locator('a').count()).toEqual(10);

// press "Show More" button to see more publications
await expect(page.locator("'Show More'")).toBeVisible();
await Promise.all([
page.waitForResponse(
(response) => response.url().includes('/users/octopus/publications?offset=10&limit=10') && response.ok()
),
page.click("'Show More'")
]);

// wait for publications to be rendered - 50ms per each
await page.waitForTimeout(500);
// Initially, 20 publications should be visible.
await expect(await octopusPublicationsSection.locator('a').count()).toEqual(20);
await expect(page.getByText(/Showing 1 - 20 of \d+/)).toBeVisible();

// the next 10 pubs should be loaded
expect(await octopusPublicationsSection.locator('a').count()).toEqual(20);
// Change page size.
await page.getByLabel('Showing').selectOption('10');
await page.waitForResponse(
(response) =>
response.request().method() === 'GET' &&
response.url().includes('/users/octopus/publications?offset=0&limit=10')
);
await expect(await octopusPublicationsSection.locator('a').count()).toEqual(10);
await expect(page.getByText(/Showing 1 - 10 of \d+/)).toBeVisible();

// press "Show More" button again
await Promise.all([
page.waitForResponse(
(response) => response.url().includes('/users/octopus/publications?offset=20&limit=10') && response.ok()
),
page.click("'Show More'")
]);
// Change page.
await page.getByLabel('Next').click();
await page.waitForResponse(
(response) =>
response.request().method() === 'GET' &&
response.url().includes('/users/octopus/publications?offset=10&limit=10')
);
await expect(page.getByText(/Showing 11 - 20 of \d+/)).toBeVisible();

// wait for publications to be rendered - 50ms per each
await page.waitForTimeout(500);
// Enter a query term and filter results.
await page.getByLabel('Quick search').fill('muco-cutaneous');
await octopusPublicationsSection.getByRole('button', { name: 'Search' }).click();
await page.waitForResponse(
(response) =>
response.request().method() === 'GET' &&
response.url().includes('/users/octopus/publications?offset=0&limit=10&query=muco-cutaneous')
);

// 30 publications should now be visible in the UI
expect(await octopusPublicationsSection.locator('a').count()).toEqual(30);
// Expect 1 result and disabled prev/next buttons.
await expect(await octopusPublicationsSection.locator('a').count()).toEqual(1);
await expect(page.getByLabel('Previous')).toBeDisabled();
await expect(page.getByLabel('Next')).toBeDisabled();
});
});
66 changes: 47 additions & 19 deletions ui/src/__tests__/components/SearchPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import * as TestUtils from '@/testUtils';

import { render, screen } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import { resolve } from 'path';

jest.mock('next/router', () => ({
useRouter: jest.fn()
Expand Down Expand Up @@ -36,26 +35,10 @@ describe('Basic search page', () => {
expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Search results');
});

it('Search type select is present', () => {
expect(screen.getByRole('combobox', { name: 'Searching' })).toBeInTheDocument();
});

it('Search type select has expected options', () => {
const searchTypeSelect = screen.getByRole('combobox', { name: 'Searching' });
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Publications' }));
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Authors' }));
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Topics' }));
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Organisations' }));
expect(searchTypeSelect.children).toHaveLength(4);
});

it('Search type select has value "publications"', () => {
expect(screen.getByRole('combobox', { name: 'Searching' })).toHaveValue('publications');
it('Search type select is not present', () => {
expect(screen.queryByRole('combobox', { name: 'Searching' })).not.toBeInTheDocument();
});

// TODO: test that changing search type select calls useRouter's push with the appropriate path.
// Couldn't get this to work.

it('Page size select is present', () => {
expect(screen.getByRole('combobox', { name: 'Showing' })).toBeInTheDocument();
});
Expand Down Expand Up @@ -110,6 +93,51 @@ describe('Basic search page', () => {
});
});

describe('Search page with search type select', () => {
const handleSearchFormSubmit = jest.fn((e) => {
e.preventDefault();
});
const setLimit = jest.fn();

beforeEach(() => {
render(
<Components.SearchPage
handleSearchFormSubmit={handleSearchFormSubmit}
isValidating={false}
limit={10}
offset={0}
query={null}
results={[]}
searchType="publication-versions"
setLimit={setLimit}
setOffset={jest.fn}
showSearchTypeSwitch={true}
finlay-jisc marked this conversation as resolved.
Show resolved Hide resolved
total={0}
/>
);
});

it('Search type select is present', () => {
expect(screen.getByRole('combobox', { name: 'Searching' })).toBeInTheDocument();
});

it('Search type select has expected options', () => {
const searchTypeSelect = screen.getByRole('combobox', { name: 'Searching' });
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Publications' }));
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Authors' }));
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Topics' }));
expect(searchTypeSelect).toContainElement(screen.getByRole('option', { name: 'Organisations' }));
expect(searchTypeSelect.children).toHaveLength(4);
});

it('Search type select has value "publications"', () => {
expect(screen.getByRole('combobox', { name: 'Searching' })).toHaveValue('publications');
});

// TODO: test that changing search type select calls useRouter's push with the appropriate path.
// Couldn't get this to work.
});

describe('Search page with filters', () => {
const resetFilters = jest.fn();

Expand Down
Loading
Loading