From 12f72f6693943266f13ad5d540814f3c935931c4 Mon Sep 17 00:00:00 2001 From: Laton Vermette <1619661+latonv@users.noreply.github.com> Date: Thu, 25 Jan 2024 14:42:28 -0800 Subject: [PATCH] Small fix for web archive results beyond first page + tests --- ...ollection-browser-data-source-interface.ts | 5 ++ .../collection-browser-data-source.ts | 50 +++++++++++++++++-- src/data-source/models.ts | 9 ++++ test/collection-browser.test.ts | 31 ++++++++++++ .../collection-browser-data-source.test.ts | 23 +++++++-- test/mocks/mock-search-responses.ts | 44 ++++++++++++++++ test/mocks/mock-search-service.ts | 2 + 7 files changed, 157 insertions(+), 7 deletions(-) diff --git a/src/data-source/collection-browser-data-source-interface.ts b/src/data-source/collection-browser-data-source-interface.ts index 616d8a470..783fa0dd9 100644 --- a/src/data-source/collection-browser-data-source-interface.ts +++ b/src/data-source/collection-browser-data-source-interface.ts @@ -219,6 +219,11 @@ export interface CollectionBrowserDataSourceInterface */ refreshLetterCounts(): void; + /** + * Returns the current page size of the data source. + */ + getPageSize(): number; + /** * Changes the page size used by the data source, discarding any previously-fetched pages. * diff --git a/src/data-source/collection-browser-data-source.ts b/src/data-source/collection-browser-data-source.ts index 689c6006f..aadcdd606 100644 --- a/src/data-source/collection-browser-data-source.ts +++ b/src/data-source/collection-browser-data-source.ts @@ -22,7 +22,7 @@ import { SortField, SORT_OPTIONS, } from '../models'; -import type { PageSpecifierParams } from './models'; +import { FACETLESS_PAGE_ELEMENTS, type PageSpecifierParams } from './models'; import type { CollectionBrowserDataSourceInterface } from './collection-browser-data-source-interface'; import type { CollectionBrowserSearchInterface } from './collection-browser-query-state'; import { sha1 } from '../utils/sha1'; @@ -31,10 +31,21 @@ import { log } from '../utils/log'; export class CollectionBrowserDataSource implements CollectionBrowserDataSourceInterface { + /** + * All pages of tile models that have been fetched so far, indexed by their page + * number (with the first being page 1). + */ private pages: Record = {}; + /** + * Tile offset to apply when looking up tiles in the pages, in order to maintain + * page alignment after tiles are removed. + */ private offset = 0; + /** + * Total number of tile models stored in this data source's pages + */ private numTileModels = 0; /** @@ -48,10 +59,21 @@ export class CollectionBrowserDataSource */ private previousQueryKey: string = ''; + /** + * Whether the initial page of search results for the current query state + * is presently being fetched. + */ private searchResultsLoading = false; + /** + * Whether the facets (aggregations) for the current query state are + * presently being fetched. + */ private facetsLoading = false; + /** + * Whether further query changes should be ignored and not trigger fetches + */ private suppressFetches = false; /** @@ -265,6 +287,13 @@ export class CollectionBrowserDataSource return Object.values(this.pages).flat().indexOf(tile); } + /** + * @inheritdoc + */ + getPageSize(): number { + return this.pageSize; + } + /** * @inheritdoc */ @@ -294,11 +323,15 @@ export class CollectionBrowserDataSource this._initialSearchCompleteResolver = res; }); + const shouldFetchFacets = + !this.host.suppressFacets && + !FACETLESS_PAGE_ELEMENTS.includes(this.host.profileElement!); + // Fire the initial page & facet requests this.queryInitialized = true; await Promise.all([ this.doInitialPageFetch(), - this.host.suppressFacets ? null : this.fetchFacets(), + shouldFetchFacets ? this.fetchFacets() : null, ]); // Resolve the `initialSearchComplete` promise for this search @@ -876,7 +909,7 @@ export class CollectionBrowserDataSource // Batch multiple initial page requests together if needed (e.g., can request // pages 1 and 2 together in a single request). - const numPages = pageNumber === 1 ? numInitialPages : 1; + let numPages = pageNumber === 1 ? numInitialPages : 1; const numRows = this.pageSize * numPages; // if a fetch is already in progress for this query and page, don't fetch again @@ -990,7 +1023,16 @@ export class CollectionBrowserDataSource } } - // Update the data source for each returned page + // Update the data source for each returned page. + // For loans and web archives, we must account for receiving more pages than we asked for. + if ( + this.host.profileElement === 'lending' || + this.host.profileElement === 'web_archives' + ) { + numPages = Math.ceil(results.length / this.pageSize); + this.endOfDataReached = true; + if (this.activeOnHost) this.host.setTileCount(this.totalResults); + } for (let i = 0; i < numPages; i += 1) { const pageStartIndex = this.pageSize * i; this.addTilesToDataSource( diff --git a/src/data-source/models.ts b/src/data-source/models.ts index e00a04386..11be6312f 100644 --- a/src/data-source/models.ts +++ b/src/data-source/models.ts @@ -27,3 +27,12 @@ export type PageSpecifierParams = { */ pageElements?: PageElementName[]; }; + +/** + * List of profile page elements that do not currently allow faceting + */ +export const FACETLESS_PAGE_ELEMENTS: PageElementName[] = [ + 'forum_posts', + 'lending', + 'web_archives', +]; diff --git a/test/collection-browser.test.ts b/test/collection-browser.test.ts index 52783d492..000591013 100644 --- a/test/collection-browser.test.ts +++ b/test/collection-browser.test.ts @@ -1562,6 +1562,37 @@ describe('Collection Browser', () => { expect(el.maxSelectedDate).not.to.exist; }); + it('correctly retrieves web archive hits', async () => { + const searchService = new MockSearchService(); + const el = await fixture( + html` + ` + ); + + el.baseQuery = 'web-archive'; + await el.updateComplete; + await el.initialSearchComplete; + await nextTick(); + + console.log( + '\n\n*****\n\n*****\n\n', + el.dataSource.getAllPages(), + '\n\n*****\n\n*****\n\n' + ); + expect(el.dataSource.totalResults, 'total results').to.equal(1); + expect(el.dataSource.getTileModelAt(0)?.title).to.equal( + 'https://example.com' + ); + expect( + el.dataSource.getTileModelAt(0)?.captureDates?.length, + 'capture dates' + ).to.equal(1); + }); + it('shows temporarily unavailable message when facets suppressed', async () => { const searchService = new MockSearchService(); const el = await fixture( diff --git a/test/data-source/collection-browser-data-source.test.ts b/test/data-source/collection-browser-data-source.test.ts index e53d1f231..72f6f0bc0 100644 --- a/test/data-source/collection-browser-data-source.test.ts +++ b/test/data-source/collection-browser-data-source.test.ts @@ -1,10 +1,12 @@ import { expect, fixture } from '@open-wc/testing'; import { html } from 'lit'; +import sinon from 'sinon'; import { ItemHit, SearchType } from '@internetarchive/search-service'; import { CollectionBrowserDataSource } from '../../src/data-source/collection-browser-data-source'; import { TileModel } from '../../src/models'; import type { CollectionBrowser } from '../../src/collection-browser'; import '../../src/collection-browser'; +import { MockSearchService } from '../mocks/mock-search-service'; const dataPage: TileModel[] = [ new TileModel( @@ -32,7 +34,7 @@ describe('Collection Browser Data Source', () => { `); }); - it('can add and retrieve data pages', async () => { + it('can add and retrieve data pages', () => { const dataSource = new CollectionBrowserDataSource(host); dataSource.addPage(1, dataPage); @@ -42,12 +44,13 @@ describe('Collection Browser Data Source', () => { expect(dataSource.getPage(1)[1].identifier).to.equal('bar'); }); - it('resets data when changing page size', async () => { + it('resets data when changing page size', () => { const dataSource = new CollectionBrowserDataSource(host); dataSource.addPage(1, dataPage); dataSource.setPageSize(100); expect(Object.keys(dataSource.getAllPages()).length).to.equal(0); + expect(dataSource.getPageSize()).to.equal(100); }); it('can be installed on the host', async () => { @@ -70,7 +73,21 @@ describe('Collection Browser Data Source', () => { host.removeController(dataSource); }); - it('refreshes prefix filter counts', async () => { + it('can suppress further fetches', async () => { + host.searchService = new MockSearchService(); + + const pageFetchSpy = sinon.spy(); + const dataSource = new CollectionBrowserDataSource(host); + dataSource.fetchPage = pageFetchSpy; + + dataSource.addPage(1, dataPage); + dataSource.setFetchesSuppressed(true); + dataSource.handleQueryChange(); + + expect(pageFetchSpy.callCount).to.equal(0); + }); + + it('refreshes prefix filter counts', () => { const dataSource = new CollectionBrowserDataSource(host); dataSource.addPage(1, dataPage); diff --git a/test/mocks/mock-search-responses.ts b/test/mocks/mock-search-responses.ts index 5bb7f600e..c873d770f 100644 --- a/test/mocks/mock-search-responses.ts +++ b/test/mocks/mock-search-responses.ts @@ -6,6 +6,7 @@ import { SearchServiceError, TextHit, } from '@internetarchive/search-service'; +import { WebArchiveHit } from '@internetarchive/search-service/dist/src/models/hit-types/web-archive-hit'; import { SearchServiceErrorType } from '@internetarchive/search-service/dist/src/search-service-error'; export const getMockSuccessSingleResult: () => Result< @@ -876,6 +877,49 @@ export const getMockSuccessWithParentCollections: () => Result< }, }); +export const getMockSuccessWithWebArchiveHits: () => Result< + SearchResponse, + SearchServiceError +> = () => ({ + success: { + request: { + kind: 'hits', + clientParameters: { + user_query: 'web-archive', + sort: [], + }, + backendRequests: { + primary: { + kind: 'hits', + finalized_parameters: { + user_query: 'web-archive', + sort: [], + }, + }, + }, + }, + rawResponse: {}, + response: { + totalResults: 1, + returnedCount: 1, + results: [ + new WebArchiveHit({ + fields: { + url: 'https://example.com', + capture_dates: ['2010-01-02T03:04:05Z'], + __href__: + 'https://web.archive.org/web/20100102030405/https%3A%2F%2Fexample.com', + }, + }), + ], + }, + responseHeader: { + succeeded: true, + query_time: 0, + }, + }, +}); + export const getMockErrorResult: () => Result< SearchResponse, SearchServiceError diff --git a/test/mocks/mock-search-service.ts b/test/mocks/mock-search-service.ts index bbe615936..def1f2760 100644 --- a/test/mocks/mock-search-service.ts +++ b/test/mocks/mock-search-service.ts @@ -28,6 +28,7 @@ import { getMockSuccessWithParentCollections, getMockSuccessManyFields, getMockSuccessNoResults, + getMockSuccessWithWebArchiveHits, } from './mock-search-responses'; const responses: Record< @@ -49,6 +50,7 @@ const responses: Record< 'default-sort-concise': getMockSuccessWithConciseDefaultSort, 'fav-sort': getMockSuccessWithDefaultFavSort, 'parent-collections': getMockSuccessWithParentCollections, + 'web-archive': getMockSuccessWithWebArchiveHits, 'many-fields': getMockSuccessManyFields, 'no-results': getMockSuccessNoResults, error: getMockErrorResult,