From 00d7a41f3a590ec790d14228810e67435728841b Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 18 Jul 2023 14:36:43 -0400 Subject: [PATCH 01/12] WIP: Handle first-party URLs --- .../document/sidebar/related-resources.hbs | 1 + .../document/sidebar/related-resources.ts | 39 ++++- .../sidebar/related-resources/add.hbs | 11 +- .../document/sidebar/related-resources/add.ts | 139 ++++++++++++++++-- web/app/components/x/dropdown-list/_shared.ts | 6 +- web/app/components/x/dropdown-list/index.ts | 6 +- web/app/services/algolia.ts | 10 ++ 7 files changed, 191 insertions(+), 21 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.hbs b/web/app/components/document/sidebar/related-resources.hbs index 3a9f2dbd9..40294deda 100644 --- a/web/app/components/document/sidebar/related-resources.hbs +++ b/web/app/components/document/sidebar/related-resources.hbs @@ -52,6 +52,7 @@ @relatedLinks={{this.relatedLinks}} @allowAddingExternalLinks={{@allowAddingExternalLinks}} @search={{perform this.search}} + @findObject={{perform this.findObject}} @searchErrorIsShown={{this.searchErrorIsShown}} @searchIsRunning={{this.search.isRunning}} /> diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index edb2097fa..3fb4c7d76 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -12,7 +12,7 @@ import htmlElement from "hermes/utils/html-element"; import Ember from "ember"; import FlashMessageService from "ember-cli-flash/services/flash-messages"; import maybeScrollIntoView from "hermes/utils/maybe-scroll-into-view"; -import { assert } from "@ember/debug"; +import { XDropdownListAnchorAPI } from "hermes/components/x/dropdown-list"; export type RelatedResource = RelatedExternalLink | RelatedHermesDocument; @@ -174,6 +174,34 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< }); } + protected findObject = restartableTask( + async (dd: XDropdownListAnchorAPI | null, objectID: string) => { + let index = this.configSvc.config.algolia_docs_index_name; + + console.log("yuh"); + console.log(objectID); + + try { + let algoliaResponse = await this.algolia.findObject.perform( + index, + objectID + ); + if (algoliaResponse) { + console.log(algoliaResponse); + + this._algoliaResults = [ + algoliaResponse, + ] as unknown as HermesDocument[]; + if (dd) { + dd.resetFocusedItemIndex(); + } + } + } catch (e: unknown) { + console.error(e); + } + } + ); + /** * The search task passed to the "Add..." modal. * Returns Algolia document matches for a query and updates @@ -181,9 +209,12 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< * Runs whenever the input value changes. */ protected search = restartableTask( - async (dd: any, query: string, shouldIgnoreDelay?: boolean) => { - let index = - this.configSvc.config.algolia_docs_index_name + "_createdTime_desc"; + async ( + dd: XDropdownListAnchorAPI | null, + query: string, + shouldIgnoreDelay?: boolean + ) => { + let index = this.configSvc.config.algolia_docs_index_name; // Make sure the current document is omitted from the results let filterString = `(NOT objectID:"${this.args.objectID}")`; diff --git a/web/app/components/document/sidebar/related-resources/add.hbs b/web/app/components/document/sidebar/related-resources/add.hbs index e707a05f6..0173be814 100644 --- a/web/app/components/document/sidebar/related-resources/add.hbs +++ b/web/app/components/document/sidebar/related-resources/add.hbs @@ -27,10 +27,9 @@ {{/if}} - {{#if (and @allowAddingExternalLinks this.queryIsURL)}} + {{#if (and @allowAddingExternalLinks this.queryIsExternalURL)}} {{/if}} + + {{#if this.queryIsFirstPartyLink}} + its a link all right + {{/if}} <:loading> Promise; + search: ( + dd: XDropdownListAnchorAPI | null, + query: string, + shouldIgnoreDelay?: boolean + ) => Promise; + findObject: ( + dd: XDropdownListAnchorAPI, + id: string + ) => Promise; allowAddingExternalLinks?: boolean; headerTitle: string; inputPlaceholder: string; @@ -38,8 +48,11 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { export default class DocumentSidebarRelatedResourcesAddComponent extends Component { @service("config") declare configSvc: ConfigService; + @service("fetch") declare fetchSvc: FetchService; @service declare flashMessages: FlashMessageService; + @tracked private _dd: XDropdownListAnchorAPI | null = null; + /** * The value of the search input. Used to query Algolia for documents, * or to set the URL of an external resource. @@ -52,6 +65,16 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone */ @tracked protected queryIsURL = false; + /** + * TODO + */ + @tracked protected queryIsExternalURL = false; + + /** + * TODO + */ + @tracked protected queryIsFirstPartyLink = false; + /** * The DOM element of the search input. Receives focus when inserted. */ @@ -96,13 +119,18 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + private get dd(): XDropdownListAnchorAPI { + assert("dd expected", this._dd); + return this._dd; + } + /** * Whether the list element is displayed. * True unless the query is a URL and adding external links is allowed. */ protected get listIsShown(): boolean { if (this.args.allowAddingExternalLinks) { - return !this.queryIsURL; + return !this.queryIsExternalURL; } else { return true; } @@ -141,7 +169,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return false; } if (this.args.allowAddingExternalLinks) { - return this.queryIsURL || this.queryIsEmpty; + return this.queryIsExternalURL || this.queryIsEmpty; } else { return false; } @@ -245,7 +273,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * Prevents the default ArrowUp/ArrowDown actions * so they can be handled by the XDropdownList component. */ - @action protected onInputKeydown(dd: any, e: KeyboardEvent) { + @action protected onInputKeydown(e: KeyboardEvent) { if (e.key === "Enter") { if (this.queryIsURL) { this.onExternalLinkSubmit(e); @@ -260,7 +288,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } - dd.onTriggerKeydown(dd.contentIsShown, dd.showContent, e); + this.dd.onTriggerKeydown(this.dd.contentIsShown, this.dd.showContent, e); } /** @@ -268,9 +296,14 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * Saves the input locally, loads initial data, then * focuses the search input. */ - @action protected didInsertInput(dd: any, e: HTMLInputElement) { + @action protected didInsertInput( + dd: XDropdownListAnchorAPI, + e: HTMLInputElement + ) { this.searchInput = e; - void this.loadInitialData.perform(dd); + this._dd = dd; + this.dd.registerAnchor(this.searchInput); + void this.loadInitialData.perform(); next(() => { assert("searchInput expected", this.searchInput); @@ -282,11 +315,58 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * The action that runs when the search-input value changes. * Updates the local query property, checks if it's a URL, and searches Algolia. */ - @action protected onInput(dd: any, e: Event) { + @action protected onInput(e: Event) { const input = e.target as HTMLInputElement; this.query = input.value; + this.queryIsFirstPartyLink = false; + this.checkURL(); - void this.args.search(dd, this.query); + void this.args.search(this.dd, this.query); + } + + @action checkIfFirstPartyLink(url: string) { + // need to check the URL to see if it's a first party link + // if it is, we'll query the database to see if it exists. + // if it does, we'll add it to the related resources + // in the correct format. + + // need to check if it starts with a the config's shortlink... + // if so, need to reverse-engineer its ID + + // need to check if the domain matches the current domain + // and ends with /documents/... + + const isShortLink = url.startsWith( + this.configSvc.config.short_link_base_url + ); + + const hermesDomain = window.location.hostname.split(".").pop(); + + if (hermesDomain) { + const urlIsFromCurrentDomain = url.includes(hermesDomain); + + if (isShortLink) { + // Short links are formatted like [shortLinkBaseURL]/[docType]/[docNumber] + const urlParts = url.split("/"); + const docType = urlParts[urlParts.length - 2]; + const docNumber = urlParts[urlParts.length - 1]; + void this.args.search( + this.dd, + `docType:${docType} AND docNumber:${docNumber}` + ); + // need to set the docType and the docNumber + return; + } + + if (urlIsFromCurrentDomain) { + const docID = url.split("/document/").pop(); + if (docID) { + void this.args.findObject(this.dd, docID); + return; + } + } + } + this.queryIsExternalURL = true; } /** @@ -297,16 +377,53 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone this.queryIsURL = isValidURL(this.query); if (this.queryIsURL) { this.checkForDuplicate(this.query); + this.checkIfFirstPartyLink(this.query); } } + private fetchFirstPartyDocument = restartableTask( + async (idOrAttributes: string | Record) => { + this.queryIsExternalURL = false; + + if (typeof idOrAttributes === "string") { + // fetch the document by id + try { + let document = await this.args.findObject(this.dd, idOrAttributes); + if (document) { + this.queryIsFirstPartyLink = true; + console.log('uh'); + const relatedHermesDocument = { + googleFileID: document.objectID, + title: document.title, + type: document.docType, + documentNumber: document.docNumber, + sortOrder: 1, + } as RelatedHermesDocument; + + this.dd.hideContent(); + return; + } + } catch { + console.log("aughts"); + this.queryIsFirstPartyLink = false; + this.queryIsExternalURL = true; + return; + } + } else { + // search for the document by attributes (docType, docNumber) + } + + this.queryIsFirstPartyLink = false; + } + ); + /** * The task that loads the initial `algoliaResults`. * Sends an empty-string query to Algolia, effectively populating its * "suggestions." Called when the search input is inserted. */ - protected loadInitialData = restartableTask(async (dd: any) => { - await this.args.search(dd, "", true); + protected loadInitialData = restartableTask(async () => { + await this.args.search(this.dd, "", true); }); } diff --git a/web/app/components/x/dropdown-list/_shared.ts b/web/app/components/x/dropdown-list/_shared.ts index 743157c09..012c4fdbf 100644 --- a/web/app/components/x/dropdown-list/_shared.ts +++ b/web/app/components/x/dropdown-list/_shared.ts @@ -25,7 +25,11 @@ export interface XDropdownListSharedArgs { */ export interface XDropdownListToggleComponentArgs { registerAnchor: (e: HTMLElement) => void; - onTriggerKeydown: (e: KeyboardEvent) => void; + onTriggerKeydown: ( + contentIsShown: boolean, + showContent: () => void, + e: KeyboardEvent + ) => void; toggleContent: () => void; contentIsShown: boolean; ariaControls: string; diff --git a/web/app/components/x/dropdown-list/index.ts b/web/app/components/x/dropdown-list/index.ts index e6799456e..880df74db 100644 --- a/web/app/components/x/dropdown-list/index.ts +++ b/web/app/components/x/dropdown-list/index.ts @@ -36,7 +36,11 @@ export interface XDropdownListAnchorAPI { contentIsShown: boolean; focusedItemIndex: number; registerAnchor: (element: HTMLElement) => void; - onTriggerKeydown: (event: KeyboardEvent) => void; + onTriggerKeydown: ( + contentIsShown: boolean, + hideContent: () => void, + event: KeyboardEvent + ) => void; resetFocusedItemIndex: () => void; scheduleAssignMenuItemIDs: () => void; toggleContent: () => void; diff --git a/web/app/services/algolia.ts b/web/app/services/algolia.ts index 7e691d86d..fa007325f 100644 --- a/web/app/services/algolia.ts +++ b/web/app/services/algolia.ts @@ -187,6 +187,16 @@ export default class AlgoliaService extends Service { } }; + findObject = task( + async ( + indexName: string, + objectID: string + ): Promise> => { + let index: SearchIndex = this.client.initIndex(indexName); + return await index.getObject(objectID); + } + ); + /** * Searches an index by query and search params. * Returns an Algolia SearchResponse. From f69241c88930e43802b97048977d50952a9b741a Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 18 Jul 2023 16:32:58 -0400 Subject: [PATCH 02/12] Add shortLinkSearch --- .../document/sidebar/related-resources.ts | 10 ++++-- .../document/sidebar/related-resources/add.ts | 33 +++++++++++-------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 3fb4c7d76..22db8543c 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -13,6 +13,7 @@ import Ember from "ember"; import FlashMessageService from "ember-cli-flash/services/flash-messages"; import maybeScrollIntoView from "hermes/utils/maybe-scroll-into-view"; import { XDropdownListAnchorAPI } from "hermes/components/x/dropdown-list"; +import { SearchOptions } from "instantsearch.js"; export type RelatedResource = RelatedExternalLink | RelatedHermesDocument; @@ -212,7 +213,8 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< async ( dd: XDropdownListAnchorAPI | null, query: string, - shouldIgnoreDelay?: boolean + shouldIgnoreDelay?: boolean, + options?: SearchOptions ) => { let index = this.configSvc.config.algolia_docs_index_name; @@ -237,10 +239,14 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< filterString += ` AND (${this.args.searchFilters})`; } + if (options?.filters) { + filterString += ` AND (${options.filters})`; + } + try { let algoliaResponse = await this.algolia.searchIndex .perform(index, query, { - hitsPerPage: 4, + hitsPerPage: options?.hitsPerPage || 4, filters: filterString, attributesToRetrieve: [ "title", diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index 68d2b99e0..b8d03ff2f 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -16,6 +16,7 @@ import { import isValidURL from "hermes/utils/is-valid-u-r-l"; import FetchService from "hermes/services/fetch"; import { XDropdownListAnchorAPI } from "hermes/components/x/dropdown-list"; +import { SearchOptions } from "instantsearch.js"; interface DocumentSidebarRelatedResourcesAddComponentSignature { Element: null; @@ -29,7 +30,8 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { search: ( dd: XDropdownListAnchorAPI | null, query: string, - shouldIgnoreDelay?: boolean + shouldIgnoreDelay?: boolean, + options?: SearchOptions ) => Promise; findObject: ( dd: XDropdownListAnchorAPI | null, @@ -340,24 +342,27 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone this.configSvc.config.short_link_base_url ); + if (isShortLink) { + // Short links are formatted like [shortLinkBaseURL]/[docType]/[docNumber] + const urlParts = url.split("/"); + const docType = urlParts[urlParts.length - 2]; + const docNumber = urlParts[urlParts.length - 1]; + const filterString = `docType:${docType} AND docNumber:${docNumber}`; + + // TODO: Confirm that this returns accurate results. + void this.args.search(this.dd, "", true, { + hitsPerPage: 1, + filters: filterString, + }); + // need to set the docType and the docNumber + return; + } + const hermesDomain = window.location.hostname.split(".").pop(); if (hermesDomain) { const urlIsFromCurrentDomain = url.includes(hermesDomain); - if (isShortLink) { - // Short links are formatted like [shortLinkBaseURL]/[docType]/[docNumber] - const urlParts = url.split("/"); - const docType = urlParts[urlParts.length - 2]; - const docNumber = urlParts[urlParts.length - 1]; - void this.args.search( - this.dd, - `docType:${docType} AND docNumber:${docNumber}` - ); - // need to set the docType and the docNumber - return; - } - if (urlIsFromCurrentDomain) { const docID = url.split("/document/").pop(); if (docID) { From cc20d25d27ec0088a3f6fb67ca887f410672388f Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 18 Jul 2023 16:34:53 -0400 Subject: [PATCH 03/12] Rename `findObject` --- .../components/document/sidebar/related-resources.hbs | 2 +- .../components/document/sidebar/related-resources.ts | 4 ++-- .../document/sidebar/related-resources/add.ts | 6 +++--- web/app/services/algolia.ts | 10 ---------- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.hbs b/web/app/components/document/sidebar/related-resources.hbs index 40294deda..6ae6bcb81 100644 --- a/web/app/components/document/sidebar/related-resources.hbs +++ b/web/app/components/document/sidebar/related-resources.hbs @@ -52,7 +52,7 @@ @relatedLinks={{this.relatedLinks}} @allowAddingExternalLinks={{@allowAddingExternalLinks}} @search={{perform this.search}} - @findObject={{perform this.findObject}} + @getObject={{perform this.getObject}} @searchErrorIsShown={{this.searchErrorIsShown}} @searchIsRunning={{this.search.isRunning}} /> diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 22db8543c..ee8a620d0 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -175,7 +175,7 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< }); } - protected findObject = restartableTask( + protected getObject = restartableTask( async (dd: XDropdownListAnchorAPI | null, objectID: string) => { let index = this.configSvc.config.algolia_docs_index_name; @@ -183,7 +183,7 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< console.log(objectID); try { - let algoliaResponse = await this.algolia.findObject.perform( + let algoliaResponse = await this.algolia.getObject.perform( index, objectID ); diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index b8d03ff2f..7fb146199 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -33,7 +33,7 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { shouldIgnoreDelay?: boolean, options?: SearchOptions ) => Promise; - findObject: ( + getObject: ( dd: XDropdownListAnchorAPI | null, id: string ) => Promise; @@ -366,7 +366,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone if (urlIsFromCurrentDomain) { const docID = url.split("/document/").pop(); if (docID) { - void this.args.findObject(this.dd, docID); + void this.args.getObject(this.dd, docID); return; } } @@ -393,7 +393,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone if (typeof idOrAttributes === "string") { // fetch the document by id try { - let document = await this.args.findObject(this.dd, idOrAttributes); + let document = await this.args.getObject(this.dd, idOrAttributes); if (document) { this.queryIsFirstPartyLink = true; console.log("uh"); diff --git a/web/app/services/algolia.ts b/web/app/services/algolia.ts index 4de61931e..8dbe700a2 100644 --- a/web/app/services/algolia.ts +++ b/web/app/services/algolia.ts @@ -187,16 +187,6 @@ export default class AlgoliaService extends Service { } }; - findObject = task( - async ( - indexName: string, - objectID: string - ): Promise> => { - let index: SearchIndex = this.client.initIndex(indexName); - return await index.getObject(objectID); - } - ); - /** * Searches an index by query and search params. * Returns an Algolia SearchResponse. From 2072b27590bcd8d4b43172485ffbc912bcc9e836 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Thu, 20 Jul 2023 16:18:08 -0400 Subject: [PATCH 04/12] WIP first party pre-merge commit --- .../document/sidebar/related-resources.ts | 29 +++++-------------- .../document/sidebar/related-resources/add.ts | 21 +++++++++----- web/config/environment.js | 2 ++ 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index ee8a620d0..73b13196c 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -175,30 +175,17 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< }); } + /** + * Note: Errors handled in the child component. + */ protected getObject = restartableTask( async (dd: XDropdownListAnchorAPI | null, objectID: string) => { - let index = this.configSvc.config.algolia_docs_index_name; - - console.log("yuh"); - console.log(objectID); - - try { - let algoliaResponse = await this.algolia.getObject.perform( - index, - objectID - ); - if (algoliaResponse) { - console.log(algoliaResponse); - - this._algoliaResults = [ - algoliaResponse, - ] as unknown as HermesDocument[]; - if (dd) { - dd.resetFocusedItemIndex(); - } + let algoliaResponse = await this.algolia.getObject.perform(objectID); + if (algoliaResponse) { + this._algoliaResults = [algoliaResponse] as unknown as HermesDocument[]; + if (dd) { + dd.resetFocusedItemIndex(); } - } catch (e: unknown) { - console.error(e); } } ); diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index 7fb146199..7bf540aad 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -148,7 +148,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } if (this.args.allowAddingExternalLinks) { - return !this.queryIsURL; + return !this.queryIsExternalURL; } return true; @@ -326,7 +326,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone void this.args.search(this.dd, this.query); } - @action checkIfFirstPartyLink(url: string) { + private checkIfFirstPartyLink = restartableTask(async (url: string) => { // need to check the URL to see if it's a first party link // if it is, we'll query the database to see if it exists. // if it does, we'll add it to the related resources @@ -354,7 +354,6 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone hitsPerPage: 1, filters: filterString, }); - // need to set the docType and the docNumber return; } @@ -366,13 +365,21 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone if (urlIsFromCurrentDomain) { const docID = url.split("/document/").pop(); if (docID) { - void this.args.getObject(this.dd, docID); - return; + try { + await this.args.getObject(this.dd, docID); + console.log("uh"); + // this.dd.resetFocusedItemIndex(); + return; + } catch { + // TODO: maybe show that we recognize the URL but it's not a valid document + this.queryIsExternalURL = true; + } } } } + this.queryIsExternalURL = true; - } + }); /** * The action to check if a URL is valid, and, if so, @@ -382,7 +389,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone this.queryIsURL = isValidURL(this.query); if (this.queryIsURL) { this.checkForDuplicate(this.query); - this.checkIfFirstPartyLink(this.query); + void this.checkIfFirstPartyLink.perform(this.query); } } diff --git a/web/config/environment.js b/web/config/environment.js index c343563d5..71c07b8c7 100644 --- a/web/config/environment.js +++ b/web/config/environment.js @@ -55,6 +55,8 @@ module.exports = function (environment) { skipGoogleAuth: getEnv("SKIP_GOOGLE_AUTH"), + shortLinkBaseURL: getEnv("SHORT_LINK_BASE_URL"), + torii: { sessionServiceName: "session", providers: { From 11ae2e0865de6afc57f59ab1ed56c393eac2b19b Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Fri, 21 Jul 2023 12:35:41 -0400 Subject: [PATCH 05/12] FullURL detection --- .../document/sidebar/related-resources.ts | 5 + .../sidebar/related-resources/add.hbs | 12 +- .../document/sidebar/related-resources/add.ts | 309 +++++++++++------- web/app/components/x/dropdown-list/_shared.ts | 6 +- 4 files changed, 194 insertions(+), 138 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index b0f099519..4f8b873bc 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -192,6 +192,11 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< if (dd) { dd.resetFocusedItemIndex(); } + if (dd) { + next(() => { + dd.scheduleAssignMenuItemIDs(); + }); + } } } ); diff --git a/web/app/components/document/sidebar/related-resources/add.hbs b/web/app/components/document/sidebar/related-resources/add.hbs index 0173be814..829a6283a 100644 --- a/web/app/components/document/sidebar/related-resources/add.hbs +++ b/web/app/components/document/sidebar/related-resources/add.hbs @@ -11,7 +11,7 @@ {{/if}} - - {{#if this.queryIsFirstPartyLink}} - its a link all right - {{/if}} <:loading>

- {{if - @searchErrorIsShown - "Search error. Type to retry." - "No results found" - }} + {{this.noMatchesMessage}}

{{/unless}} diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index 7bf540aad..fa27b5ede 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -48,11 +48,26 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { }; } +enum RelatedResourceQueryType { + AlgoliaSearch = "algoliaSearch", + AlgoliaSearchWithFilters = "algoliaSearchWithFilters", + AlgoliaGetObject = "algoliaGetObject", + ExternalLink = "externalLink", +} + +enum FirstPartyURLFormat { + ShortLink = "shortLink", + FullURL = "fullURL", +} + export default class DocumentSidebarRelatedResourcesAddComponent extends Component { @service("config") declare configSvc: ConfigService; @service("fetch") declare fetchSvc: FetchService; @service declare flashMessages: FlashMessageService; + @tracked queryType = RelatedResourceQueryType.AlgoliaSearch; + @tracked firstPartyURLFormat: FirstPartyURLFormat | null = null; + @tracked private _dd: XDropdownListAnchorAPI | null = null; /** @@ -67,16 +82,6 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone */ @tracked protected queryIsURL = false; - /** - * TODO - */ - @tracked protected queryIsExternalURL = false; - - /** - * TODO - */ - @tracked protected queryIsFirstPartyLink = false; - /** * The DOM element of the search input. Receives focus when inserted. */ @@ -106,6 +111,17 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone */ @tracked externalLinkTitleErrorIsShown = false; + protected get shownDocuments() { + if (this.linkIsDuplicate) { + return {}; + } + return this.args.shownDocuments; + } + + protected get queryIsExternalURL() { + return this.queryType === RelatedResourceQueryType.ExternalLink; + } + /** * Whether a query has no results. * May determine whether the list header (e.g., "suggestions," "results") is shown. @@ -121,6 +137,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + private get shortLinkBaseURL() { + return this.configSvc.config.short_link_base_url; + } + private get dd(): XDropdownListAnchorAPI { assert("dd expected", this._dd); return this._dd; @@ -131,6 +151,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * True unless the query is a URL and adding external links is allowed. */ protected get listIsShown(): boolean { + // we don't want to necessarily gate this behind `allowAddingExternalLinks` + // since we now have the concept of first-party URLs that we can search for. + // TODO: Handle this logic. + if (this.args.allowAddingExternalLinks) { return !this.queryIsExternalURL; } else { @@ -138,6 +162,15 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + protected get noMatchesMessage() { + if (this.args.searchErrorIsShown) { + return "Search error. Type to retry."; + } + if (this.linkIsDuplicate) { + return "This doc has already been added."; + } + return "No results found"; + } /** * Whether to show a header above the search results (e.g., "suggestions", "results") * True when there's results to show. @@ -147,6 +180,13 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return false; } + if (this.queryIsFirstPartyURL(this.query)) { + if (this.queryType === RelatedResourceQueryType.ExternalLink) { + return false; + } + return !this.linkIsDuplicate; + } + if (this.args.allowAddingExternalLinks) { return !this.queryIsExternalURL; } @@ -237,10 +277,20 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * The action to check for duplicate ExternalResources. * Used to dictate whether a warning message is displayed. */ - @action private checkForDuplicate(url: string) { - const isDuplicate = this.args.relatedLinks.find((link) => { - return link.url === url; - }); + @action private checkForDuplicate( + urlOrID: string, + resourceIsHermesDocument = false + ) { + let isDuplicate = false; + if (resourceIsHermesDocument) { + isDuplicate = !!this.args.relatedDocuments.find((document) => { + return document.googleFileID === urlOrID; + }); + } else { + isDuplicate = !!this.args.relatedLinks.find((link) => { + return link.url === urlOrID; + }); + } if (isDuplicate) { this.linkIsDuplicate = true; } else { @@ -252,13 +302,14 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * The action to add an external link to a document. * Correctly formats the link data and saves it, unless it already exists. */ - @action addRelatedExternalLink() { + @action private addRelatedExternalLink() { let externalLink = { url: this.query, name: this.externalLinkTitle || this.query, sortOrder: 1, }; + // see if this is already covered this.checkForDuplicate(externalLink.url); if (!this.linkIsDuplicate) { @@ -269,6 +320,26 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + /** + * The action run when the search input is inserted. + * Saves the input locally, loads initial data, then + * focuses the search input. + */ + @action protected didInsertInput( + dd: XDropdownListAnchorAPI, + e: HTMLInputElement + ) { + this.searchInput = e; + this._dd = dd; + this.dd.registerAnchor(this.searchInput); + void this.loadInitialData.perform(); + + next(() => { + assert("searchInput expected", this.searchInput); + this.searchInput.focus(); + }); + } + /** * Keyboard listener for the search input. * Allows "enter" to add external links. @@ -289,28 +360,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return; } } - - this.dd.onTriggerKeydown(this.dd.contentIsShown, this.dd.showContent, e); - } - - /** - * The action run when the search input is inserted. - * Saves the input locally, loads initial data, then - * focuses the search input. - */ - @action protected didInsertInput( - dd: XDropdownListAnchorAPI, - e: HTMLInputElement - ) { - this.searchInput = e; - this._dd = dd; - this.dd.registerAnchor(this.searchInput); - void this.loadInitialData.perform(); - - next(() => { - assert("searchInput expected", this.searchInput); - this.searchInput.focus(); - }); + this.dd.onTriggerKeydown(e); } /** @@ -320,114 +370,127 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone @action protected onInput(e: Event) { const input = e.target as HTMLInputElement; this.query = input.value; - this.queryIsFirstPartyLink = false; - this.checkURL(); - void this.args.search(this.dd, this.query); + this.processQueryType(); + this.handleQuery(); } - private checkIfFirstPartyLink = restartableTask(async (url: string) => { - // need to check the URL to see if it's a first party link - // if it is, we'll query the database to see if it exists. - // if it does, we'll add it to the related resources - // in the correct format. - - // need to check if it starts with a the config's shortlink... - // if so, need to reverse-engineer its ID - - // need to check if the domain matches the current domain - // and ends with /documents/... - - const isShortLink = url.startsWith( - this.configSvc.config.short_link_base_url - ); + /** + * Processes the query to determine if it's a document search or a URL. + * If it's a URL, checks if it's a first- or third-party link. + */ + @action private processQueryType() { + this.queryIsURL = isValidURL(this.query); - if (isShortLink) { - // Short links are formatted like [shortLinkBaseURL]/[docType]/[docNumber] - const urlParts = url.split("/"); - const docType = urlParts[urlParts.length - 2]; - const docNumber = urlParts[urlParts.length - 1]; - const filterString = `docType:${docType} AND docNumber:${docNumber}`; - - // TODO: Confirm that this returns accurate results. - void this.args.search(this.dd, "", true, { - hitsPerPage: 1, - filters: filterString, - }); + if (this.queryIsURL) { + if (this.queryIsFirstPartyURL(this.query)) { + switch (this.firstPartyURLFormat) { + case FirstPartyURLFormat.ShortLink: + this.queryType = RelatedResourceQueryType.AlgoliaSearchWithFilters; + return; + case FirstPartyURLFormat.FullURL: + this.queryType = RelatedResourceQueryType.AlgoliaGetObject; + return; + } + } + this.queryType = RelatedResourceQueryType.ExternalLink; return; } + this.queryType = RelatedResourceQueryType.AlgoliaSearch; + } - const hermesDomain = window.location.hostname.split(".").pop(); - - if (hermesDomain) { - const urlIsFromCurrentDomain = url.includes(hermesDomain); + @action private handleQuery() { + switch (this.queryType) { + case RelatedResourceQueryType.AlgoliaSearch: + void this.args.search(this.dd, this.query); + break; + case RelatedResourceQueryType.AlgoliaGetObject: + let docID = this.query.split("/document/").pop(); + if (docID === this.query) { + // URL splitting didn't work. Treat the query as an external link. + this.queryType = RelatedResourceQueryType.ExternalLink; + this.handleQuery(); + break; + } - if (urlIsFromCurrentDomain) { - const docID = url.split("/document/").pop(); if (docID) { - try { - await this.args.getObject(this.dd, docID); - console.log("uh"); - // this.dd.resetFocusedItemIndex(); - return; - } catch { - // TODO: maybe show that we recognize the URL but it's not a valid document - this.queryIsExternalURL = true; + if (docID.includes("?draft=false")) { + docID = docID.replace("?draft=false", ""); } + void this.getAlgoliaObject.perform(docID); + break; + } else { + this.queryType = RelatedResourceQueryType.ExternalLink; + this.handleQuery(); } + + case RelatedResourceQueryType.AlgoliaSearchWithFilters: + const urlParts = this.query.split("/"); + const docType = urlParts[urlParts.length - 2]; + const docNumber = urlParts[urlParts.length - 1]; + const filterString = `docType:${docType} AND docNumber:${docNumber}`; + // TODO: Confirm that this returns accurate results. + void this.args.search(this.dd, "", true, { + hitsPerPage: 1, + filters: filterString, + }); + break; + case RelatedResourceQueryType.ExternalLink: + this.checkForDuplicate(this.query); + + break; + } + } + + @action private queryIsFirstPartyURL(url: string) { + if (this.shortLinkBaseURL) { + if (url.startsWith(this.shortLinkBaseURL)) { + this.firstPartyURLFormat = FirstPartyURLFormat.ShortLink; + return true; } } - this.queryIsExternalURL = true; - }); + const currentDomain = window.location.hostname.split(".").pop(); - /** - * The action to check if a URL is valid, and, if so, - * whether it's a duplicate. - */ - @action private checkURL() { - this.queryIsURL = isValidURL(this.query); - if (this.queryIsURL) { - this.checkForDuplicate(this.query); - void this.checkIfFirstPartyLink.perform(this.query); + if (currentDomain) { + if (url.includes(currentDomain)) { + this.firstPartyURLFormat = FirstPartyURLFormat.FullURL; + return true; + } } + + this.firstPartyURLFormat = null; + return false; } - private fetchFirstPartyDocument = restartableTask( - async (idOrAttributes: string | Record) => { - this.queryIsExternalURL = false; - - if (typeof idOrAttributes === "string") { - // fetch the document by id - try { - let document = await this.args.getObject(this.dd, idOrAttributes); - if (document) { - this.queryIsFirstPartyLink = true; - console.log("uh"); - const relatedHermesDocument = { - googleFileID: document.objectID, - title: document.title, - type: document.docType, - documentNumber: document.docNumber, - sortOrder: 1, - } as RelatedHermesDocument; - - this.dd.hideContent(); - return; - } - } catch { - console.log("aughts"); - this.queryIsFirstPartyLink = false; - this.queryIsExternalURL = true; + private getAlgoliaObject = restartableTask(async (id: string) => { + assert( + "full url format expected", + this.firstPartyURLFormat === FirstPartyURLFormat.FullURL + ); + + this.checkForDuplicate(id, true); + + if (this.linkIsDuplicate) { + return; + } + + try { + await this.args.getObject(this.dd, id); + } catch (e: unknown) { + const typedError = e as { status?: number }; + + if (typedError.status) { + if (typedError.status === 404) { + this.queryType = RelatedResourceQueryType.ExternalLink; + this.handleQuery(); return; } - } else { - // search for the document by attributes (docType, docNumber) } - - this.queryIsFirstPartyLink = false; + // TODO: confirm that this triggers a `@searchErrorIsShown` update + throw e; } - ); + }); /** * The task that loads the initial `algoliaResults`. diff --git a/web/app/components/x/dropdown-list/_shared.ts b/web/app/components/x/dropdown-list/_shared.ts index 012c4fdbf..743157c09 100644 --- a/web/app/components/x/dropdown-list/_shared.ts +++ b/web/app/components/x/dropdown-list/_shared.ts @@ -25,11 +25,7 @@ export interface XDropdownListSharedArgs { */ export interface XDropdownListToggleComponentArgs { registerAnchor: (e: HTMLElement) => void; - onTriggerKeydown: ( - contentIsShown: boolean, - showContent: () => void, - e: KeyboardEvent - ) => void; + onTriggerKeydown: (e: KeyboardEvent) => void; toggleContent: () => void; contentIsShown: boolean; ariaControls: string; From b5e7fe1500d91739cdd932e90feda4b2df483bdc Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Fri, 21 Jul 2023 14:04:32 -0400 Subject: [PATCH 06/12] ShortURL handling --- .../document/sidebar/related-resources.ts | 7 +-- .../document/sidebar/related-resources/add.ts | 54 +++++++++++++++---- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 4f8b873bc..21cf947d2 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -237,10 +237,6 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< filterString += ` AND (${this.args.searchFilters})`; } - if (options?.filters) { - filterString += ` AND (${options.filters})`; - } - try { let algoliaResponse = await this.algolia.searchIndex .perform(index, query, { @@ -258,7 +254,8 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< // https://www.algolia.com/doc/guides/managing-results/rules/merchandising-and-promoting/in-depth/optional-filters/ // Include any optional search filters, e.g., "product:Terraform" // to give a higher ranking to results that match the filter. - optionalFilters: this.args.optionalSearchFilters, + optionalFilters: + options?.optionalFilters || this.args.optionalSearchFilters, }) .then((response) => response); if (algoliaResponse) { diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index fa27b5ede..8728a718c 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -131,7 +131,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone Object.entries(this.args.shownDocuments).length === 0; if (this.args.allowAddingExternalLinks) { - return objectEntriesLengthIsZero && !this.queryIsURL; + return objectEntriesLengthIsZero && this.queryIsFirstPartyURL(this.query); } else { return objectEntriesLengthIsZero; } @@ -425,15 +425,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } case RelatedResourceQueryType.AlgoliaSearchWithFilters: - const urlParts = this.query.split("/"); - const docType = urlParts[urlParts.length - 2]; - const docNumber = urlParts[urlParts.length - 1]; - const filterString = `docType:${docType} AND docNumber:${docNumber}`; - // TODO: Confirm that this returns accurate results. - void this.args.search(this.dd, "", true, { - hitsPerPage: 1, - filters: filterString, - }); + void this.searchWithFilters.perform(); break; case RelatedResourceQueryType.ExternalLink: this.checkForDuplicate(this.query); @@ -463,6 +455,48 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return false; } + private searchWithFilters = restartableTask(async () => { + const handleAsExternalLink = () => { + this.queryType = RelatedResourceQueryType.ExternalLink; + this.handleQuery(); + }; + + // Short links are formatted like [shortLinkBaseURL]/[docType]/[docNumber] + + const urlParts = this.query.split("/"); + const docType = urlParts[urlParts.length - 2]; + const docNumber = urlParts[urlParts.length - 1]; + + if (!docType || !docNumber) { + handleAsExternalLink(); + return; + } + + const filterString = docNumber; + // TODO: Confirm that this returns accurate results. + try { + await this.args.search(this.dd, filterString, true, { + hitsPerPage: 1, + optionalFilters: [`docType:"${docType}" AND docNumber:"${docNumber}"`], + }); + + if (this.noMatchesFound) { + handleAsExternalLink(); + } + } catch (e: unknown) { + const typedError = e as { status?: number }; + + if (typedError.status) { + if (typedError.status === 404) { + handleAsExternalLink(); + return; + } + } + // TODO: confirm that this triggers a `@searchErrorIsShown` update + throw e; + } + }); + private getAlgoliaObject = restartableTask(async (id: string) => { assert( "full url format expected", From d19a6dce14c564cb2883345204a491867b989003 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Fri, 21 Jul 2023 17:11:04 -0400 Subject: [PATCH 07/12] Stub tests; start of ShortLink duplicate detection --- web/app/components/document/sidebar.hbs | 2 +- .../document/sidebar/related-resources.ts | 15 ++++++++++++--- .../document/sidebar/related-resources/add.ts | 2 ++ .../document/sidebar/related-resources-test.ts | 12 ++++++++++++ 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/web/app/components/document/sidebar.hbs b/web/app/components/document/sidebar.hbs index 3d04e2595..d72079be3 100644 --- a/web/app/components/document/sidebar.hbs +++ b/web/app/components/document/sidebar.hbs @@ -341,7 +341,7 @@ @modalHeaderTitle="Add related resource" @modalInputPlaceholder="Search docs or paste a URL..." @scrollContainer={{this.body}} - @optionalSearchFilters={{array (concat "product:" @document.product)}} + @optionalSearchFilters={{concat "product:" @document.product}} /> diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 21cf947d2..4a26476f8 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -44,7 +44,7 @@ export interface DocumentSidebarRelatedResourcesComponentArgs { headerTitle: string; modalHeaderTitle: string; searchFilters?: string; - optionalSearchFilters?: string[]; + optionalSearchFilters?: string; itemLimit?: number; modalInputPlaceholder: string; documentIsDraft?: boolean; @@ -237,6 +237,16 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< filterString += ` AND (${this.args.searchFilters})`; } + let maybeOptionalFilters = ""; + + if (this.args.optionalSearchFilters) { + maybeOptionalFilters = this.args.optionalSearchFilters; + } + + if (options?.optionalFilters) { + maybeOptionalFilters += ` ${options.optionalFilters}`; + } + try { let algoliaResponse = await this.algolia.searchIndex .perform(index, query, { @@ -254,8 +264,7 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< // https://www.algolia.com/doc/guides/managing-results/rules/merchandising-and-promoting/in-depth/optional-filters/ // Include any optional search filters, e.g., "product:Terraform" // to give a higher ranking to results that match the filter. - optionalFilters: - options?.optionalFilters || this.args.optionalSearchFilters, + optionalFilters: maybeOptionalFilters, }) .then((response) => response); if (algoliaResponse) { diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index 8728a718c..27ea46f35 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -472,6 +472,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return; } + // TODO: duplicate resources are being treated like external links + const filterString = docNumber; // TODO: Confirm that this returns accurate results. try { diff --git a/web/tests/integration/components/document/sidebar/related-resources-test.ts b/web/tests/integration/components/document/sidebar/related-resources-test.ts index 97019dacd..a8b0d6670 100644 --- a/web/tests/integration/components/document/sidebar/related-resources-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources-test.ts @@ -720,5 +720,17 @@ module( .dom(EDIT_EXTERNAL_RESOURCE_ERROR_SELECTOR) .hasText("A title is required."); }); + + test("first class links are recognized (full URL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + + test("first-class links are recognized (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + + test("a failed algoliaObject lookup is handled", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + + test("a failed searchWithFilters call is handled", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + + test("a duplicate first-class link is handled (full URL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + + test("a duplicate first-class link is handled (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); } ); From 2f6a82310d7da10142e22248e6998764338ff8a4 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 25 Jul 2023 11:35:04 -0400 Subject: [PATCH 08/12] Tweak fallback handling; Write tests --- .../document/sidebar/related-resources.hbs | 3 +- .../document/sidebar/related-resources.ts | 54 ++- .../document/sidebar/related-resources/add.ts | 85 +++-- web/config/environment.js | 4 + web/mirage/algolia/hosts.ts | 22 ++ web/mirage/config.ts | 203 +++++++---- .../sidebar/related-resources-test.ts | 345 +++++++++++++++--- .../sidebar/related-resources/add-test.ts | 36 +- .../components/header/search-test.ts | 4 +- 9 files changed, 566 insertions(+), 190 deletions(-) create mode 100644 web/mirage/algolia/hosts.ts diff --git a/web/app/components/document/sidebar/related-resources.hbs b/web/app/components/document/sidebar/related-resources.hbs index 2e0fc2a9c..149b85dbb 100644 --- a/web/app/components/document/sidebar/related-resources.hbs +++ b/web/app/components/document/sidebar/related-resources.hbs @@ -47,13 +47,14 @@ @inputPlaceholder={{@modalInputPlaceholder}} @onClose={{this.hideAddResourceModal}} @addResource={{this.addResource}} - @shownDocuments={{this.algoliaResults}} + @algoliaResults={{this.algoliaResults}} @objectID={{@objectID}} @relatedDocuments={{this.relatedDocuments}} @relatedLinks={{this.relatedLinks}} @allowAddingExternalLinks={{@allowAddingExternalLinks}} @search={{perform this.search}} @getObject={{perform this.getObject}} + @resetAlgoliaResults={{this.resetAlgoliaResults}} @searchErrorIsShown={{this.searchErrorIsShown}} @searchIsRunning={{this.search.isRunning}} /> diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 4a26476f8..0163082fd 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -186,16 +186,29 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< */ protected getObject = restartableTask( async (dd: XDropdownListAnchorAPI | null, objectID: string) => { - let algoliaResponse = await this.algolia.getObject.perform(objectID); - if (algoliaResponse) { - this._algoliaResults = [algoliaResponse] as unknown as HermesDocument[]; - if (dd) { - dd.resetFocusedItemIndex(); + try { + let algoliaResponse = await this.algolia.getObject.perform(objectID); + if (algoliaResponse) { + this._algoliaResults = [ + algoliaResponse, + ] as unknown as HermesDocument[]; + if (dd) { + dd.resetFocusedItemIndex(); + } + if (dd) { + next(() => { + dd.scheduleAssignMenuItemIDs(); + }); + } } - if (dd) { - next(() => { - dd.scheduleAssignMenuItemIDs(); - }); + } catch (e: unknown) { + const typedError = e as { status?: number }; + if (typedError.status === 404) { + // This means the document wasn't found. + // Throw the error and let the child component handle it. + throw e; + } else { + this.handleSearchError(e); } } } @@ -287,15 +300,19 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< await timeout(Ember.testing ? 0 : 200); } } catch (e: unknown) { - // This will trigger the "no matches" block, - // which is where we're displaying the error. - this._algoliaResults = null; - this.searchErrorIsShown = true; - console.error(e); + this.handleSearchError(e); } } ); + @action private handleSearchError(e: unknown) { + // This will trigger the "no matches" block, + // which is where we're displaying the error. + this.resetAlgoliaResults(); + this.searchErrorIsShown = true; + console.error("Algolia search failed", e); + } + /** * The action run when the "add resource" plus button is clicked. * Shows the modal. @@ -365,6 +382,15 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< this.hideAddResourceModal(); } + /** + * The action to set the locally tracked Algolia results to null. + * Used in template computations when a search fails, or when a link is + * recognized as an external resource by a child component. + */ + @action protected resetAlgoliaResults() { + this._algoliaResults = null; + } + /** * The task called to remove a resource from a document. * Triggered via the overflow menu or the "Edit resource" modal. diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index 27ea46f35..84f84cb2c 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -23,7 +23,7 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { Args: { onClose: () => void; addResource: (resource: RelatedResource) => void; - shownDocuments: Record; + algoliaResults: Record; objectID?: string; relatedDocuments: RelatedHermesDocument[]; relatedLinks: RelatedExternalLink[]; @@ -33,15 +33,13 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { shouldIgnoreDelay?: boolean, options?: SearchOptions ) => Promise; - getObject: ( - dd: XDropdownListAnchorAPI | null, - id: string - ) => Promise; + getObject: (dd: XDropdownListAnchorAPI | null, id: string) => Promise; allowAddingExternalLinks?: boolean; headerTitle: string; inputPlaceholder: string; searchErrorIsShown?: boolean; searchIsRunning?: boolean; + resetAlgoliaResults: () => void; }; Blocks: { default: []; @@ -115,7 +113,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone if (this.linkIsDuplicate) { return {}; } - return this.args.shownDocuments; + return this.args.algoliaResults; } protected get queryIsExternalURL() { @@ -127,14 +125,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * May determine whether the list header (e.g., "suggestions," "results") is shown. */ private get noMatchesFound(): boolean { - const objectEntriesLengthIsZero = - Object.entries(this.args.shownDocuments).length === 0; - - if (this.args.allowAddingExternalLinks) { - return objectEntriesLengthIsZero && this.queryIsFirstPartyURL(this.query); - } else { - return objectEntriesLengthIsZero; - } + return Object.entries(this.args.algoliaResults).length === 0; } private get shortLinkBaseURL() { @@ -393,9 +384,13 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return; } } - this.queryType = RelatedResourceQueryType.ExternalLink; - return; + + if (this.args.allowAddingExternalLinks) { + this.queryType = RelatedResourceQueryType.ExternalLink; + return; + } } + this.queryType = RelatedResourceQueryType.AlgoliaSearch; } @@ -428,8 +423,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone void this.searchWithFilters.perform(); break; case RelatedResourceQueryType.ExternalLink: + this.args.resetAlgoliaResults(); this.checkForDuplicate(this.query); - break; } } @@ -467,34 +462,46 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone const docType = urlParts[urlParts.length - 2]; const docNumber = urlParts[urlParts.length - 1]; - if (!docType || !docNumber) { - handleAsExternalLink(); - return; + const hasTypeAndNumber = docType && docNumber; + + if (this.args.allowAddingExternalLinks) { + if (!hasTypeAndNumber) { + handleAsExternalLink(); + return; + } } - // TODO: duplicate resources are being treated like external links + const filterString = docNumber || this.query; + const optionalFilters = docNumber + ? [`docType:"${docType}" AND docNumber:"${docNumber}"`] + : []; - const filterString = docNumber; - // TODO: Confirm that this returns accurate results. try { await this.args.search(this.dd, filterString, true, { hitsPerPage: 1, - optionalFilters: [`docType:"${docType}" AND docNumber:"${docNumber}"`], + optionalFilters, }); - if (this.noMatchesFound) { - handleAsExternalLink(); - } - } catch (e: unknown) { - const typedError = e as { status?: number }; + // This will update the `shownDocuments` object + // need to check the value of the first key + const firstResult = Object.values( + this.shownDocuments + )[0] as HermesDocument; - if (typedError.status) { - if (typedError.status === 404) { + if (this.noMatchesFound) { + if (this.args.allowAddingExternalLinks) { handleAsExternalLink(); return; } } - // TODO: confirm that this triggers a `@searchErrorIsShown` update + + this.checkForDuplicate(firstResult?.objectID, true); + + if (this.linkIsDuplicate) { + return; + } + } catch (e: unknown) { + // TODO: Confirm what happens in this case throw e; } }); @@ -514,17 +521,9 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone try { await this.args.getObject(this.dd, id); } catch (e: unknown) { - const typedError = e as { status?: number }; - - if (typedError.status) { - if (typedError.status === 404) { - this.queryType = RelatedResourceQueryType.ExternalLink; - this.handleQuery(); - return; - } - } - // TODO: confirm that this triggers a `@searchErrorIsShown` update - throw e; + this.queryType = RelatedResourceQueryType.ExternalLink; + this.handleQuery(); + return; } }); diff --git a/web/config/environment.js b/web/config/environment.js index 71c07b8c7..8020fe543 100644 --- a/web/config/environment.js +++ b/web/config/environment.js @@ -74,6 +74,8 @@ module.exports = function (environment) { }; if (environment === "development") { + ENV.shortLinkBaseURL = "https://go.hashi.co"; + // ENV.APP.LOG_RESOLVER = true; // ENV.APP.LOG_ACTIVE_GENERATION = true; // ENV.APP.LOG_TRANSITIONS = true; @@ -82,6 +84,8 @@ module.exports = function (environment) { } if (environment === "test") { + ENV.shortLinkBaseURL = "https://go.hashi.co"; + // Testem prefers this... ENV.locationType = "none"; diff --git a/web/mirage/algolia/hosts.ts b/web/mirage/algolia/hosts.ts new file mode 100644 index 000000000..01b4ac089 --- /dev/null +++ b/web/mirage/algolia/hosts.ts @@ -0,0 +1,22 @@ +/** + * Algolia has one static endpoint and several wildcard hosts, e.g., + * - appID-1.algolianet.com + * - appID-2.algolianet.com + * + * Mirage lacks wildcard support, so we create a route for each. + * Used in tests to mock Algolia endpoints. + */ + +import config from "hermes/config/environment"; + +// Start with the static route. +let algoliaHosts = [ + `https://${config.algolia.appID}-dsn.algolia.net/1/indexes/**`, +]; + +// Add wildcard routes. +for (let i = 1; i <= 9; i++) { + algoliaHosts.push(`https://${config.algolia.appID}-${i}.algolianet.com/1/indexes/**`); +} + +export default algoliaHosts; diff --git a/web/mirage/config.ts b/web/mirage/config.ts index 83be8f997..6e7a958c1 100644 --- a/web/mirage/config.ts +++ b/web/mirage/config.ts @@ -1,10 +1,8 @@ // https://www.ember-cli-mirage.com/docs/advanced/server-configuration import { Collection, Response, createServer } from "miragejs"; -import config from "../config/environment"; -import { SearchResponse } from "@algolia/client-search"; import { getTestDocNumber } from "./factories/document"; -import { SearchForFacetValuesResponse } from "@algolia/client-search"; +import algoliaHosts from "./algolia/hosts"; export default function (mirageConfig) { let finalConfig = { @@ -13,6 +11,142 @@ export default function (mirageConfig) { routes() { this.namespace = "api/v1"; + /************************************************************************* + * + * Algolia requests + * + *************************************************************************/ + + /** + * A triage function for all Algolia requests. + * Reviews the request and determines how to respond. + */ + const handleAlgoliaRequest = (schema, request) => { + const requestBody = JSON.parse(request.requestBody); + + if (requestBody) { + const { facetQuery, query } = requestBody; + if (facetQuery) { + let facetMatch = schema.document.all().models.filter((doc) => { + return doc.attrs.product + .toLowerCase() + .includes(facetQuery.toLowerCase()); + })[0]; + if (facetMatch) { + return new Response( + 200, + {}, + { facetHits: [{ value: facetMatch.attrs.product }] } + ); + } else { + return new Response(200, {}, { facetHits: [] }); + } + } else if (query !== undefined) { + /** + * A query exists, but may be empty. + * Typically, this is a query for a document title or product, + * but sometimes it's a query by some optionalFilters. + */ + + let docMatches = []; + let idsToExclude: string[] = []; + + const filters = requestBody.filters; + + if (filters?.includes("NOT objectID")) { + // there can be a number of objectIDs in the format of + // NOT objectID:"1234" AND NOT objectID:"5678" + // we need to parse these and use them to filter the results below. + filters.split("NOT objectID:").forEach((filter: string) => { + if (filter) { + const id = filter.split('"')[1]; + if (id) { + idsToExclude.push(id); + } + } + }); + } + + const optionalFilters = requestBody.optionalFilters; + + if (optionalFilters?.includes("docNumber")) { + const docNumber = optionalFilters + .split('docNumber:"')[1] + .split('"')[0]; + + docMatches = schema.document.all().models.filter((doc) => { + return doc.attrs.docNumber === docNumber; + }); + + // Duplicates are detected in the front end + return new Response(200, {}, { hits: docMatches }); + } else { + docMatches = schema.document.all().models.filter((doc) => { + return ( + doc.attrs.title.toLowerCase().includes(query.toLowerCase()) || + doc.attrs.product.toLowerCase().includes(query.toLowerCase()) + ); + }); + } + + if (idsToExclude) { + docMatches = docMatches.filter((doc) => { + return !idsToExclude.includes(doc.attrs.objectID); + }); + } + + return new Response(200, {}, { hits: docMatches }); + } else { + /** + * A request we're not currently handling with any specificity. + * Returns the entire document index. + */ + return new Response( + 200, + {}, + { hits: schema.document.all().models } + ); + } + } else { + /** + * This is a `getObject` request, (a search by a :document_id), + * which arrives like this: { *: "index-name/id" }. + * We use the ID to search Mirage. + * If an object is found, we return it. + * If not, we return a 404. + */ + const docID = request.params["*"].split("/")[1]; + const doc = schema.document.findBy({ id: docID }); + + if (doc) { + return new Response(200, {}, doc.attrs); + } else { + // Mimic Algolia's response when its getObject method fails. + return new Response(404, {}, {}); + } + } + }; + + /** + * Algolia has several search hosts, e.g., + * - appID-1.algolianet.com + * - appID-2.algolianet.com + * + * And Mirage lacks wildcards, so we create a route for each. + * + * Additionally, we support the remaining Algolia routes. + */ + + algoliaHosts.forEach((host) => { + this.post(host, (schema, request) => { + return handleAlgoliaRequest(schema, request); + }); + + this.get(host, (schema, request) => { + return handleAlgoliaRequest(schema, request); + }); + }); + /************************************************************************* * * HEAD requests @@ -66,69 +200,6 @@ export default function (mirageConfig) { return new Response(200, {}); }); - const getAlgoliaSearchResults = (schema, request) => { - const requestBody = JSON.parse(request.requestBody); - const { facetQuery, query } = requestBody; - - if (facetQuery) { - let facetMatch = schema.document.all().models.filter((doc) => { - return doc.attrs.product - .toLowerCase() - .includes(facetQuery.toLowerCase()); - })[0]; - - if (!facetMatch) { - return new Response(200, {}, { facetHits: [] }); - } else { - return new Response( - 200, - {}, - { facetHits: [{ value: facetMatch.attrs.product }] } - ); - } - } else { - let docMatches = schema.document.all().models.filter((doc) => { - return ( - doc.attrs.title.toLowerCase().includes(query.toLowerCase()) || - doc.attrs.product.toLowerCase().includes(query.toLowerCase()) - ); - }); - return new Response(200, {}, { hits: docMatches }); - } - }; - - /** - * Used by the AlgoliaSearchService to query Algolia. - */ - this.post( - `https://${config.algolia.appID}-dsn.algolia.net/1/indexes/**`, - (schema, request) => { - return getAlgoliaSearchResults(schema, request); - } - ); - - /** - * Algolia has several search hosts, e.g., appID-1.algolianet.com, - * and Mirage doesn't support wildcards in routes. - * So, we create a route for each host. - * - * TODO: Export this into a function that can be used in tests also. - */ - - let algoliaSearchHosts = []; - - for (let i = 1; i <= 9; i++) { - algoliaSearchHosts.push( - `https://${config.algolia.appID}-${i}.algolianet.com/1/indexes/**` - ); - } - - algoliaSearchHosts.forEach((host) => { - this.post(host, (schema, request) => { - return getAlgoliaSearchResults(schema, request); - }); - }); - /** * Called by the Document route to log a document view. */ diff --git a/web/tests/integration/components/document/sidebar/related-resources-test.ts b/web/tests/integration/components/document/sidebar/related-resources-test.ts index a8b0d6670..ecf2b367e 100644 --- a/web/tests/integration/components/document/sidebar/related-resources-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources-test.ts @@ -14,6 +14,7 @@ import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; import { HermesDocument } from "hermes/types/document"; import { Response } from "miragejs"; import config from "hermes/config/environment"; +import algoliaHosts from "hermes/mirage/algolia/hosts"; const LOADING_ICON_SELECTOR = "[data-test-related-resources-list-loading-icon]"; const LIST_SELECTOR = "[data-test-related-resources-list]"; @@ -57,6 +58,14 @@ const EDIT_EXTERNAL_RESOURCE_ERROR_SELECTOR = const TOOLTIP_TRIGGER_SELECTOR = "[data-test-tooltip-icon-trigger]"; const TOOLTIP_SELECTOR = ".hermes-tooltip"; +const CURRENT_DOMAIN_PROTOCOL = window.location.protocol + "//"; +const CURRENT_DOMAIN = window.location.hostname; +const CURRENT_PORT = window.location.port; + +const SHORT_LINK_BASE_URL = config.shortLinkBaseURL; + +const SEARCH_ERROR_MESSAGE = "Search error. Type to retry."; + interface DocumentSidebarRelatedResourcesTestContext extends MirageTestContext { document: HermesDocument; body: HTMLElement; @@ -76,6 +85,13 @@ module( objectID: "1234", }); + // Populate the database with at least one more doc. + this.server.create("document", { + title: "Foobar", + product: "Labs", + objectID: "4321", + }); + this.set("document", this.server.schema.document.first().attrs); const bodyDiv = document.createElement("div"); this.set("body", bodyDiv); @@ -514,7 +530,7 @@ module( await fillIn( ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, - "https://example.com" + "http://test.com" ); assert @@ -526,54 +542,6 @@ module( .exists("the fallback message is shown"); }); - test("it shows an error when searching fails", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { - this.server.createList("document", 3); - - this.server.post( - `https://${config.algolia.appID}-dsn.algolia.net/1/indexes/**`, - () => { - return new Response(500, {}, {}); - } - ); - - let algoliaSearchHosts = []; - - for (let i = 1; i <= 9; i++) { - algoliaSearchHosts.push( - `https://${config.algolia.appID}-${i}.algolianet.com/1/indexes/**` - ); - } - - algoliaSearchHosts.forEach((host) => { - this.server.post(host, () => { - return new Response(500, {}, {}); - }); - }); - - await render(hbs` - - `); - - await click(ADD_RESOURCE_BUTTON_SELECTOR); - - await waitFor(NO_RESOURCES_FOUND_SELECTOR); - - assert - .dom(NO_RESOURCES_FOUND_SELECTOR) - .containsText( - "Search error. Type to retry.", - "the error message is shown in the modal" - ); - }); - test("it calls the correct endpoint when editing a draft", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { this.server.create("relatedExternalLink", { name: "Example", @@ -721,16 +689,283 @@ module( .hasText("A title is required."); }); - test("first class links are recognized (full URL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + test("first class links are recognized (full URL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + const docID = "777"; + const docTitle = "Jackpot!"; + + this.server.create("document", { + id: docID, + title: docTitle, + }); + + await render(hbs` + + `); + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + + // Construct a "valid" first-class URL + const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/${docID}`; + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); + + await waitFor(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); + + assert + .dom(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR) + .containsText(docTitle, "the document URL is correctly parsed"); + }); + + test("first-class links are recognized (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + const docID = "777"; + const docTitle = "Jackpot!"; + const docType = "PRD"; + const docNumber = "VLT-777"; + + this.server.create("document", { + id: docID, + objectID: docID, + title: docTitle, + docType, + docNumber, + }); + + await render(hbs` + + `); + + const shortLink = `${SHORT_LINK_BASE_URL}/${docType}/${docNumber}`; + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); + + await waitFor(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); + + assert + .dom(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR) + .containsText(docTitle, "the shortLink is correctly parsed"); + }); + + test("an invalid hermes URL is handled like an external link", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + await render(hbs` + + `); + + // We build a URL with the correct format, but an invalid ID. + + const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/999`; + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); + + assert + .dom(ADD_EXTERNAL_RESOURCE_FORM_SELECTOR) + .exists('the "add resource" form is shown'); + }); + + test("an invalid shortLink URL is handled like an external link", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + await render(hbs` + + `); + + const shortLink = `${SHORT_LINK_BASE_URL}/RFC/VLT-999`; + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); + + assert + .dom(ADD_EXTERNAL_RESOURCE_FORM_SELECTOR) + .exists('the "add resource" form is shown'); + }); + + test("a duplicate first-class link is handled (full URL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + const docID = "777"; + const docTitle = "Foo"; - test("first-class links are recognized (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + this.server.create("document", { + id: docID, + title: docTitle, + objectID: docID, + }); - test("a failed algoliaObject lookup is handled", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + await render(hbs` + + `); + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + + const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/${docID}`; - test("a failed searchWithFilters call is handled", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); + await click(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); - test("a duplicate first-class link is handled (full URL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + assert.dom(LIST_ITEM_SELECTOR).exists({ count: 1 }); + + // Enter the same URL + await click(ADD_RESOURCE_BUTTON_SELECTOR); + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); + + assert + .dom(NO_RESOURCES_FOUND_SELECTOR) + .hasText("This doc has already been added."); + }); + + test("a duplicate first-class link is handled (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + const docID = "777"; + const docTitle = "Foo"; + const docNumber = "VLT-777"; + + this.server.create("document", { + id: docID, + title: docTitle, + objectID: docID, + docNumber, + }); + + await render(hbs` + + `); + + const shortLink = `${SHORT_LINK_BASE_URL}/RFC/${docNumber}`; + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); + await click(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); + + assert.dom(LIST_ITEM_SELECTOR).exists({ count: 1 }); + + await click(ADD_RESOURCE_BUTTON_SELECTOR); - test("a duplicate first-class link is handled (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) {}); + // Enter the same URL + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); + + assert + .dom(NO_RESOURCES_FOUND_SELECTOR) + .hasText("This doc has already been added."); + }); + + test("a non-404 getAlgoliaObject call is handled", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + algoliaHosts.forEach((host) => { + this.server.get(host, () => { + return new Response(500, {}, {}); + }); + }); + + await render(hbs` + + `); + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + + // Enter what looks like a valid URL to trigger an object lookup + await fillIn( + ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, + `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/xyz` + ); + + await waitFor(NO_RESOURCES_FOUND_SELECTOR); + + assert + .dom(NO_RESOURCES_FOUND_SELECTOR) + .containsText(SEARCH_ERROR_MESSAGE); + }); + + test("it shows an error when searching fails", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + this.server.createList("document", 3); + + algoliaHosts.forEach((host) => { + this.server.post(host, () => { + return new Response(500, {}, {}); + }); + }); + + await render(hbs` + + `); + + await click(ADD_RESOURCE_BUTTON_SELECTOR); + + await waitFor(NO_RESOURCES_FOUND_SELECTOR); + + assert + .dom(NO_RESOURCES_FOUND_SELECTOR) + .containsText( + SEARCH_ERROR_MESSAGE, + "the error message is shown in the modal" + ); + }); } ); diff --git a/web/tests/integration/components/document/sidebar/related-resources/add-test.ts b/web/tests/integration/components/document/sidebar/related-resources/add-test.ts index ca5a4e2e9..e2b98ddb5 100644 --- a/web/tests/integration/components/document/sidebar/related-resources/add-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources/add-test.ts @@ -1,13 +1,10 @@ import { module, test } from "qunit"; import { setupRenderingTest } from "ember-qunit"; import { - click, fillIn, render, triggerEvent, triggerKeyEvent, - waitFor, - waitUntil, } from "@ember/test-helpers"; import { hbs } from "ember-cli-htmlbars"; import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; @@ -25,6 +22,7 @@ interface DocumentSidebarRelatedResourcesAddTestContext extends MirageTestContext { noop: () => void; search: (dd: XDropdownListAnchorAPI | null, query: string) => Promise; + getObject: (dd: XDropdownListAnchorAPI | null, id: string) => Promise; shownDocuments: Record; allowAddingExternalLinks: boolean; searchIsRunning: boolean; @@ -89,6 +87,10 @@ module( return Promise.resolve(); } ); + + this.set("getObject", (dd: XDropdownListAnchorAPI | null, id: string) => { + return Promise.resolve(); + }); }); test("it renders correctly (initial load)", async function (this: DocumentSidebarRelatedResourcesAddTestContext, assert) { @@ -98,11 +100,13 @@ module( @inputPlaceholder="Test placeholder" @addResource={{this.noop}} @onClose={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); @@ -121,12 +125,14 @@ module( @inputPlaceholder="Test placeholder" @onClose={{this.noop}} @addResource={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @allowAddingExternalLinks={{true}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); @@ -163,11 +169,13 @@ module( @inputPlaceholder="Test placeholder" @onClose={{this.noop}} @addResource={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); assert.dom("[data-test-add-related-resource-spinner]").exists(); @@ -182,12 +190,14 @@ module( @inputPlaceholder="Test placeholder" @onClose={{this.noop}} @addResource={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @allowAddingExternalLinks={{this.allowAddingExternalLinks}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); @@ -221,11 +231,13 @@ module( @inputPlaceholder="Test placeholder" @onClose={{this.noop}} @addResource={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); @@ -243,11 +255,13 @@ module( @inputPlaceholder="Test placeholder" @onClose={{this.noop}} @addResource={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); @@ -289,12 +303,14 @@ module( @inputPlaceholder="Test placeholder" @onClose={{this.noop}} @addResource={{this.noop}} - @shownDocuments={{this.shownDocuments}} + @algoliaResults={{this.shownDocuments}} @objectID="test" @relatedDocuments={{array}} @relatedLinks={{array}} @search={{this.search}} @searchIsRunning={{this.searchIsRunning}} + @resetAlgoliaResults={{this.noop}} + @getObject={{this.getObject}} /> `); const iconSelector = "[data-test-related-resources-search-loading-icon]"; diff --git a/web/tests/integration/components/header/search-test.ts b/web/tests/integration/components/header/search-test.ts index d16c41060..b2f74157e 100644 --- a/web/tests/integration/components/header/search-test.ts +++ b/web/tests/integration/components/header/search-test.ts @@ -1,7 +1,7 @@ import { module, test } from "qunit"; import { setupRenderingTest } from "ember-qunit"; import { hbs } from "ember-cli-htmlbars"; -import { click, fillIn, render, triggerKeyEvent } from "@ember/test-helpers"; +import { click, fillIn, render, triggerKeyEvent, waitFor } from "@ember/test-helpers"; import { setupMirage } from "ember-cli-mirage/test-support"; import { MirageTestContext } from "ember-cli-mirage/test-support"; @@ -89,6 +89,8 @@ module("Integration | Component | header/search", function (hooks) { await fillIn(SEARCH_INPUT_SELECTOR, "vault"); + await waitFor(BEST_MATCHES_HEADER_SELECTOR); + assert .dom(BEST_MATCHES_HEADER_SELECTOR) .exists('the "best matches" header is shown when matches are found'); From 77cd330a3d58c62ca6083364723a3f4e38c3cfab Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 25 Jul 2023 12:07:07 -0400 Subject: [PATCH 09/12] Add documentation --- .../document/sidebar/related-resources.ts | 8 +- .../sidebar/related-resources/add.hbs | 2 +- .../document/sidebar/related-resources/add.ts | 128 +++++++++++++----- 3 files changed, 104 insertions(+), 34 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 0163082fd..8f9b2e88a 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -159,6 +159,7 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< } return documents; } + /** * The text passed to the TooltipIcon beside the title. */ @@ -305,8 +306,12 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< } ); + /** + * The action run when a search errors. Resets the Algolia results + * and causes a search error to appear. + */ @action private handleSearchError(e: unknown) { - // This will trigger the "no matches" block, + // This triggers the "no matches" block, // which is where we're displaying the error. this.resetAlgoliaResults(); this.searchErrorIsShown = true; @@ -347,6 +352,7 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< // The getter doesn't update when a new resource is added, so we manually save it. // TODO: Improve this this.relatedLinks = this.relatedLinks; + void this.saveRelatedResources.perform( this.relatedDocuments, cachedLinks, diff --git a/web/app/components/document/sidebar/related-resources/add.hbs b/web/app/components/document/sidebar/related-resources/add.hbs index 829a6283a..b39810b4e 100644 --- a/web/app/components/document/sidebar/related-resources/add.hbs +++ b/web/app/components/document/sidebar/related-resources/add.hbs @@ -11,7 +11,7 @@ { if (this.linkIsDuplicate) { return {}; } return this.args.algoliaResults; } + /** + * Whether the query is an external URL. + * Used as a shorthand check when determining layout and behavior. + */ protected get queryIsExternalURL() { return this.queryType === RelatedResourceQueryType.ExternalLink; } @@ -128,10 +165,17 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return Object.entries(this.args.algoliaResults).length === 0; } - private get shortLinkBaseURL() { + /** + * The app's configured shortLinkBaseURL if it exists. + * Used to determine whether a URL is a first-party shortLink. + */ + private get shortLinkBaseURL(): string | undefined { return this.configSvc.config.short_link_base_url; } + /** + * An asserted-true reference to the XDropdownListAnchorAPI. + */ private get dd(): XDropdownListAnchorAPI { assert("dd expected", this._dd); return this._dd; @@ -142,10 +186,6 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * True unless the query is a URL and adding external links is allowed. */ protected get listIsShown(): boolean { - // we don't want to necessarily gate this behind `allowAddingExternalLinks` - // since we now have the concept of first-party URLs that we can search for. - // TODO: Handle this logic. - if (this.args.allowAddingExternalLinks) { return !this.queryIsExternalURL; } else { @@ -153,6 +193,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + /** + * The message to show in the "<:no-matches>" block + * when the query errors, detects a duplicate, or returns no results. + */ protected get noMatchesMessage() { if (this.args.searchErrorIsShown) { return "Search error. Type to retry."; @@ -162,6 +206,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } return "No results found"; } + /** * Whether to show a header above the search results (e.g., "suggestions", "results") * True when there's results to show. @@ -184,6 +229,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return true; } + /** * Whether the query is empty. * Helps determine whether the "no results" message. @@ -224,6 +270,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone this.keyboardNavIsEnabled = true; } + /** + * The action to run when the external link form is submitted. + * Validates the title input, then adds the link, if it's not a duplicate. + */ @action onExternalLinkSubmit(e: Event) { // Prevent the form from blindly submitting e.preventDefault(); @@ -394,6 +444,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone this.queryType = RelatedResourceQueryType.AlgoliaSearch; } + /** + * The action run once the query type is determined. + * Calls the appropriate method for the query. + */ @action private handleQuery() { switch (this.queryType) { case RelatedResourceQueryType.AlgoliaSearch: @@ -402,23 +456,24 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone case RelatedResourceQueryType.AlgoliaGetObject: let docID = this.query.split("/document/").pop(); if (docID === this.query) { - // URL splitting didn't work. Treat the query as an external link. + // URL splitting didn't work. + // Re-handle the query as an external link. this.queryType = RelatedResourceQueryType.ExternalLink; this.handleQuery(); break; } - if (docID) { + // Trim any trailing query params if (docID.includes("?draft=false")) { docID = docID.replace("?draft=false", ""); } void this.getAlgoliaObject.perform(docID); break; } else { + // The query looked like a full URL, but this.queryType = RelatedResourceQueryType.ExternalLink; this.handleQuery(); } - case RelatedResourceQueryType.AlgoliaSearchWithFilters: void this.searchWithFilters.perform(); break; @@ -429,6 +484,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + /** + * An action to check if is a query is a first-party URL. + * Sets the `firstPartyURLFormat` property depending on its assessment. + */ @action private queryIsFirstPartyURL(url: string) { if (this.shortLinkBaseURL) { if (url.startsWith(this.shortLinkBaseURL)) { @@ -450,6 +509,11 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return false; } + /** + * An Algolia search for queries identified as first-party shortLinks. + * Checks the query for a docType and docNumber, and if they exist, + * uses them as filters in the Algolia search. + */ private searchWithFilters = restartableTask(async () => { const handleAsExternalLink = () => { this.queryType = RelatedResourceQueryType.ExternalLink; @@ -461,7 +525,6 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone const urlParts = this.query.split("/"); const docType = urlParts[urlParts.length - 2]; const docNumber = urlParts[urlParts.length - 1]; - const hasTypeAndNumber = docType && docNumber; if (this.args.allowAddingExternalLinks) { @@ -476,36 +539,35 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone ? [`docType:"${docType}" AND docNumber:"${docNumber}"`] : []; - try { - await this.args.search(this.dd, filterString, true, { - hitsPerPage: 1, - optionalFilters, - }); + // Errors are handled in the parent method + await this.args.search(this.dd, filterString, true, { + hitsPerPage: 1, + optionalFilters, + }); - // This will update the `shownDocuments` object - // need to check the value of the first key - const firstResult = Object.values( - this.shownDocuments - )[0] as HermesDocument; + // This will update the `shownDocuments` object + // need to check the value of the first key + const firstResult = Object.values(this.algoliaResults)[0] as HermesDocument; - if (this.noMatchesFound) { - if (this.args.allowAddingExternalLinks) { - handleAsExternalLink(); - return; - } + if (this.noMatchesFound) { + if (this.args.allowAddingExternalLinks) { + handleAsExternalLink(); + return; } + } - this.checkForDuplicate(firstResult?.objectID, true); + this.checkForDuplicate(firstResult?.objectID, true); - if (this.linkIsDuplicate) { - return; - } - } catch (e: unknown) { - // TODO: Confirm what happens in this case - throw e; + if (this.linkIsDuplicate) { + return; } }); + /** + * An Algolia getObject request for queries identified as first-party full URLs. + * Fetches non-duplicate docs by ID. If the request fails, the query is handled + * as an external link. + */ private getAlgoliaObject = restartableTask(async (id: string) => { assert( "full url format expected", @@ -521,6 +583,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone try { await this.args.getObject(this.dd, id); } catch (e: unknown) { + // The parent method throws when a 404 is returned. + // We catch it here and reprocess the query as an external link. this.queryType = RelatedResourceQueryType.ExternalLink; this.handleQuery(); return; From 36b83d5883a82ff88477073ae2061646c250e2f2 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 25 Jul 2023 13:44:54 -0400 Subject: [PATCH 10/12] Cleanup --- .../components/document/sidebar/related-resources.ts | 6 ++++-- web/tests/integration/components/header/search-test.ts | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 8f9b2e88a..01bb44e5e 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -183,7 +183,9 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< } /** - * Note: Errors handled in the child component. + * Requests an Algolia document by ID. + * If found, sets the local Algolia results to an array + * with that document. If not, throws a 404 to the child component. */ protected getObject = restartableTask( async (dd: XDropdownListAnchorAPI | null, objectID: string) => { @@ -206,7 +208,7 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< const typedError = e as { status?: number }; if (typedError.status === 404) { // This means the document wasn't found. - // Throw the error and let the child component handle it. + // Let the child component handle the error. throw e; } else { this.handleSearchError(e); diff --git a/web/tests/integration/components/header/search-test.ts b/web/tests/integration/components/header/search-test.ts index b2f74157e..3faace7f4 100644 --- a/web/tests/integration/components/header/search-test.ts +++ b/web/tests/integration/components/header/search-test.ts @@ -1,7 +1,13 @@ import { module, test } from "qunit"; import { setupRenderingTest } from "ember-qunit"; import { hbs } from "ember-cli-htmlbars"; -import { click, fillIn, render, triggerKeyEvent, waitFor } from "@ember/test-helpers"; +import { + click, + fillIn, + render, + triggerKeyEvent, + waitFor, +} from "@ember/test-helpers"; import { setupMirage } from "ember-cli-mirage/test-support"; import { MirageTestContext } from "ember-cli-mirage/test-support"; @@ -89,8 +95,6 @@ module("Integration | Component | header/search", function (hooks) { await fillIn(SEARCH_INPUT_SELECTOR, "vault"); - await waitFor(BEST_MATCHES_HEADER_SELECTOR); - assert .dom(BEST_MATCHES_HEADER_SELECTOR) .exists('the "best matches" header is shown when matches are found'); From 69214d08608326a4df2406c10288c7d15c487491 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Wed, 9 Aug 2023 10:29:21 -0400 Subject: [PATCH 11/12] Add Google URL support --- .../document/sidebar/related-resources/add.ts | 29 +++++++++++++++++-- .../sidebar/related-resources-test.ts | 21 ++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/document/sidebar/related-resources/add.ts index 52ded4d07..57076752a 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/document/sidebar/related-resources/add.ts @@ -454,7 +454,13 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone void this.args.search(this.dd, this.query); break; case RelatedResourceQueryType.AlgoliaGetObject: + /** + * First-class queries are either: + * - Hermes URLs (e.g., /document/:document_id?queryParams) + * - Google Docs URLs (e.g., /document/d/:document_id/viewMode) + */ let docID = this.query.split("/document/").pop(); + if (docID === this.query) { // URL splitting didn't work. // Re-handle the query as an external link. @@ -462,11 +468,23 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone this.handleQuery(); break; } + if (docID) { + // Trim any leading "d/" + if (docID.includes("d/")) { + docID = docID.replace("d/", ""); + } + + // Trim anything after a slash + if (docID.includes("/")) { + docID = docID.split("/")[0] as string; + } + // Trim any trailing query params - if (docID.includes("?draft=false")) { - docID = docID.replace("?draft=false", ""); + if (docID.includes("?")) { + docID = docID.split("?")[0] as string; } + void this.getAlgoliaObject.perform(docID); break; } else { @@ -505,6 +523,13 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } } + const googleDocsURL = "https://docs.google.com/document/d/"; + + if (url.includes(googleDocsURL)) { + this.firstPartyURLFormat = FirstPartyURLFormat.FullURL; + return true; + } + this.firstPartyURLFormat = null; return false; } diff --git a/web/tests/integration/components/document/sidebar/related-resources-test.ts b/web/tests/integration/components/document/sidebar/related-resources-test.ts index ecf2b367e..7444c8dca 100644 --- a/web/tests/integration/components/document/sidebar/related-resources-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources-test.ts @@ -713,16 +713,33 @@ module( await click(ADD_RESOURCE_BUTTON_SELECTOR); - // Construct a "valid" first-class URL + // Construct a "valid" first-class Hermes URL const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/${docID}`; await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); - await waitFor(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); assert .dom(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR) .containsText(docTitle, "the document URL is correctly parsed"); + + // Reset the input + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, ""); + + // Confirm the reset + assert + .dom(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR) + .doesNotContainText(docTitle); + + // Construct a first-class Google URL + const googleURL = `https://docs.google.com/document/d/${docID}/edit`; + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, googleURL); + await waitFor(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); + + assert + .dom(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR) + .containsText(docTitle, "the Google URL is correctly parsed"); }); test("first-class links are recognized (shortURL)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { From 1824535aab41467bb0fdf54fb4ffbb3ed8aee573 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Wed, 9 Aug 2023 13:58:36 -0400 Subject: [PATCH 12/12] Rename shortlink --- web/config/environment.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/config/environment.js b/web/config/environment.js index 8020fe543..343982452 100644 --- a/web/config/environment.js +++ b/web/config/environment.js @@ -74,7 +74,7 @@ module.exports = function (environment) { }; if (environment === "development") { - ENV.shortLinkBaseURL = "https://go.hashi.co"; + ENV.shortLinkBaseURL = "https://fake.short.link"; // ENV.APP.LOG_RESOLVER = true; // ENV.APP.LOG_ACTIVE_GENERATION = true; @@ -84,7 +84,7 @@ module.exports = function (environment) { } if (environment === "test") { - ENV.shortLinkBaseURL = "https://go.hashi.co"; + ENV.shortLinkBaseURL = "https://fake.short.link"; // Testem prefers this... ENV.locationType = "none";