From 922a2d13c458e4528d73ab40b40a927e795f5027 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Wed, 6 Sep 2023 17:50:58 -0400 Subject: [PATCH 01/15] Unscope RelatedResource from Sidebar --- web/app/components/document/sidebar.hbs | 2 +- .../document/sidebar/related-resources.hbs | 121 +-- .../document/sidebar/related-resources.ts | 204 +---- .../sidebar/related-resources/add.hbs | 108 --- .../sidebar/related-resources/add/document.ts | 19 - .../related-resources/list-item/edit.hbs | 51 +- .../related-resources/list-item/edit.ts | 57 +- web/app/components/related-resources.hbs | 32 + web/app/components/related-resources.ts | 279 +++++++ web/app/components/related-resources/add.hbs | 110 +++ .../sidebar => }/related-resources/add.ts | 52 +- .../related-resources/add/document.hbs | 0 .../related-resources/add/document.ts | 19 + .../add/external-resource.hbs | 0 .../add/external-resource.ts | 6 +- .../external-resource-form.hbs | 37 + .../external-resource-form.ts | 33 + .../sidebar/related-resources-test.ts | 470 +---------- .../components/related-resources-test.ts | 735 ++++++++++++++++++ .../related-resources/add-test.ts | 79 +- .../add/external-resource-test.ts | 9 +- 21 files changed, 1453 insertions(+), 970 deletions(-) delete mode 100644 web/app/components/document/sidebar/related-resources/add.hbs delete mode 100644 web/app/components/document/sidebar/related-resources/add/document.ts create mode 100644 web/app/components/related-resources.hbs create mode 100644 web/app/components/related-resources.ts create mode 100644 web/app/components/related-resources/add.hbs rename web/app/components/{document/sidebar => }/related-resources/add.ts (92%) rename web/app/components/{document/sidebar => }/related-resources/add/document.hbs (100%) create mode 100644 web/app/components/related-resources/add/document.ts rename web/app/components/{document/sidebar => }/related-resources/add/external-resource.hbs (100%) rename web/app/components/{document/sidebar => }/related-resources/add/external-resource.ts (61%) create mode 100644 web/app/components/related-resources/external-resource-form.hbs create mode 100644 web/app/components/related-resources/external-resource-form.ts create mode 100644 web/tests/integration/components/related-resources-test.ts rename web/tests/integration/components/{document/sidebar => }/related-resources/add-test.ts (80%) rename web/tests/integration/components/{document/sidebar => }/related-resources/add/external-resource-test.ts (71%) diff --git a/web/app/components/document/sidebar.hbs b/web/app/components/document/sidebar.hbs index 4826d42f4..873e5fbbc 100644 --- a/web/app/components/document/sidebar.hbs +++ b/web/app/components/document/sidebar.hbs @@ -299,7 +299,7 @@ @documentIsDraft={{this.isDraft}} @productArea={{@document.product}} @objectID={{@document.objectID}} - @allowAddingExternalLinks={{true}} + @scope="all" @headerTitle="Related resources" @modalHeaderTitle="Add related resource" @modalInputPlaceholder="Search docs or paste a URL..." diff --git a/web/app/components/document/sidebar/related-resources.hbs b/web/app/components/document/sidebar/related-resources.hbs index 1b6909c76..4e8f55092 100644 --- a/web/app/components/document/sidebar/related-resources.hbs +++ b/web/app/components/document/sidebar/related-resources.hbs @@ -1,61 +1,66 @@ - - -{{#if this.loadRelatedResources.isRunning}} -
- -
-{{else if this.loadingHasFailed}} -
-
- Failed to load -
- + <:header as |rr|> + -
-{{else}} - - <:resource as |r|> - + <:list-loading> +
+ +
+ + <:list-error> +
+
+ Failed to load +
+ - - -{{/if}} +
+ + <:list> + {{! TODO: yield something better than a generic block }} + + <:resource as |r|> + + + + + <:list-empty> +
+ TODO -{{#if this.addResourceModalIsShown}} - -{{/if}} +
+ + diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 01bb44e5e..4ef920588 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -2,7 +2,6 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; import { tracked } from "@glimmer/tracking"; import { inject as service } from "@ember/service"; -import { HermesDocument } from "hermes/types/document"; import FetchService from "hermes/services/fetch"; import ConfigService from "hermes/services/config"; import AlgoliaService from "hermes/services/algolia"; @@ -12,8 +11,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 { XDropdownListAnchorAPI } from "hermes/components/x/dropdown-list"; -import { SearchOptions } from "instantsearch.js"; +import { RelatedResourcesScope } from "hermes/components/related-resources"; export type RelatedResource = RelatedExternalLink | RelatedHermesDocument; @@ -40,7 +38,6 @@ export interface RelatedHermesDocument { export interface DocumentSidebarRelatedResourcesComponentArgs { productArea?: string; objectID?: string; - allowAddingExternalLinks?: boolean; headerTitle: string; modalHeaderTitle: string; searchFilters?: string; @@ -50,6 +47,7 @@ export interface DocumentSidebarRelatedResourcesComponentArgs { documentIsDraft?: boolean; editingIsDisabled?: boolean; scrollContainer: HTMLElement; + scope: `${RelatedResourcesScope}`; } interface DocumentSidebarRelatedResourcesComponentSignature { @@ -64,18 +62,8 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< @tracked relatedLinks: RelatedExternalLink[] = []; @tracked relatedDocuments: RelatedHermesDocument[] = []; - - @tracked _algoliaResults: HermesDocument[] | null = null; - - @tracked addResourceModalIsShown = false; @tracked loadingHasFailed = false; - /** - * Whether to show an error message in the search modal. - * Set true when an Algolia search fails. - */ - @tracked searchErrorIsShown = false; - /** * The related resources object, minimally formatted for a PUT request to the API. */ @@ -119,7 +107,6 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< return resourcesArray; } - /** * Whether the "Add Resource" button should be hidden. * True when editing is explicitly disabled (e.g., when the viewer doesn't have edit @@ -138,28 +125,6 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< } } - /** - * The Algolia results for a query. Updated by the `search` task - * and displayed in the "add resources" modal. - */ - protected get algoliaResults(): { [key: string]: HermesDocument } { - /** - * The array initially looks like this: - * [{title: "foo", objectID: "bar"...}, ...] - * - * We transform it to look like: - * { "bar": {title: "foo", objectID: "bar"...}, ...} - */ - let documents: any = {}; - - if (this._algoliaResults) { - this._algoliaResults.forEach((doc) => { - documents[doc.objectID] = doc; - }); - } - return documents; - } - /** * The text passed to the TooltipIcon beside the title. */ @@ -182,160 +147,6 @@ export default class DocumentSidebarRelatedResourcesComponent extends 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) => { - 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(); - }); - } - } - } catch (e: unknown) { - const typedError = e as { status?: number }; - if (typedError.status === 404) { - // This means the document wasn't found. - // Let the child component handle the error. - throw e; - } else { - this.handleSearchError(e); - } - } - } - ); - - /** - * The search task passed to the "Add..." modal. - * Returns Algolia document matches for a query and updates - * the dropdown with the correct menu item IDs. - * Runs whenever the input value changes. - */ - protected search = restartableTask( - async ( - dd: XDropdownListAnchorAPI | null, - query: string, - shouldIgnoreDelay?: boolean, - options?: SearchOptions - ) => { - 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}")`; - - // And if there are any related documents, omit those too - if (this.relatedDocuments.length) { - let relatedDocIDs = this.relatedDocuments.map( - (doc) => doc.googleFileID - ); - - filterString = filterString.slice(0, -1) + " "; - - filterString += `AND NOT objectID:"${relatedDocIDs.join( - '" AND NOT objectID:"' - )}")`; - } - - // If there are search filters, e.g., "doctype:RFC" add them to the query - if (this.args.searchFilters) { - 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, { - hitsPerPage: options?.hitsPerPage || 4, - filters: filterString, - attributesToRetrieve: [ - "title", - "product", - "docNumber", - "docType", - "status", - "owners", - ], - - // 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: maybeOptionalFilters, - }) - .then((response) => response); - if (algoliaResponse) { - this._algoliaResults = algoliaResponse.hits as HermesDocument[]; - if (dd) { - dd.resetFocusedItemIndex(); - } - } - if (dd) { - next(() => { - dd.scheduleAssignMenuItemIDs(); - }); - } - this.searchErrorIsShown = false; - - if (!shouldIgnoreDelay) { - // This will show the "loading" spinner for some additional time - // unless the task is restarted. This is to prevent the spinner - // from flashing when the user types and results return quickly. - await timeout(Ember.testing ? 0 : 200); - } - } catch (e: unknown) { - this.handleSearchError(e); - } - } - ); - - /** - * The action run when a search errors. Resets the Algolia results - * and causes a search error to appear. - */ - @action private handleSearchError(e: unknown) { - // This triggers 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. - */ - @action protected showAddResourceModal() { - this.addResourceModalIsShown = true; - } - - /** - * The action run to close the "add resources" modal. - * Called on `esc` and by clicking the X button. - */ - @action protected hideAddResourceModal() { - this.addResourceModalIsShown = false; - } - /** * The action run when the user saves changes on a * RelatedExternalLink. Confirms that the resource exists, @@ -386,17 +197,6 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< cachedLinks, resourceSelector ); - - 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; } /** diff --git a/web/app/components/document/sidebar/related-resources/add.hbs b/web/app/components/document/sidebar/related-resources/add.hbs deleted file mode 100644 index b39810b4e..000000000 --- a/web/app/components/document/sidebar/related-resources/add.hbs +++ /dev/null @@ -1,108 +0,0 @@ -{{#in-element (html-element ".ember-application") insertBefore=null}} - - - {{@headerTitle}} - - - - <:anchor as |dd|> -
- - - {{#if @searchIsRunning}} -
- -
- {{/if}} -
- - <:header> - {{#if this.listHeaderIsShown}} -
-

- {{#if this.query.length}} - Results - {{else}} - Suggestions - {{/if}} -

-
- {{/if}} - - {{#if (and @allowAddingExternalLinks this.queryIsExternalURL)}} - - {{/if}} - - <:loading> - - - <:no-matches> - {{#unless this.noResultsMessageIsHidden}} -
-

- {{this.noMatchesMessage}} -

-
- {{/unless}} - - <:item as |dd|> - - - - - - - -{{/in-element}} diff --git a/web/app/components/document/sidebar/related-resources/add/document.ts b/web/app/components/document/sidebar/related-resources/add/document.ts deleted file mode 100644 index f0fca91ca..000000000 --- a/web/app/components/document/sidebar/related-resources/add/document.ts +++ /dev/null @@ -1,19 +0,0 @@ -import Component from "@glimmer/component"; -import { HermesDocument } from "hermes/types/document"; - -interface DocumentSidebarRelatedResourcesAddDocumentComponentSignature { - Args: { - document: HermesDocument; - }; - Blocks: { - default: []; - }; -} - -export default class DocumentSidebarRelatedResourcesAddDocumentComponent extends Component {} - -declare module "@glint/environment-ember-loose/registry" { - export default interface Registry { - "Document::Sidebar::RelatedResources::Add::Document": typeof DocumentSidebarRelatedResourcesAddDocumentComponent; - } -} diff --git a/web/app/components/document/sidebar/related-resources/list-item/edit.hbs b/web/app/components/document/sidebar/related-resources/list-item/edit.hbs index 71b1c8fd4..908285ef8 100644 --- a/web/app/components/document/sidebar/related-resources/list-item/edit.hbs +++ b/web/app/components/document/sidebar/related-resources/list-item/edit.hbs @@ -8,46 +8,16 @@ Edit resource - {{! TODO: Combine with `Add::ExternalResource` }} -
- - Title - {{#if this.titleErrorMessageIsShown}} - - A title is required. - - {{/if}} - - - URL - {{#unless this.urlIsValid}} - - A valid URL is required. - - {{/unless}} - - {{! This adds enter-to-submit functionality }} - -
+ @url={{this.url}} + @title={{this.title}} + @onSave={{this.maybeSave}} + @resource={{@resource}} + @urlIsValid={{this.urlIsValid}} + @updateFormValues={{this.updateFormValues}} + @titleErrorIsShown={{this.titleErrorIsShown}} + /> @@ -55,7 +25,8 @@ data-test-edit-related-resource-modal-save-button @text="Save changes" @color="primary" - {{on "click" this.maybeSaveResource}} + {{! TODO: fix me }} + {{on "click" this.maybeSave}} /> +{{/if}} diff --git a/web/app/components/related-resources.ts b/web/app/components/related-resources.ts new file mode 100644 index 000000000..eba039473 --- /dev/null +++ b/web/app/components/related-resources.ts @@ -0,0 +1,279 @@ +import { inject as service } from "@ember/service"; +import ConfigService from "hermes/services/config"; +import AlgoliaService from "hermes/services/algolia"; +import Component from "@glimmer/component"; +import { HermesDocument } from "hermes/types/document"; +import { + RelatedExternalLink, + RelatedHermesDocument, + RelatedResource, +} from "./document/sidebar/related-resources"; +import { restartableTask, timeout } from "ember-concurrency"; +import { tracked } from "@glimmer/tracking"; +import { action } from "@ember/object"; +import { XDropdownListAnchorAPI } from "./x/dropdown-list"; +import { SearchOptions } from "instantsearch.js"; +import { next } from "@ember/runloop"; +import Ember from "ember"; + +export enum RelatedResourcesScope { + ExternalLinks = "external-links", + Documents = "documents", + All = "all", +} + +interface RelatedResourcesComponentSignature { + Element: null; + Args: { + items?: RelatedResource[]; + isLoading?: boolean; + loadingHasFailed?: boolean; + modalHeaderTitle: string; // TODO: make optional + modalInputPlaceholder: string; // TODO: make optional + documentObjectID?: string; + optionalSearchFilters?: string; + searchFilters?: string; + addResource: (resource: RelatedResource) => void; + scope: `${RelatedResourcesScope}`; + }; + Blocks: { + header: [ + rr: { + showModal: () => void; + } + ]; + list: [ + rr: { + items: RelatedResource[]; + } + ]; + "list-empty": []; + "list-error": []; + "list-loading": []; + }; +} + +export default class RelatedResourcesComponent extends Component { + @service("config") declare configSvc: ConfigService; + @service declare algolia: AlgoliaService; + + @tracked private _algoliaResults: HermesDocument[] | null = null; + + @tracked protected modalIsShown = false; + + /** + * Whether to show an error message in the search modal. + * Set true when an Algolia search fails. + */ + @tracked searchErrorIsShown = false; + + @tracked items: RelatedResource[] = this.args.items || []; + + /** + * The Algolia results for a query. Updated by the `search` task + * and displayed in the "add resources" modal. + */ + protected get algoliaResults(): { [key: string]: HermesDocument } { + /** + * The array initially looks like this: + * [{title: "foo", objectID: "bar"...}, ...] + * + * We transform it to look like: + * { "bar": {title: "foo", objectID: "bar"...}, ...} + */ + let documents: any = {}; + + if (this._algoliaResults) { + this._algoliaResults.forEach((doc) => { + documents[doc.objectID] = doc; + }); + } + return documents; + } + + /** + * 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; + } + + @action protected showModal() { + this.modalIsShown = true; + } + + @action protected hideModal() { + this.modalIsShown = false; + } + + @action protected addResource(resource: RelatedResource) { + this.args.addResource(resource); + this.hideModal(); + } + + /** + * The action run when a search errors. Resets the Algolia results + * and causes a search error to appear. + */ + @action private handleSearchError(e: unknown) { + // This triggers the "no matches" block, + // which is where we're displaying the error. + this.resetAlgoliaResults(); + this.searchErrorIsShown = true; + console.error("Algolia search failed", e); + } + + protected get relatedDocuments() { + return ( + (this.args.items?.filter((resource) => { + return "googleFileID" in resource; + }) as RelatedHermesDocument[]) ?? [] + ); + } + + protected get relatedLinks() { + return ( + (this.args.items?.filter((resource) => { + return "url" in resource; + }) as RelatedExternalLink[]) ?? [] + ); + } + + /** + * The search task passed to the "Add..." modal. + * Returns Algolia document matches for a query and updates + * the dropdown with the correct menu item IDs. + * Runs whenever the input value changes. + */ + protected search = restartableTask( + async ( + dd: XDropdownListAnchorAPI | null, + query: string, + shouldIgnoreDelay?: boolean, + options?: SearchOptions + ) => { + let index = this.configSvc.config.algolia_docs_index_name; + + let filterString = ""; + + // Make sure the current document is omitted from the results + if (this.args.documentObjectID) { + filterString = `(NOT objectID:"${this.args.documentObjectID}")`; + } + + // And if there are any related documents, omit those too + if (this.relatedDocuments.length) { + let relatedDocIDs = this.relatedDocuments.map( + (doc) => doc.googleFileID + ); + + filterString = filterString.slice(0, -1) + " "; + + filterString += `AND NOT objectID:"${relatedDocIDs.join( + '" AND NOT objectID:"' + )}")`; + } + + // If there are search filters, e.g., "doctype:RFC" add them to the query + if (this.args.searchFilters) { + 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, { + hitsPerPage: options?.hitsPerPage || 4, + filters: filterString, + attributesToRetrieve: [ + "title", + "product", + "docNumber", + "docType", + "status", + "owners", + ], + + // 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: maybeOptionalFilters, + }) + .then((response) => response); + if (algoliaResponse) { + this._algoliaResults = algoliaResponse.hits as HermesDocument[]; + if (dd) { + dd.resetFocusedItemIndex(); + } + } + if (dd) { + next(() => { + dd.scheduleAssignMenuItemIDs(); + }); + } + this.searchErrorIsShown = false; + + if (!shouldIgnoreDelay) { + // This will show the "loading" spinner for some additional time + // unless the task is restarted. This is to prevent the spinner + // from flashing when the user types and results return quickly. + await timeout(Ember.testing ? 0 : 200); + } + } catch (e: unknown) { + this.handleSearchError(e); + } + } + ); + + /** + * 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) => { + 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(); + }); + } + } + } catch (e: unknown) { + const typedError = e as { status?: number }; + if (typedError.status === 404) { + // This means the document wasn't found. + // Let the child component handle the error. + throw e; + } else { + this.handleSearchError(e); + } + } + } + ); +} + +declare module "@glint/environment-ember-loose/registry" { + export default interface Registry { + RelatedResources: typeof RelatedResourcesComponent; + } +} diff --git a/web/app/components/related-resources/add.hbs b/web/app/components/related-resources/add.hbs new file mode 100644 index 000000000..c8fe788b9 --- /dev/null +++ b/web/app/components/related-resources/add.hbs @@ -0,0 +1,110 @@ +{{#in-element (html-element ".ember-application") insertBefore=null}} + + + {{@headerTitle}} + + + {{#if this.scopeIsExternalLinks}} + {{! FIXME }} + THIS NEED TO BE THE FORM + {{! }} + {{else}} + + <:anchor as |dd|> +
+ + {{#if @searchIsRunning}} +
+ +
+ {{/if}} +
+ + <:header> + {{#if this.listHeaderIsShown}} +
+

+ {{#if this.query.length}} + Results + {{else}} + Suggestions + {{/if}} +

+
+ {{/if}} + {{#if this.externalResourceFormIsShown}} + + {{/if}} + + <:loading> + + + <:no-matches> + {{#unless this.noResultsMessageIsHidden}} +
+

+ {{this.noMatchesMessage}} +

+
+ {{/unless}} + + <:item as |dd|> + + + + + + {{/if}} + + +{{/in-element}} diff --git a/web/app/components/document/sidebar/related-resources/add.ts b/web/app/components/related-resources/add.ts similarity index 92% rename from web/app/components/document/sidebar/related-resources/add.ts rename to web/app/components/related-resources/add.ts index 57076752a..703b135db 100644 --- a/web/app/components/document/sidebar/related-resources/add.ts +++ b/web/app/components/related-resources/add.ts @@ -17,8 +17,9 @@ 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"; +import { RelatedResourcesScope } from "../related-resources"; -interface DocumentSidebarRelatedResourcesAddComponentSignature { +interface RelatedResourcesAddComponentSignature { Element: null; Args: { onClose: () => void; @@ -34,12 +35,13 @@ interface DocumentSidebarRelatedResourcesAddComponentSignature { options?: SearchOptions ) => Promise; getObject: (dd: XDropdownListAnchorAPI | null, id: string) => Promise; - allowAddingExternalLinks?: boolean; + // TODO: remove headerTitle: string; inputPlaceholder: string; searchErrorIsShown?: boolean; searchIsRunning?: boolean; resetAlgoliaResults: () => void; + scope: `${RelatedResourcesScope}`; }; Blocks: { default: []; @@ -74,7 +76,7 @@ enum FirstPartyURLFormat { FullURL = "fullURL", } -export default class DocumentSidebarRelatedResourcesAddComponent extends Component { +export default class RelatedResourcesAddComponent extends Component { @service("config") declare configSvc: ConfigService; @service("fetch") declare fetchSvc: FetchService; @service declare flashMessages: FlashMessageService; @@ -149,6 +151,13 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return this.args.algoliaResults; } + private allowAddingExternalLinks = + this.args.scope === RelatedResourcesScope.ExternalLinks || + this.args.scope === RelatedResourcesScope.All; + + private scopeIsExternalLinks = + this.args.scope === RelatedResourcesScope.ExternalLinks; + /** * Whether the query is an external URL. * Used as a shorthand check when determining layout and behavior. @@ -186,13 +195,25 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * True unless the query is a URL and adding external links is allowed. */ protected get listIsShown(): boolean { - if (this.args.allowAddingExternalLinks) { + if (this.scopeIsExternalLinks) { + return false; + } + + if (this.allowAddingExternalLinks) { return !this.queryIsExternalURL; } else { return true; } } + protected get externalResourceFormIsShown() { + if (this.args.scope === RelatedResourcesScope.ExternalLinks) { + return true; + } else { + return this.allowAddingExternalLinks && this.queryIsExternalURL; + } + } + /** * The message to show in the "<:no-matches>" block * when the query errors, detects a duplicate, or returns no results. @@ -212,6 +233,10 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone * True when there's results to show. */ protected get listHeaderIsShown(): boolean { + if (this.scopeIsExternalLinks) { + return false; + } + if (this.noMatchesFound) { return false; } @@ -222,8 +247,7 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone } return !this.linkIsDuplicate; } - - if (this.args.allowAddingExternalLinks) { + if (this.allowAddingExternalLinks) { return !this.queryIsExternalURL; } @@ -247,7 +271,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone if (this.args.searchErrorIsShown) { return false; } - if (this.args.allowAddingExternalLinks) { + // TODO: replace with `scope` + if (this.allowAddingExternalLinks) { return this.queryIsExternalURL || this.queryIsEmpty; } else { return false; @@ -434,8 +459,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone return; } } - - if (this.args.allowAddingExternalLinks) { + // TODO: replace with `scope` + if (this.allowAddingExternalLinks) { this.queryType = RelatedResourceQueryType.ExternalLink; return; } @@ -551,8 +576,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone const docType = urlParts[urlParts.length - 2]; const docNumber = urlParts[urlParts.length - 1]; const hasTypeAndNumber = docType && docNumber; - - if (this.args.allowAddingExternalLinks) { + // TODO: replace with `scope` + if (this.allowAddingExternalLinks) { if (!hasTypeAndNumber) { handleAsExternalLink(); return; @@ -575,7 +600,8 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone const firstResult = Object.values(this.algoliaResults)[0] as HermesDocument; if (this.noMatchesFound) { - if (this.args.allowAddingExternalLinks) { + // TODO: replace with `scope` + if (this.allowAddingExternalLinks) { handleAsExternalLink(); return; } @@ -628,6 +654,6 @@ export default class DocumentSidebarRelatedResourcesAddComponent extends Compone declare module "@glint/environment-ember-loose/registry" { export default interface Registry { - "Document::Sidebar::RelatedResources::Add": typeof DocumentSidebarRelatedResourcesAddComponent; + "RelatedResources::Add": typeof RelatedResourcesAddComponent; } } diff --git a/web/app/components/document/sidebar/related-resources/add/document.hbs b/web/app/components/related-resources/add/document.hbs similarity index 100% rename from web/app/components/document/sidebar/related-resources/add/document.hbs rename to web/app/components/related-resources/add/document.hbs diff --git a/web/app/components/related-resources/add/document.ts b/web/app/components/related-resources/add/document.ts new file mode 100644 index 000000000..4aedc06cc --- /dev/null +++ b/web/app/components/related-resources/add/document.ts @@ -0,0 +1,19 @@ +import Component from "@glimmer/component"; +import { HermesDocument } from "hermes/types/document"; + +interface RelatedResourcesAddDocumentComponentSignature { + Args: { + document: HermesDocument; + }; + Blocks: { + default: []; + }; +} + +export default class RelatedResourcesAddDocumentComponent extends Component {} + +declare module "@glint/environment-ember-loose/registry" { + export default interface Registry { + "RelatedResources::Add::Document": typeof RelatedResourcesAddDocumentComponent; + } +} diff --git a/web/app/components/document/sidebar/related-resources/add/external-resource.hbs b/web/app/components/related-resources/add/external-resource.hbs similarity index 100% rename from web/app/components/document/sidebar/related-resources/add/external-resource.hbs rename to web/app/components/related-resources/add/external-resource.hbs diff --git a/web/app/components/document/sidebar/related-resources/add/external-resource.ts b/web/app/components/related-resources/add/external-resource.ts similarity index 61% rename from web/app/components/document/sidebar/related-resources/add/external-resource.ts rename to web/app/components/related-resources/add/external-resource.ts index 90a4f690a..ea7e99fe0 100644 --- a/web/app/components/document/sidebar/related-resources/add/external-resource.ts +++ b/web/app/components/related-resources/add/external-resource.ts @@ -1,6 +1,6 @@ import Component from "@glimmer/component"; -interface DocumentSidebarRelatedResourcesAddExternalResourceSignature { +interface RelatedResourcesAddExternalResourceSignature { Element: null; Args: { title: string; @@ -12,7 +12,7 @@ interface DocumentSidebarRelatedResourcesAddExternalResourceSignature { }; } -export default class DocumentSidebarRelatedResourcesAddExternalResource extends Component { +export default class RelatedResourcesAddExternalResource extends Component { /** * Whether an error message should be shown. * True if the title is empty or the URL is a duplicate. @@ -24,6 +24,6 @@ export default class DocumentSidebarRelatedResourcesAddExternalResource extends declare module "@glint/environment-ember-loose/registry" { export default interface Registry { - "Document::Sidebar::RelatedResources::Add::ExternalResource": typeof DocumentSidebarRelatedResourcesAddExternalResource; + "RelatedResources::Add::ExternalResource": typeof RelatedResourcesAddExternalResource; } } diff --git a/web/app/components/related-resources/external-resource-form.hbs b/web/app/components/related-resources/external-resource-form.hbs new file mode 100644 index 000000000..a29f22854 --- /dev/null +++ b/web/app/components/related-resources/external-resource-form.hbs @@ -0,0 +1,37 @@ +
+
+ + Title + {{#if @titleErrorIsShown}} + + A title is required. + + {{/if}} + +
+ + URL + {{#unless @urlIsValid}} + + A valid URL is required. + + {{/unless}} + + {{! This adds enter-to-submit functionality }} + +
diff --git a/web/app/components/related-resources/external-resource-form.ts b/web/app/components/related-resources/external-resource-form.ts new file mode 100644 index 000000000..7050ee9df --- /dev/null +++ b/web/app/components/related-resources/external-resource-form.ts @@ -0,0 +1,33 @@ +import Component from "@glimmer/component"; +import { RelatedExternalLink } from "../document/sidebar/related-resources"; +import { action } from "@ember/object"; + +interface RelatedResourcesExternalResourceFormComponentSignature { + Element: HTMLFormElement; + Args: { + resource?: RelatedExternalLink; + url: string; + title: string; + onSave: () => void; + updateFormValues: () => void; + titleErrorIsShown?: boolean; + urlIsValid?: boolean; + }; + Blocks: { + default: []; + }; +} + +export default class RelatedResourcesExternalResourceFormComponent extends Component { + @action protected onSave(e: Event) { + // prevent the form from submitting on enter + e.preventDefault(); + this.args.onSave?.(); + } +} + +declare module "@glint/environment-ember-loose/registry" { + export default interface Registry { + "RelatedResources::ExternalResourceForm": typeof RelatedResourcesExternalResourceFormComponent; + } +} 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 95362a6ca..4070ac8d6 100644 --- a/web/tests/integration/components/document/sidebar/related-resources-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources-test.ts @@ -13,8 +13,7 @@ import { hbs } from "ember-cli-htmlbars"; 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"; +import { wait } from "ember-animated/."; const LOADING_ICON_SELECTOR = "[data-test-related-resources-list-loading-icon]"; const LIST_SELECTOR = "[data-test-related-resources-list]"; @@ -37,9 +36,6 @@ const EDIT_RESOURCE_URL_INPUT_SELECTOR = const EDIT_RESOURCE_SAVE_BUTTON_SELECTOR = "[data-test-edit-related-resource-modal-save-button]"; const ADD_RESOURCE_BUTTON_SELECTOR = ".sidebar-section-header-button"; -const ADD_RESOURCE_MODAL_SELECTOR = "[data-test-add-related-resource-modal]"; -const ADD_RELATED_RESOURCES_LIST_SELECTOR = - "[data-test-add-related-resources-list]"; const ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR = ".related-document-option"; const ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR = @@ -51,8 +47,6 @@ const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = "[data-test-add-external-resource-submit-button]"; const ADD_EXTERNAL_RESOURCE_MODAL_DELETE_BUTTON_SELECTOR = "[data-test-edit-related-resource-modal-delete-button]"; -const ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR = - "[data-test-add-external-resource-error]"; const EDIT_EXTERNAL_RESOURCE_ERROR_SELECTOR = "[data-test-external-resource-title-error]"; const RESOURCE_TITLE_SELECTOR = "[data-test-resource-title]"; @@ -60,14 +54,6 @@ const RESOURCE_SECONDARY_TEXT_SELECTOR = "[data-test-resource-secondary-text]"; 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; @@ -118,7 +104,7 @@ module( { return new Response(200, {}, {}); }); await click(ERROR_BUTTON_SELECTOR); - assert.dom(LIST_SELECTOR).exists("the list is shown again"); + assert + .dom("[data-test-sidebar-related-resources-empty-list]") + .exists("the list is shown again"); }); test("resources can be deleted (overflow menu)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { @@ -223,7 +210,7 @@ module( (hbs` (hbs` - - `); - - await click(ADD_RESOURCE_BUTTON_SELECTOR); - - await waitFor(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); - - assert.dom(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR).exists({ - count: 1, - }); - - await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, "XYZ"); - - await waitFor(NO_RESOURCES_FOUND_SELECTOR); + await waitFor(LIST_ITEM_SELECTOR); - assert.dom(NO_RESOURCES_FOUND_SELECTOR).exists(); + assert + .dom(LIST_ITEM_SELECTOR + " a") + .exists() + .hasAttribute("href", "/document/300"); }); test("you can add related external resources", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { @@ -387,7 +339,7 @@ module( (hbs` - - `); - - await click(ADD_RESOURCE_BUTTON_SELECTOR); - - await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, url); - - assert - .dom(ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR) - .hasText("This resource has already been added."); - - await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + await waitFor(LIST_ITEM_SELECTOR); assert - .dom(ADD_RESOURCE_MODAL_SELECTOR) - .exists("the button is disabled when the URL is a duplicate"); - - await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, "https://"); - - assert - .dom(ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR) - .doesNotExist("the error message is removed when the URL changes"); + .dom(LIST_ITEM_SELECTOR + " a") + .hasAttribute("href", "https://example.com"); }); test("you can set an item limit", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { @@ -497,7 +380,7 @@ module( @productArea={{this.document.product}} @objectID={{this.document.objectID}} @itemLimit={{1}} - @allowAddingExternalLinks={{true}} + @scope="all" @headerTitle="Test title" @modalHeaderTitle="Add related resource" @modalInputPlaceholder="Test placeholder" @@ -519,7 +402,7 @@ module( (hbs` - - `); - - await click(ADD_RESOURCE_BUTTON_SELECTOR); - - // 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) { - 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"; - - this.server.create("document", { - id: docID, - title: docTitle, - objectID: docID, - }); - - await render(hbs` - - `); - - await click(ADD_RESOURCE_BUTTON_SELECTOR); - - const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/${docID}`; - - await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); - await click(ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR); - - 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); - - // 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/related-resources-test.ts b/web/tests/integration/components/related-resources-test.ts new file mode 100644 index 000000000..29442c55c --- /dev/null +++ b/web/tests/integration/components/related-resources-test.ts @@ -0,0 +1,735 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "ember-qunit"; +import { + click, + fillIn, + find, + render, + waitFor, + waitUntil, +} from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; +import { Response } from "miragejs"; +import config from "hermes/config/environment"; +import algoliaHosts from "hermes/mirage/algolia/hosts"; +import { RelatedResource } from "hermes/components/document/sidebar/related-resources"; +import { RelatedResourcesScope } from "hermes/components/related-resources"; + +const RELATED_DOCUMENT_OPTION_SELECTOR = ".related-document-option"; +const ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR = + "[data-test-related-resources-search-input]"; +const NO_RESOURCES_FOUND_SELECTOR = "[data-test-no-related-resources-found]"; +const ADD_EXTERNAL_RESOURCE_FORM_SELECTOR = + "[data-test-add-external-resource-form]"; +const EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR = ".external-resource-title-input"; +const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = + "[data-test-add-external-resource-submit-button]"; +const ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR = + "[data-test-add-external-resource-error]"; +const ADD_RESOURCE_MODAL_SELECTOR = "[data-test-add-related-resource-modal]"; +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 RelatedResourcesComponentTestContext extends MirageTestContext { + modalHeaderTitle: string; + modalInputPlaceholder: string; + addResource: (resource: RelatedResource) => void; + items?: RelatedResource[]; + isLoading?: boolean; + loadingHasFailed?: boolean; + scope: `${RelatedResourcesScope}`; +} + +module("Integration | Component | related-resources", function (hooks) { + setupRenderingTest(hooks); + setupMirage(hooks); + + hooks.beforeEach(function (this: RelatedResourcesComponentTestContext) { + this.set("modalHeaderTitle", "Modal title"); + this.set("modalInputPlaceholder", "Modal input placeholder"); + this.set("addResource", (resource: RelatedResource) => { + const items = this.items || []; + this.set("items", [...items, resource]); + }); + this.set("items", undefined); + }); + + test("it yields list blocks", async function (this: RelatedResourcesComponentTestContext, assert) { + await render(hbs` + + <:list as |rr|> +
+ {{#each rr.items as |item|}} + {{!@glint-ignore}} +
{{item}}
+ {{/each}} + + <:list-empty> +
+ + + `); + + assert + .dom(".empty") + .exists("the list-empty block yields when there are no items"); + + assert.dom(".list").doesNotExist(); + + this.set("items", ["item1", "item2"]); + + assert.dom(".empty").doesNotExist(); + assert.dom(".list").exists(); + assert.dom(".item").exists({ count: 2 }); + }); + + test("it yields a loading block", async function (this: RelatedResourcesComponentTestContext, assert) { + this.set("isLoading", true); + + await render(hbs` + + <:list-loading> +
+ + <:list-empty> +
+ + + `); + + assert.dom(".loading").exists(); + assert.dom(".empty").doesNotExist(); + + this.set("isLoading", false); + + assert.dom(".loading").doesNotExist(); + assert.dom(".empty").exists(); + }); + + test("it yields an error block when related resources fail to load", async function (this: RelatedResourcesComponentTestContext, assert) { + this.set("loadingHasFailed", true); + + await render(hbs` + + <:list-error> +
+ + <:list-empty> +
+ + + `); + + assert.dom(".error").exists(); + assert.dom(".empty").doesNotExist(); + + this.set("loadingHasFailed", false); + + assert.dom(".error").doesNotExist(); + assert.dom(".empty").exists(); + }); + + test('it yields a header block with a "show modal" action', async function (this: RelatedResourcesComponentTestContext, assert) { + await render(hbs` + + <:header as |rr|> + + + + `); + + assert.dom("button").exists(); + + await click("button"); + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + + assert.dom(ADD_RESOURCE_MODAL_SELECTOR).exists(); + }); + + // do we want this component to handle editing, removing? + + test("you can add related hermes documents", async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + this.set("addResource", (resource: RelatedResource) => { + this.set("items", [resource]); + }); + + await render(hbs` + + <:header as |rr|> + + + <:list as |rr|> + {{! can this improve}} +
+ {{#each rr.items as |item|}} + {{!@glint-ignore}} +
{{item.title}}
+ {{/each}} + + <:list-empty> +
+ + + `); + + assert.dom(".empty").exists(); + + await click("button"); + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + await click(RELATED_DOCUMENT_OPTION_SELECTOR); + + assert.dom(ADD_RESOURCE_MODAL_SELECTOR).doesNotExist(); + assert.dom(".empty").doesNotExist(); + + assert.dom(".list").exists(); + assert.dom(".item").hasText("Test Document 0"); + }); + + test('it shows a "no results" fallback message when searching documents', async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + + assert.dom(RELATED_DOCUMENT_OPTION_SELECTOR).exists({ count: 3 }); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, "XYZ"); + + await waitFor(NO_RESOURCES_FOUND_SELECTOR); + + assert.dom(NO_RESOURCES_FOUND_SELECTOR).exists(); + + assert.dom(RELATED_DOCUMENT_OPTION_SELECTOR).doesNotExist(); + }); + + test("you can add related external links as a fallback", async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + await render(hbs` + + <:header as |rr|> + + + <:list as |rr|> +
+ {{#each rr.items as |item|}} + {{!@glint-ignore}} +
{{item.name}}
+ {{/each}} + + + `); + + assert.dom(".list").doesNotExist(); + + await click("button"); + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + + assert.dom(RELATED_DOCUMENT_OPTION_SELECTOR).exists({ count: 3 }); + + await fillIn( + ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, + "https://example.com" + ); + + assert + .dom(RELATED_DOCUMENT_OPTION_SELECTOR) + .doesNotExist("documents are removed when a valid URL is entered"); + assert.dom(ADD_EXTERNAL_RESOURCE_FORM_SELECTOR).exists(); + + assert + .dom(EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR) + .hasAttribute("placeholder", "Enter a title"); + + // Try to add a resource without a title + + await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + + // Confirm that it fails + + assert + .dom(ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR) + .hasText("A title is required."); + + // Now add a a title + + await fillIn(EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR, "Example"); + await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + + assert.dom(ADD_RESOURCE_MODAL_SELECTOR).doesNotExist("the modal is closed"); + assert.dom(".item").hasText("Example"); + }); + + test("it prevents duplicate external links", async function (this: RelatedResourcesComponentTestContext, assert) { + const url = "https://example.com"; + + this.server.create("relatedExternalLink", { + url, + }); + + this.set("items", this.server.schema.relatedExternalLinks.all().models); + + await render(hbs` + + <:header as |rr|> + + + <:list as |rr|> +
+ {{#each rr.items as |item|}} + {{!@glint-ignore}} +
{{item}}
+ {{/each}} + + + `); + + await click("button"); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, url); + + assert + .dom(ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR) + .hasText("This resource has already been added."); + + await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + + assert + .dom(ADD_RESOURCE_MODAL_SELECTOR) + .exists("the button is disabled when the URL is a duplicate"); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, "https://"); + + assert + .dom(ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR) + .doesNotExist("the error message is removed when the URL changes"); + }); + + test("you can scope the component to document resource", async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + + await fillIn( + ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, + "https://hashicorp.com" + ); + + assert.dom(NO_RESOURCES_FOUND_SELECTOR).exists(); + assert.dom(EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR).doesNotExist(); + }); + + test("you can scope the component to external-link resources", async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + + assert.dom(RELATED_DOCUMENT_OPTION_SELECTOR).doesNotExist(); + // TODO: assert the form exists + }); + + test("first-class links are recognized (full URL)", async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + const docID = "777"; + const docTitle = "Jackpot!"; + + this.server.create("document", { + id: docID, + title: docTitle, + }); + + this.server.create("document"); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + // Construct a "valid" first-class Hermes URL + const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/${docID}`; + + await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); + await waitFor(RELATED_DOCUMENT_OPTION_SELECTOR); + + assert + .dom(RELATED_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(RELATED_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(RELATED_DOCUMENT_OPTION_SELECTOR); + + assert + .dom(RELATED_DOCUMENT_OPTION_SELECTOR) + .containsText(docTitle, "the Google URL is correctly parsed"); + }); + + test("first-class links are recognized (short URL)", async function (this: RelatedResourcesComponentTestContext, 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` + + <:header as |rr|> + + + + `); + + const shortLink = `${SHORT_LINK_BASE_URL}/${docType}/${docNumber}`; + + await click("button"); + + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); + + await waitFor(RELATED_DOCUMENT_OPTION_SELECTOR); + + assert + .dom(RELATED_DOCUMENT_OPTION_SELECTOR) + .containsText(docTitle, "the shortLink is correctly parsed"); + }); + + test("an invalid hermes URL is handled like an external link", async function (this: RelatedResourcesComponentTestContext, assert) { + await render(hbs` + + <:header as |rr|> + + + + `); + + // 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("button"); + 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: RelatedResourcesComponentTestContext, assert) { + await render(hbs` + + <:header as |rr|> + + + + `); + + const shortLink = `${SHORT_LINK_BASE_URL}/RFC/VLT-999`; + + await click("button"); + 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: RelatedResourcesComponentTestContext, assert) { + const docID = "777"; + const docTitle = "Foo"; + + this.server.create("document", { + id: docID, + title: docTitle, + objectID: docID, + }); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + const documentURL = `${CURRENT_DOMAIN_PROTOCOL}${CURRENT_DOMAIN}:${CURRENT_PORT}/document/${docID}`; + + // Find and add the document + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); + await click(RELATED_DOCUMENT_OPTION_SELECTOR); + + // Reopen the modal and paste the same URL + await click("button"); + 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 (short URL)", async function (this: RelatedResourcesComponentTestContext, 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` + + <:header as |rr|> + + + + `); + + const shortLink = `${SHORT_LINK_BASE_URL}/RFC/${docNumber}`; + + await click("button"); + + // Find and add the document + await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); + await click(RELATED_DOCUMENT_OPTION_SELECTOR); + + // Reopen the modal and paste the same URL + await click("button"); + 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: RelatedResourcesComponentTestContext, assert) { + algoliaHosts.forEach((host) => { + this.server.get(host, () => { + return new Response(500, {}, {}); + }); + }); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + // 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); + + await waitUntil(() => { + return find(NO_RESOURCES_FOUND_SELECTOR)?.textContent?.includes( + SEARCH_ERROR_MESSAGE + ); + }); + + assert.dom(NO_RESOURCES_FOUND_SELECTOR).containsText(SEARCH_ERROR_MESSAGE); + }); + + test("it shows an error when searching fails", async function (this: RelatedResourcesComponentTestContext, assert) { + this.server.createList("document", 3); + + algoliaHosts.forEach((host) => { + this.server.post(host, () => { + return new Response(500, {}, {}); + }); + }); + + await render(hbs` + + <:header as |rr|> + + + + `); + + await click("button"); + + 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/related-resources/add-test.ts similarity index 80% rename from web/tests/integration/components/document/sidebar/related-resources/add-test.ts rename to web/tests/integration/components/related-resources/add-test.ts index e2b98ddb5..0b158b295 100644 --- a/web/tests/integration/components/document/sidebar/related-resources/add-test.ts +++ b/web/tests/integration/components/related-resources/add-test.ts @@ -10,6 +10,7 @@ import { hbs } from "ember-cli-htmlbars"; import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; import { HermesDocument } from "hermes/types/document"; import { XDropdownListAnchorAPI } from "hermes/components/x/dropdown-list"; +import { RelatedResourcesScope } from "hermes/components/related-resources"; const MODAL_TITLE_SELECTOR = "[data-test-add-related-resource-modal-title]"; const SEARCH_INPUT_SELECTOR = "[data-test-related-resources-search-input]"; @@ -18,14 +19,13 @@ const LIST_ITEM_SELECTOR = ".x-dropdown-list-item"; const DOCUMENT_OPTION_SELECTOR = ".related-document-option"; const NO_MATCHES_SELECTOR = ".related-resources-modal-body-header"; -interface DocumentSidebarRelatedResourcesAddTestContext - extends MirageTestContext { +interface RelatedResourcesAddTestContext extends MirageTestContext { noop: () => void; search: (dd: XDropdownListAnchorAPI | null, query: string) => Promise; getObject: (dd: XDropdownListAnchorAPI | null, id: string) => Promise; shownDocuments: Record; - allowAddingExternalLinks: boolean; searchIsRunning: boolean; + scope: `${RelatedResourcesScope}`; } module( @@ -34,9 +34,7 @@ module( setupRenderingTest(hooks); setupMirage(hooks); - hooks.beforeEach(function ( - this: DocumentSidebarRelatedResourcesAddTestContext - ) { + hooks.beforeEach(function (this: RelatedResourcesAddTestContext) { this.server.createList("document", 10); this.set("noop", () => {}); @@ -93,14 +91,15 @@ module( }); }); - test("it renders correctly (initial load)", async function (this: DocumentSidebarRelatedResourcesAddTestContext, assert) { - await render(hbs` - (hbs` + (hbs` - (hbs` + ; }); - await render(hbs` - (hbs` + (hbs` - (hbs` + (hbs` - (hbs` + (hbs` - (hbs` + (hbs` - (hbs` + void; onInput: () => void; } @@ -14,12 +13,12 @@ module( function (hooks) { setupRenderingTest(hooks); - test("it renders as expected", async function (this: DocumentSidebarRelatedResourcesAddExternalResourceTestContext, assert) { + test("it renders as expected", async function (this: RelatedResourcesAddExternalResourceTestContext, assert) { this.set("onSubmit", () => {}); this.set("onInput", () => {}); - await render(hbs` - (hbs` + Date: Wed, 6 Sep 2023 21:38:46 -0400 Subject: [PATCH 02/15] Improvements to Link-scoped modal --- web/app/components/related-resources/add.hbs | 27 ++++++++++++++++--- web/app/components/related-resources/add.ts | 13 +++++---- .../add/external-resource.hbs | 2 +- .../add/external-resource.ts | 8 +++++- .../external-resource-form.hbs | 5 ++-- .../external-resource-form.ts | 11 ++++++-- .../components/related-resources-test.ts | 1 + 7 files changed, 53 insertions(+), 14 deletions(-) diff --git a/web/app/components/related-resources/add.hbs b/web/app/components/related-resources/add.hbs index c8fe788b9..1be3a7c09 100644 --- a/web/app/components/related-resources/add.hbs +++ b/web/app/components/related-resources/add.hbs @@ -2,7 +2,7 @@ @@ -11,8 +11,14 @@ {{#if this.scopeIsExternalLinks}} {{! FIXME }} - THIS NEED TO BE THE FORM - {{! }} + {{else}} {{/if}} + {{#if this.scopeIsExternalLinks}} + + + + + + + {{/if}} {{/in-element}} diff --git a/web/app/components/related-resources/add.ts b/web/app/components/related-resources/add.ts index 703b135db..05fe1ccbd 100644 --- a/web/app/components/related-resources/add.ts +++ b/web/app/components/related-resources/add.ts @@ -299,10 +299,7 @@ export default class RelatedResourcesAddComponent extends Component
{{! TODO: Combine with `ListItem::Edit` }} -
+ void; + onSubmit: () => void; onInput: (e: Event) => void; linkIsDuplicate?: boolean; titleErrorIsShown?: boolean; @@ -20,6 +21,11 @@ export default class RelatedResourcesAddExternalResource extends Component
URL + {{! TODO: make this more accompanying to External-scoped components }} {{#unless @urlIsValid}} A valid URL is required. diff --git a/web/app/components/related-resources/external-resource-form.ts b/web/app/components/related-resources/external-resource-form.ts index 7050ee9df..7112a1470 100644 --- a/web/app/components/related-resources/external-resource-form.ts +++ b/web/app/components/related-resources/external-resource-form.ts @@ -9,7 +9,7 @@ interface RelatedResourcesExternalResourceFormComponentSignature { url: string; title: string; onSave: () => void; - updateFormValues: () => void; + updateFormValues?: () => void; titleErrorIsShown?: boolean; urlIsValid?: boolean; }; @@ -20,10 +20,17 @@ interface RelatedResourcesExternalResourceFormComponentSignature { export default class RelatedResourcesExternalResourceFormComponent extends Component { @action protected onSave(e: Event) { - // prevent the form from submitting on enter e.preventDefault(); this.args.onSave?.(); } + + @action protected updateFormValues(e: Event) { + if (this.args.updateFormValues) { + this.args.updateFormValues(); + } else { + console.log("should i do something here"); + } + } } declare module "@glint/environment-ember-loose/registry" { diff --git a/web/tests/integration/components/related-resources-test.ts b/web/tests/integration/components/related-resources-test.ts index 29442c55c..8667e2cf5 100644 --- a/web/tests/integration/components/related-resources-test.ts +++ b/web/tests/integration/components/related-resources-test.ts @@ -421,6 +421,7 @@ module("Integration | Component | related-resources", function (hooks) { await waitFor(ADD_RESOURCE_MODAL_SELECTOR); assert.dom(RELATED_DOCUMENT_OPTION_SELECTOR).doesNotExist(); + await this.pauseTest(); // TODO: assert the form exists }); From 05240f2e897dd699905c856add01ba84d23c1ec0 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Thu, 7 Sep 2023 10:08:01 -0400 Subject: [PATCH 03/15] Refactor Edit modal --- .../related-resources/list-item/edit.hbs | 46 ++++--- .../related-resources/list-item/edit.ts | 102 ++------------- web/app/components/related-resources/add.hbs | 8 +- web/app/components/related-resources/add.ts | 12 +- .../external-resource-form.hbs | 23 +++- .../external-resource-form.ts | 118 +++++++++++++++--- 6 files changed, 165 insertions(+), 144 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources/list-item/edit.hbs b/web/app/components/document/sidebar/related-resources/list-item/edit.hbs index 908285ef8..fc4813c32 100644 --- a/web/app/components/document/sidebar/related-resources/list-item/edit.hbs +++ b/web/app/components/document/sidebar/related-resources/list-item/edit.hbs @@ -8,26 +8,15 @@ Edit resource - +
+ {{! Content placed via in-element }} +
- + + {{! Button placed via in-element }} + {{/in-element}} + +{{#if this.formIsRendered}} + {{! TODO: explain this: }} + {{#in-element (html-element this.bodySelector) insertBefore=null}} + + <:submit as |rr|> + {{#in-element (html-element this.submitSelector) insertBefore=null}} + + {{/in-element}} + + + {{/in-element}} +{{else}} +
+{{/if}} diff --git a/web/app/components/document/sidebar/related-resources/list-item/edit.ts b/web/app/components/document/sidebar/related-resources/list-item/edit.ts index 903bdb131..4215a2aa9 100644 --- a/web/app/components/document/sidebar/related-resources/list-item/edit.ts +++ b/web/app/components/document/sidebar/related-resources/list-item/edit.ts @@ -1,9 +1,8 @@ -import { assert } from "@ember/debug"; import { action } from "@ember/object"; +import { guidFor } from "@ember/object/internals"; import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; import { RelatedExternalLink } from "hermes/components/document/sidebar/related-resources"; -import isValidURL from "hermes/utils/is-valid-u-r-l"; interface DocumentSidebarRelatedResourcesListItemEditComponentSignature { Args: { @@ -18,101 +17,14 @@ interface DocumentSidebarRelatedResourcesListItemEditComponentSignature { } export default class DocumentSidebarRelatedResourcesListItemEditComponent extends Component { - /** - * A locally tracked URL property. Starts as the passed-in value; - * updated when the URL input-value changes. - */ - @tracked protected url = this.args.resource ? this.args.resource.url : ""; + protected id = guidFor(this); + protected bodySelector = `#${this.id}-body`; + protected submitSelector = `#${this.id}-submit`; - /** - * The title of the resource. If the name is the same as the URL, - * we treat it like it's an empty value so the placeholder text shows. - */ - @tracked protected title = !this.args.resource - ? "" - : this.args.resource.name === this.args.resource.url - ? "" - : this.args.resource.name; + @tracked protected formIsRendered = false; - /** - * Whether the URL is valid, as determined by the `isValidURL` utility. - * Used to dictate whether an error message is shown. - */ - @tracked protected urlIsValid = true; - - /** - * Whether the error warning is shown. - * True if the title is empty on submit. - */ - @tracked protected titleErrorIsShown = false; - - /** - * A local reference to the form element. - * Registered when inserted. - */ - @tracked private _form: HTMLFormElement | null = null; - - /** - * An asserted-true reference to the form element. - */ - protected get form(): HTMLFormElement { - assert("this._form must exist", this._form); - return this._form; - } - - /** - * The action to register the form element locally. - * Called when the form is rendered. - */ - @action protected registerForm(form: HTMLFormElement): void { - this._form = form; - } - - /** - * The action that updates the local form properties and eagerly validates the URL. - * Runs on the "input" events of the URL and title inputs. - */ - @action protected updateFormValues(): void { - const formObject = Object.fromEntries(new FormData(this.form).entries()); - - const title = formObject["title"]; - const url = formObject["url"]; - - assert("title must be a string", typeof title === "string"); - assert("url must be a string", typeof url === "string"); - - this.title = title; - this.url = url; - - this.validateURL(); - } - - /** - * The action that validates the locally tracked URL. - * Updates the `urlIsValid` property, which is used to show - * an error message for invalid URLs. - */ - @action private validateURL() { - this.urlIsValid = isValidURL(this.url); - } - - @action maybeSave() { - this.validateURL(); - - if (!this.title) { - this.titleErrorIsShown = true; - return; - } - - if (this.urlIsValid) { - if (this.args.onSave) { - this.args.onSave({ - sortOrder: this.args.resource.sortOrder, - name: this.title, - url: this.url, - }); - } - } + @action protected renderForm() { + this.formIsRendered = true; } } diff --git a/web/app/components/related-resources/add.hbs b/web/app/components/related-resources/add.hbs index 1be3a7c09..a027b5311 100644 --- a/web/app/components/related-resources/add.hbs +++ b/web/app/components/related-resources/add.hbs @@ -10,14 +10,12 @@ {{#if this.scopeIsExternalLinks}} - {{! FIXME }} {{else}} +
Title - {{#if @titleErrorIsShown}} + {{#if this.titleErrorIsShown}} A title is required. @@ -20,19 +25,27 @@ URL {{! TODO: make this more accompanying to External-scoped components }} - {{#unless @urlIsValid}} + {{#unless this.urlIsValid}} A valid URL is required. {{/unless}} + {{! This adds enter-to-submit functionality }} + + {{#if (has-block "submit")}} + {{yield + (hash submit=this.maybeSubmit formIsValid=this.formIsValid) + to="submit" + }} + {{/if}} diff --git a/web/app/components/related-resources/external-resource-form.ts b/web/app/components/related-resources/external-resource-form.ts index 7112a1470..84d7edabe 100644 --- a/web/app/components/related-resources/external-resource-form.ts +++ b/web/app/components/related-resources/external-resource-form.ts @@ -1,34 +1,124 @@ import Component from "@glimmer/component"; import { RelatedExternalLink } from "../document/sidebar/related-resources"; import { action } from "@ember/object"; +import { tracked } from "@glimmer/tracking"; +import { assert } from "@ember/debug"; +import isValidURL from "hermes/utils/is-valid-u-r-l"; interface RelatedResourcesExternalResourceFormComponentSignature { Element: HTMLFormElement; Args: { resource?: RelatedExternalLink; - url: string; - title: string; - onSave: () => void; - updateFormValues?: () => void; - titleErrorIsShown?: boolean; - urlIsValid?: boolean; + onSave: (resource: RelatedExternalLink) => void; }; Blocks: { default: []; + submit: [ + rr: { + submit: (e: Event) => void; + formIsValid: boolean; + } + ]; }; } export default class RelatedResourcesExternalResourceFormComponent extends Component { - @action protected onSave(e: Event) { - e.preventDefault(); - this.args.onSave?.(); + /** + * A locally tracked URL property. Starts as the passed-in value; + * updated when the URL input-value changes. + */ + @tracked protected url = this.args.resource ? this.args.resource.url : ""; + + /** + * The title of the resource. If the name is the same as the URL, + * we treat it like it's an empty value so the placeholder text shows. + */ + @tracked protected title = !this.args.resource + ? "" + : this.args.resource.name === this.args.resource.url + ? "" + : this.args.resource.name; + + /** + * Whether the URL is valid, as determined by the `isValidURL` utility. + * Used to dictate whether an error message is shown. + */ + @tracked protected urlIsValid = true; + + /** + * Whether the error warning is shown. + * True if the title is empty on submit. + */ + @tracked protected titleErrorIsShown = false; + + /** + * A local reference to the form element. + * Registered when inserted. + */ + @tracked private _form: HTMLFormElement | null = null; + + /** + * An asserted-true reference to the form element. + */ + protected get form(): HTMLFormElement { + assert("this._form must exist", this._form); + return this._form; } - @action protected updateFormValues(e: Event) { - if (this.args.updateFormValues) { - this.args.updateFormValues(); - } else { - console.log("should i do something here"); + /** + * The action to register the form element locally. + * Called when the form is rendered. + */ + @action protected registerForm(form: HTMLFormElement): void { + this._form = form; + } + + /** + * The action that updates the local form properties and eagerly validates the URL. + * Runs on the "input" events of the URL and title inputs. + */ + @action protected updateFormValues(): void { + const formObject = Object.fromEntries(new FormData(this.form).entries()); + + const title = formObject["title"]; + const url = formObject["url"]; + + assert("title must be a string", typeof title === "string"); + assert("url must be a string", typeof url === "string"); + + this.title = title; + this.url = url; + + this.processURL(); + } + + /** + * The action that validates the locally tracked URL. + * Updates the `urlIsValid` property, which is used to show + * an error message for invalid URLs. + */ + @action private processURL() { + this.urlIsValid = isValidURL(this.url); + } + + @action maybeSubmit(e: Event) { + e.preventDefault(); + + this.processURL(); + + if (!this.title) { + this.titleErrorIsShown = true; + return; + } + + if (this.urlIsValid) { + const sortOrder = this.args.resource ? this.args.resource.sortOrder : 1; + console.log("submitting", sortOrder, this.title, this.url); + this.args.onSave({ + sortOrder, + name: this.title, + url: this.url, + }); } } } From 590a581f2394924659e8e440f926297e730d38a9 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Thu, 7 Sep 2023 10:48:41 -0400 Subject: [PATCH 04/15] Add submit function to strict "Add" form --- .../related-resources/list-item/edit.ts | 7 ++- web/app/components/related-resources/add.hbs | 49 ++++++++++++++----- web/app/components/related-resources/add.ts | 49 ++++++++++++------- .../external-resource-form.hbs | 7 ++- .../external-resource-form.ts | 9 ++-- .../components/related-resources-test.ts | 19 ++++++- 6 files changed, 102 insertions(+), 38 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources/list-item/edit.ts b/web/app/components/document/sidebar/related-resources/list-item/edit.ts index 4215a2aa9..63da2cb0c 100644 --- a/web/app/components/document/sidebar/related-resources/list-item/edit.ts +++ b/web/app/components/document/sidebar/related-resources/list-item/edit.ts @@ -18,8 +18,11 @@ interface DocumentSidebarRelatedResourcesListItemEditComponentSignature { export default class DocumentSidebarRelatedResourcesListItemEditComponent extends Component { protected id = guidFor(this); - protected bodySelector = `#${this.id}-body`; - protected submitSelector = `#${this.id}-submit`; + protected bodyID = `${this.id}-body`; + protected submitID = `${this.id}-submit`; + + protected bodySelector = `#${this.bodyID}`; + protected submitSelector = `#${this.submitID}`; @tracked protected formIsRendered = false; diff --git a/web/app/components/related-resources/add.hbs b/web/app/components/related-resources/add.hbs index a027b5311..2c1a2703a 100644 --- a/web/app/components/related-resources/add.hbs +++ b/web/app/components/related-resources/add.hbs @@ -10,13 +10,9 @@ {{#if this.scopeIsExternalLinks}} - +
+ {{! Positioned via in-element }} +
{{else}} - + + {{! Positioned via in-element }} + {{/in-element}} + +{{! TODO: explain this }} +{{#if this.scopeIsExternalLinks}} + {{#if this.externalLinkFormIsRendered}} + {{#in-element + (html-element this.externalLinkFormBodySelector) + insertBefore=null + }} + + <:submit as |rr|> + {{#in-element + (html-element this.externalLinkFormSubmitSelector) + insertBefore=null + }} + + {{/in-element}} + + + {{/in-element}} + {{else}} +
+ {{/if}} +{{/if}} diff --git a/web/app/components/related-resources/add.ts b/web/app/components/related-resources/add.ts index 39d73763e..77aafce9e 100644 --- a/web/app/components/related-resources/add.ts +++ b/web/app/components/related-resources/add.ts @@ -18,6 +18,7 @@ import FetchService from "hermes/services/fetch"; import { XDropdownListAnchorAPI } from "hermes/components/x/dropdown-list"; import { SearchOptions } from "instantsearch.js"; import { RelatedResourcesScope } from "../related-resources"; +import { guidFor } from "@ember/object/internals"; interface RelatedResourcesAddComponentSignature { Element: null; @@ -84,6 +85,12 @@ export default class RelatedResourcesAddComponent extends Component
{{/each}} - <:list-empty> -
- `); - assert - .dom(".empty") - .exists("the list-empty block yields when there are no items"); - - assert.dom(".list").doesNotExist(); - - this.set("items", ["item1", "item2"]); - - assert.dom(".empty").doesNotExist(); assert.dom(".list").exists(); assert.dom(".item").exists({ count: 2 }); }); @@ -107,19 +97,19 @@ module("Integration | Component | related-resources", function (hooks) { <:list-loading>
- <:list-empty> -
- + <:list> +
+ `); assert.dom(".loading").exists(); - assert.dom(".empty").doesNotExist(); + assert.dom(".list").doesNotExist(); this.set("isLoading", false); assert.dom(".loading").doesNotExist(); - assert.dom(".empty").exists(); + assert.dom(".list").exists(); }); test("it yields an error block when related resources fail to load", async function (this: RelatedResourcesComponentTestContext, assert) { @@ -136,19 +126,19 @@ module("Integration | Component | related-resources", function (hooks) { <:list-error>
- <:list-empty> -
- + <:list> +
+ `); assert.dom(".error").exists(); - assert.dom(".empty").doesNotExist(); + assert.dom(".list").doesNotExist(); this.set("loadingHasFailed", false); assert.dom(".error").doesNotExist(); - assert.dom(".empty").exists(); + assert.dom(".list").exists(); }); test('it yields a header block with a "show modal" action', async function (this: RelatedResourcesComponentTestContext, assert) { @@ -202,14 +192,9 @@ module("Integration | Component | related-resources", function (hooks) {
{{item.title}}
{{/each}} - <:list-empty> -
- `); - assert.dom(".empty").exists(); - await click("button"); await waitFor(ADD_RESOURCE_MODAL_SELECTOR); From 9306c39104e97f1c2ca75d091a42426098fc41b5 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Thu, 7 Sep 2023 15:41:46 -0400 Subject: [PATCH 10/15] Refactor Add/Edit modal --- .../document/sidebar/related-resources.ts | 1 - .../sidebar/related-resources/list-item.hbs | 5 +- .../related-resources/list-item/edit.hbs | 59 ------------- .../related-resources/list-item/edit.ts | 38 -------- .../add-or-edit-external-resource-modal.hbs | 85 ++++++++++++++++++ ...=> add-or-edit-external-resource-modal.ts} | 43 ++++++--- web/app/components/related-resources/add.hbs | 87 +++++-------------- web/app/components/related-resources/add.ts | 2 +- ...rce.hbs => fallback-external-resource.hbs} | 2 - ...ource.ts => fallback-external-resource.ts} | 6 +- .../external-resource-form.hbs | 50 ----------- .../sidebar/related-resources-test.ts | 24 +++-- .../related-resources/list-item-test.ts | 8 +- .../components/related-resources-test.ts | 8 +- .../add/external-resource-test.ts | 2 +- 15 files changed, 162 insertions(+), 258 deletions(-) delete mode 100644 web/app/components/document/sidebar/related-resources/list-item/edit.hbs delete mode 100644 web/app/components/document/sidebar/related-resources/list-item/edit.ts create mode 100644 web/app/components/related-resources/add-or-edit-external-resource-modal.hbs rename web/app/components/related-resources/{external-resource-form.ts => add-or-edit-external-resource-modal.ts} (72%) rename web/app/components/related-resources/add/{external-resource.hbs => fallback-external-resource.hbs} (96%) rename web/app/components/related-resources/add/{external-resource.ts => fallback-external-resource.ts} (69%) delete mode 100644 web/app/components/related-resources/external-resource-form.hbs diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 44d3d72d5..55e6022ce 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -224,7 +224,6 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< if (resources.externalLinks) { this.relatedLinks = resources.externalLinks; } - this.loadingHasFailed = false; } catch (e: unknown) { this.loadingHasFailed = true; diff --git a/web/app/components/document/sidebar/related-resources/list-item.hbs b/web/app/components/document/sidebar/related-resources/list-item.hbs index ffbc3759a..28cdc89a4 100644 --- a/web/app/components/document/sidebar/related-resources/list-item.hbs +++ b/web/app/components/document/sidebar/related-resources/list-item.hbs @@ -36,14 +36,13 @@ @onEditClick={{this.showModal}} /> {{/unless}} -
{{#if this.modalIsShown}} - diff --git a/web/app/components/document/sidebar/related-resources/list-item/edit.hbs b/web/app/components/document/sidebar/related-resources/list-item/edit.hbs deleted file mode 100644 index fc4813c32..000000000 --- a/web/app/components/document/sidebar/related-resources/list-item/edit.hbs +++ /dev/null @@ -1,59 +0,0 @@ -{{#in-element (html-element ".ember-application") insertBefore=null}} - - - Edit resource - - -
- {{! Content placed via in-element }} -
- - - - - {{! Button placed via in-element }} - - - - - - -{{/in-element}} - -{{#if this.formIsRendered}} - {{! TODO: explain this: }} - {{#in-element (html-element this.bodySelector) insertBefore=null}} - - <:submit as |rr|> - {{#in-element (html-element this.submitSelector) insertBefore=null}} - - {{/in-element}} - - - {{/in-element}} -{{else}} -
-{{/if}} diff --git a/web/app/components/document/sidebar/related-resources/list-item/edit.ts b/web/app/components/document/sidebar/related-resources/list-item/edit.ts deleted file mode 100644 index 01bca0333..000000000 --- a/web/app/components/document/sidebar/related-resources/list-item/edit.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { action } from "@ember/object"; -import { guidFor } from "@ember/object/internals"; -import Component from "@glimmer/component"; -import { tracked } from "@glimmer/tracking"; -import { RelatedExternalLink } from "hermes/components/related-resources"; - -interface DocumentSidebarRelatedResourcesListItemEditComponentSignature { - Args: { - resource: RelatedExternalLink; - hideModal: () => void; - onSave: (resource: RelatedExternalLink) => void; - removeResource: (resource: RelatedExternalLink) => void; - }; - Blocks: { - default: []; - }; -} - -export default class DocumentSidebarRelatedResourcesListItemEditComponent extends Component { - protected id = guidFor(this); - protected bodyID = `${this.id}-body`; - protected submitID = `${this.id}-submit`; - - protected bodySelector = `#${this.bodyID}`; - protected submitSelector = `#${this.submitID}`; - - @tracked protected formIsRendered = false; - - @action protected renderForm() { - this.formIsRendered = true; - } -} - -declare module "@glint/environment-ember-loose/registry" { - export default interface Registry { - "Document::Sidebar::RelatedResources::ListItem::Edit": typeof DocumentSidebarRelatedResourcesListItemEditComponent; - } -} diff --git a/web/app/components/related-resources/add-or-edit-external-resource-modal.hbs b/web/app/components/related-resources/add-or-edit-external-resource-modal.hbs new file mode 100644 index 000000000..39264e1fd --- /dev/null +++ b/web/app/components/related-resources/add-or-edit-external-resource-modal.hbs @@ -0,0 +1,85 @@ +{{#in-element (html-element ".ember-application") insertBefore=null}} + + + Edit resource + + +
+
+ + Title + {{#if this.titleErrorIsShown}} + + A title is required. + + {{/if}} + +
+ + URL + {{#unless this.urlIsValid}} + + A valid URL is required. + + {{/unless}} + + + {{! This adds enter-to-submit functionality }} + +
+ + + + + + {{#if this.removeResourceButtonIsShown}} + + {{/if}} + + + +{{/in-element}} diff --git a/web/app/components/related-resources/external-resource-form.ts b/web/app/components/related-resources/add-or-edit-external-resource-modal.ts similarity index 72% rename from web/app/components/related-resources/external-resource-form.ts rename to web/app/components/related-resources/add-or-edit-external-resource-modal.ts index 1c48edda2..dd9d15cc3 100644 --- a/web/app/components/related-resources/external-resource-form.ts +++ b/web/app/components/related-resources/add-or-edit-external-resource-modal.ts @@ -1,28 +1,33 @@ -import Component from "@glimmer/component"; import { action } from "@ember/object"; +import { guidFor } from "@ember/object/internals"; +import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; -import { assert } from "@ember/debug"; +import { RelatedExternalLink } from "hermes/components/related-resources"; import isValidURL from "hermes/utils/is-valid-u-r-l"; -import { RelatedExternalLink } from "../related-resources"; +import { assert } from "@ember/debug"; -interface RelatedResourcesExternalResourceFormComponentSignature { - Element: HTMLFormElement; +interface RelatedResourcesAddOrEditExternalResourceModalComponentSignature { Args: { resource?: RelatedExternalLink; + onClose: () => void; onSave: (resource: RelatedExternalLink) => void; + removeResource?: (resource: RelatedExternalLink) => void; }; Blocks: { default: []; - submit: [ - rr: { - submit: (e: Event) => void; - } - ]; }; } -export default class RelatedResourcesExternalResourceFormComponent extends Component { +export default class RelatedResourcesAddOrEditExternalResourceModalComponent extends Component { + protected id = guidFor(this); + protected bodyID = `${this.id}-body`; + protected submitID = `${this.id}-submit`; + + protected bodySelector = `#${this.bodyID}`; + protected submitSelector = `#${this.submitID}`; + @tracked protected shouldValidateEagerly = false; + /** * A locally tracked URL property. Starts as the passed-in value; * updated when the URL input-value changes. @@ -65,6 +70,20 @@ export default class RelatedResourcesExternalResourceFormComponent extends Compo return this._form; } + protected get removeResourceButtonIsShown() { + return !!this.args.removeResource && !!this.args.resource; + } + + protected get removeResource() { + assert("this.args.removeResource must exist", this.args.removeResource); + return this.args.removeResource; + } + + protected get resource() { + assert("this.args.resource must exist", this.args.resource); + return this.args.resource; + } + /** * The action to register the form element locally. * Called when the form is rendered. @@ -129,6 +148,6 @@ export default class RelatedResourcesExternalResourceFormComponent extends Compo declare module "@glint/environment-ember-loose/registry" { export default interface Registry { - "RelatedResources::ExternalResourceForm": typeof RelatedResourcesExternalResourceFormComponent; + "RelatedResources::AddOrEditExternalResourceModal": typeof RelatedResourcesAddOrEditExternalResourceModalComponent; } } diff --git a/web/app/components/related-resources/add.hbs b/web/app/components/related-resources/add.hbs index 96d3b7e93..d703fb224 100644 --- a/web/app/components/related-resources/add.hbs +++ b/web/app/components/related-resources/add.hbs @@ -1,19 +1,20 @@ {{#in-element (html-element ".ember-application") insertBefore=null}} - - - {{@headerTitle}} - - - {{#if this.scopeIsExternalLinks}} -
- {{! Positioned via in-element }} -
- {{else}} + {{#if this.scopeIsExternalLinks}} + + {{else}} + + + {{@headerTitle}} + + {{/if}} {{#if this.externalResourceFormIsShown}} - - {{/if}} - - {{#if this.scopeIsExternalLinks}} - - - - {{! Positioned via in-element }} - - - - - {{/if}} - -{{/in-element}} - -{{! TODO: explain this }} -{{#if this.scopeIsExternalLinks}} - {{#if this.externalLinkFormIsRendered}} - {{#in-element - (html-element this.externalLinkFormBodySelector) - insertBefore=null - }} - - <:submit as |rr|> - {{#in-element - (html-element this.externalLinkFormSubmitSelector) - insertBefore=null - }} - - {{/in-element}} - - - {{/in-element}} - {{else}} -
+ + {{/if}} -{{/if}} +{{/in-element}} diff --git a/web/app/components/related-resources/add.ts b/web/app/components/related-resources/add.ts index bab9358bc..7907fcdd4 100644 --- a/web/app/components/related-resources/add.ts +++ b/web/app/components/related-resources/add.ts @@ -320,7 +320,7 @@ export default class RelatedResourcesAddComponent extends Component
- {{! TODO: Combine with `ListItem::Edit` }}
- { +export default class RelatedResourcesAddFallbackExternalResource extends Component { /** * Whether an error message should be shown. * True if the title is empty or the URL is a duplicate. @@ -30,6 +30,6 @@ export default class RelatedResourcesAddExternalResource extends Component -
- - Title - {{#if this.titleErrorIsShown}} - - A title is required. - - {{/if}} - -
- - URL - {{! TODO: make this more accompanying to External-scoped components }} - {{#unless this.urlIsValid}} - - A valid URL is required. - - {{/unless}} - - - {{! This adds enter-to-submit functionality }} - - - {{#if (has-block "submit")}} - {{yield (hash submit=this.maybeSubmit) to="submit"}} - {{/if}} - 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 79088d8e0..e61295630 100644 --- a/web/tests/integration/components/document/sidebar/related-resources-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources-test.ts @@ -27,26 +27,20 @@ const ERROR_BUTTON_SELECTOR = "[data-test-related-resources-error-button]"; const OVERFLOW_BUTTON_SELECTOR = ".related-resource-overflow-button"; const EDIT_BUTTON_SELECTOR = "[data-test-overflow-menu-action='edit']"; const REMOVE_BUTTON_SELECTOR = "[data-test-overflow-menu-action='remove']"; -const EDIT_MODAL_SELECTOR = "[data-test-edit-related-resource-modal]"; -const EDIT_MODAL_HEADER_SELECTOR = - "[data-test-edit-related-resource-modal-header]"; +const EDIT_MODAL_SELECTOR = "[data-test-add-or-edit-external-resource-modal]"; +const EDIT_MODAL_HEADER_SELECTOR = `${EDIT_MODAL_SELECTOR} [data-test-modal-header]`; +const EDIT_RESOURCE_SAVE_BUTTON_SELECTOR = `${EDIT_MODAL_SELECTOR} [data-test-save-button]`; +const ADD_EXTERNAL_RESOURCE_MODAL_DELETE_BUTTON_SELECTOR = `${EDIT_MODAL_SELECTOR} [data-test-delete-button]`; const EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR = ".external-resource-title-input"; const EDIT_RESOURCE_URL_INPUT_SELECTOR = "[data-test-external-resource-url-input]"; -const EDIT_RESOURCE_SAVE_BUTTON_SELECTOR = - "[data-test-edit-related-resource-modal-save-button]"; const ADD_RESOURCE_BUTTON_SELECTOR = ".sidebar-section-header-button"; const ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR = ".related-document-option"; const ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR = "[data-test-related-resources-search-input]"; -const NO_RESOURCES_FOUND_SELECTOR = "[data-test-no-related-resources-found]"; -const ADD_EXTERNAL_RESOURCE_FORM_SELECTOR = - "[data-test-add-external-resource-form]"; const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = "[data-test-add-external-resource-submit-button]"; -const ADD_EXTERNAL_RESOURCE_MODAL_DELETE_BUTTON_SELECTOR = - "[data-test-edit-related-resource-modal-delete-button]"; const EDIT_EXTERNAL_RESOURCE_ERROR_SELECTOR = "[data-test-external-resource-title-error]"; const RESOURCE_TITLE_SELECTOR = "[data-test-resource-title]"; @@ -167,7 +161,7 @@ module( ]); }); - test("it shows an error message when the related resources fail to load", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { + test("it shows an error when related resources fail to load", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { this.server.get("/documents/:document_id/related-resources", () => { return new Response(500, {}, {}); }); @@ -195,9 +189,7 @@ module( await click(ERROR_BUTTON_SELECTOR); - assert - .dom("[data-test-sidebar-related-resources-empty-list]") - .exists("the list is shown again"); + assert.dom(LIST_SELECTOR).exists("the list is shown again"); }); test("resources can be deleted (overflow menu)", async function (this: DocumentSidebarRelatedResourcesTestContext, assert) { @@ -250,6 +242,8 @@ module( await click(OVERFLOW_BUTTON_SELECTOR); await click(EDIT_BUTTON_SELECTOR); + await waitFor(EDIT_MODAL_SELECTOR); + await click(ADD_EXTERNAL_RESOURCE_MODAL_DELETE_BUTTON_SELECTOR); assert.dom(LIST_ITEM_SELECTOR).doesNotExist("the item is removed"); @@ -275,6 +269,8 @@ module( await click(OVERFLOW_BUTTON_SELECTOR); await click(EDIT_BUTTON_SELECTOR); + await waitFor(EDIT_MODAL_SELECTOR); + assert.dom(EDIT_MODAL_SELECTOR).exists("the edit modal is shown"); assert.dom(EDIT_MODAL_HEADER_SELECTOR).hasText("Edit resource"); diff --git a/web/tests/integration/components/document/sidebar/related-resources/list-item-test.ts b/web/tests/integration/components/document/sidebar/related-resources/list-item-test.ts index d499c2696..19648e93c 100644 --- a/web/tests/integration/components/document/sidebar/related-resources/list-item-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources/list-item-test.ts @@ -173,15 +173,15 @@ module( await click(editButton); - assert - .dom("[data-test-edit-related-resource-modal]") - .exists("edit modal is visible"); + const modalSelector = "[data-test-add-or-edit-external-resource-modal]"; + + assert.dom(modalSelector).exists("edit modal is visible"); assert .dom(OVERFLOW_MENU_SELECTOR) .doesNotExist("overflow menu is closed"); - await click("[data-test-edit-related-resource-modal-save-button]"); + await click(`${modalSelector} [data-test-save-button]`); assert.equal(count, 1, "edit button was clicked"); await click(OVERFLOW_BUTTON_SELECTOR); diff --git a/web/tests/integration/components/related-resources-test.ts b/web/tests/integration/components/related-resources-test.ts index 7306f0dea..c94e1ba7b 100644 --- a/web/tests/integration/components/related-resources-test.ts +++ b/web/tests/integration/components/related-resources-test.ts @@ -28,6 +28,8 @@ const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = const ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR = "[data-test-add-external-resource-error]"; const ADD_RESOURCE_MODAL_SELECTOR = "[data-test-add-related-resource-modal]"; +const EXTERNAL_RESOURCE_MODAL_SELECTOR = + "[data-test-add-or-edit-external-resource-modal]"; const CURRENT_DOMAIN_PROTOCOL = window.location.protocol + "//"; const CURRENT_DOMAIN = window.location.hostname; const CURRENT_PORT = window.location.port; @@ -260,8 +262,6 @@ module("Integration | Component | related-resources", function (hooks) { `); - assert.dom(".list").doesNotExist(); - await click("button"); await waitFor(ADD_RESOURCE_MODAL_SELECTOR); @@ -407,7 +407,7 @@ module("Integration | Component | related-resources", function (hooks) { await click("button"); - await waitFor(ADD_RESOURCE_MODAL_SELECTOR); + await waitFor(EXTERNAL_RESOURCE_MODAL_SELECTOR); assert.dom(RELATED_DOCUMENT_OPTION_SELECTOR).doesNotExist(); assert.dom("[data-test-external-resource-form]").exists(); @@ -419,7 +419,7 @@ module("Integration | Component | related-resources", function (hooks) { await click("[data-test-add-external-resource-button"); - assert.dom(ADD_RESOURCE_MODAL_SELECTOR).doesNotExist(); + assert.dom(EXTERNAL_RESOURCE_MODAL_SELECTOR).doesNotExist(); assert.dom(".item").hasText("Example - https://example.com"); }); diff --git a/web/tests/integration/components/related-resources/add/external-resource-test.ts b/web/tests/integration/components/related-resources/add/external-resource-test.ts index 382d93d8b..bc9bd0b64 100644 --- a/web/tests/integration/components/related-resources/add/external-resource-test.ts +++ b/web/tests/integration/components/related-resources/add/external-resource-test.ts @@ -18,7 +18,7 @@ module( this.set("onInput", () => {}); await render(hbs` - Date: Thu, 7 Sep 2023 16:04:27 -0400 Subject: [PATCH 11/15] Test selector cleanup --- .../add/fallback-external-resource.hbs | 13 ++++++---- .../sidebar/related-resources-test.ts | 3 +-- .../components/related-resources-test.ts | 25 +++++++++---------- .../add/external-resource-test.ts | 6 ++--- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/web/app/components/related-resources/add/fallback-external-resource.hbs b/web/app/components/related-resources/add/fallback-external-resource.hbs index 5aea2b67c..44b6c355d 100644 --- a/web/app/components/related-resources/add/fallback-external-resource.hbs +++ b/web/app/components/related-resources/add/fallback-external-resource.hbs @@ -1,6 +1,9 @@ -
+
-
+
-
+
{{@url}}
@@ -22,7 +25,7 @@
{{#if this.errorIsShown}} - + {{if @linkIsDuplicate "This resource has already been added." 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 e61295630..9f9db38de 100644 --- a/web/tests/integration/components/document/sidebar/related-resources-test.ts +++ b/web/tests/integration/components/document/sidebar/related-resources-test.ts @@ -39,8 +39,7 @@ const ADD_RELATED_RESOURCES_DOCUMENT_OPTION_SELECTOR = ".related-document-option"; const ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR = "[data-test-related-resources-search-input]"; -const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = - "[data-test-add-external-resource-submit-button]"; +const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = `[data-test-add-fallback-external-resource] [data-test-submit-button]`; const EDIT_EXTERNAL_RESOURCE_ERROR_SELECTOR = "[data-test-external-resource-title-error]"; const RESOURCE_TITLE_SELECTOR = "[data-test-resource-title]"; diff --git a/web/tests/integration/components/related-resources-test.ts b/web/tests/integration/components/related-resources-test.ts index c94e1ba7b..3f480be5d 100644 --- a/web/tests/integration/components/related-resources-test.ts +++ b/web/tests/integration/components/related-resources-test.ts @@ -20,16 +20,15 @@ const RELATED_DOCUMENT_OPTION_SELECTOR = ".related-document-option"; const ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR = "[data-test-related-resources-search-input]"; const NO_RESOURCES_FOUND_SELECTOR = "[data-test-no-related-resources-found]"; -const ADD_EXTERNAL_RESOURCE_FORM_SELECTOR = - "[data-test-add-external-resource-form]"; +const ADD_FALLBACK_EXTERNAL_RESOURCE_SELECTOR = + "[data-test-add-fallback-external-resource]"; const EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR = ".external-resource-title-input"; -const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = - "[data-test-add-external-resource-submit-button]"; -const ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR = - "[data-test-add-external-resource-error]"; +const ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR = `${ADD_FALLBACK_EXTERNAL_RESOURCE_SELECTOR} [data-test-error]`; const ADD_RESOURCE_MODAL_SELECTOR = "[data-test-add-related-resource-modal]"; const EXTERNAL_RESOURCE_MODAL_SELECTOR = "[data-test-add-or-edit-external-resource-modal]"; +const ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = `${EXTERNAL_RESOURCE_MODAL_SELECTOR} [data-test-submit-button]`; +const ADD_FALLBACK_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR = `${ADD_FALLBACK_EXTERNAL_RESOURCE_SELECTOR} [data-test-submit-button]`; const CURRENT_DOMAIN_PROTOCOL = window.location.protocol + "//"; const CURRENT_DOMAIN = window.location.hostname; const CURRENT_PORT = window.location.port; @@ -276,7 +275,7 @@ module("Integration | Component | related-resources", function (hooks) { assert .dom(RELATED_DOCUMENT_OPTION_SELECTOR) .doesNotExist("documents are removed when a valid URL is entered"); - assert.dom(ADD_EXTERNAL_RESOURCE_FORM_SELECTOR).exists(); + assert.dom(ADD_FALLBACK_EXTERNAL_RESOURCE_SELECTOR).exists(); assert .dom(EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR) @@ -284,7 +283,7 @@ module("Integration | Component | related-resources", function (hooks) { // Try to add a resource without a title - await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + await click(ADD_FALLBACK_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); // Confirm that it fails @@ -295,7 +294,7 @@ module("Integration | Component | related-resources", function (hooks) { // Now add a a title await fillIn(EXTERNAL_RESOURCE_TITLE_INPUT_SELECTOR, "Example"); - await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + await click(ADD_FALLBACK_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); assert.dom(ADD_RESOURCE_MODAL_SELECTOR).doesNotExist("the modal is closed"); assert.dom(".item").hasText("Example"); @@ -338,7 +337,7 @@ module("Integration | Component | related-resources", function (hooks) { .dom(ADD_EXTERNAL_RESOURCE_ERROR_SELECTOR) .hasText("This resource has already been added."); - await click(ADD_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); + await click(ADD_FALLBACK_EXTERNAL_RESOURCE_SUBMIT_BUTTON_SELECTOR); assert .dom(ADD_RESOURCE_MODAL_SELECTOR) @@ -417,7 +416,7 @@ module("Integration | Component | related-resources", function (hooks) { "https://example.com" ); - await click("[data-test-add-external-resource-button"); + await click("[data-test-save-button]"); assert.dom(EXTERNAL_RESOURCE_MODAL_SELECTOR).doesNotExist(); assert.dom(".item").hasText("Example - https://example.com"); @@ -542,7 +541,7 @@ module("Integration | Component | related-resources", function (hooks) { await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, documentURL); assert - .dom(ADD_EXTERNAL_RESOURCE_FORM_SELECTOR) + .dom(ADD_FALLBACK_EXTERNAL_RESOURCE_SELECTOR) .exists('the "add resource" form is shown'); }); @@ -566,7 +565,7 @@ module("Integration | Component | related-resources", function (hooks) { await fillIn(ADD_RELATED_RESOURCES_SEARCH_INPUT_SELECTOR, shortLink); assert - .dom(ADD_EXTERNAL_RESOURCE_FORM_SELECTOR) + .dom(ADD_FALLBACK_EXTERNAL_RESOURCE_SELECTOR) .exists('the "add resource" form is shown'); }); diff --git a/web/tests/integration/components/related-resources/add/external-resource-test.ts b/web/tests/integration/components/related-resources/add/external-resource-test.ts index bc9bd0b64..c7321e459 100644 --- a/web/tests/integration/components/related-resources/add/external-resource-test.ts +++ b/web/tests/integration/components/related-resources/add/external-resource-test.ts @@ -27,14 +27,12 @@ module( `); assert - .dom("[data-test-add-external-resource-form]") + .dom("[data-test-add-fallback-external-resource]") .exists("shows the form when the URL is not loading"); assert.dom(".external-resource-title-input").hasValue("Test"); - assert - .dom("[data-test-add-external-resource-truncated-url]") - .hasText("https://example.com"); + assert.dom("[data-test-url]").hasText("https://example.com"); }); } ); From e8558fc73b5aaa9ee68c2185e29deab4b88c72b5 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Thu, 7 Sep 2023 16:12:50 -0400 Subject: [PATCH 12/15] Cleanup --- .../add-or-edit-external-resource-modal.ts | 8 -------- web/app/components/related-resources/add.ts | 12 ------------ .../add/fallback-external-resource.ts | 2 +- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/web/app/components/related-resources/add-or-edit-external-resource-modal.ts b/web/app/components/related-resources/add-or-edit-external-resource-modal.ts index dd9d15cc3..f77ca3bf5 100644 --- a/web/app/components/related-resources/add-or-edit-external-resource-modal.ts +++ b/web/app/components/related-resources/add-or-edit-external-resource-modal.ts @@ -1,5 +1,4 @@ import { action } from "@ember/object"; -import { guidFor } from "@ember/object/internals"; import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; import { RelatedExternalLink } from "hermes/components/related-resources"; @@ -19,13 +18,6 @@ interface RelatedResourcesAddOrEditExternalResourceModalComponentSignature { } export default class RelatedResourcesAddOrEditExternalResourceModalComponent extends Component { - protected id = guidFor(this); - protected bodyID = `${this.id}-body`; - protected submitID = `${this.id}-submit`; - - protected bodySelector = `#${this.bodyID}`; - protected submitSelector = `#${this.submitID}`; - @tracked protected shouldValidateEagerly = false; /** diff --git a/web/app/components/related-resources/add.ts b/web/app/components/related-resources/add.ts index 7907fcdd4..65beb3365 100644 --- a/web/app/components/related-resources/add.ts +++ b/web/app/components/related-resources/add.ts @@ -151,11 +151,6 @@ export default class RelatedResourcesAddComponent extends Component Date: Thu, 7 Sep 2023 17:01:05 -0400 Subject: [PATCH 13/15] Remove comment --- web/app/components/related-resources/add.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/web/app/components/related-resources/add.ts b/web/app/components/related-resources/add.ts index 65beb3365..809421b57 100644 --- a/web/app/components/related-resources/add.ts +++ b/web/app/components/related-resources/add.ts @@ -36,7 +36,6 @@ interface RelatedResourcesAddComponentSignature { options?: SearchOptions ) => Promise; getObject: (dd: XDropdownListAnchorAPI | null, id: string) => Promise; - // TODO: remove headerTitle: string; inputPlaceholder: string; searchErrorIsShown?: boolean; From 925d7ed5c4dc887b117ab17d9a1d8291aa5ba38d Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Thu, 7 Sep 2023 21:03:46 -0400 Subject: [PATCH 14/15] Improve animation --- .../document/sidebar/related-resources.hbs | 30 +++++++++---------- .../document/sidebar/related-resources.ts | 9 +++++- .../sidebar/related-resources/list.hbs | 2 ++ .../sidebar/related-resources/list.ts | 11 ------- .../add-or-edit-external-resource-modal.ts | 4 +-- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources.hbs b/web/app/components/document/sidebar/related-resources.hbs index e3ab3a493..5f6f2b9f2 100644 --- a/web/app/components/document/sidebar/related-resources.hbs +++ b/web/app/components/document/sidebar/related-resources.hbs @@ -25,21 +25,6 @@ />
- <:list-error> -
-
- Failed to load -
- -
- <:list as |rr|> + <:list-error> +
+
+ Failed to load +
+ +
+ diff --git a/web/app/components/document/sidebar/related-resources.ts b/web/app/components/document/sidebar/related-resources.ts index 55e6022ce..23dab799d 100644 --- a/web/app/components/document/sidebar/related-resources.ts +++ b/web/app/components/document/sidebar/related-resources.ts @@ -16,6 +16,7 @@ import { RelatedHermesDocument, RelatedResource, } from "hermes/components/related-resources"; +import { assert } from "@ember/debug"; enum RelatedResourceSelector { ExternalLink = ".external-resource", @@ -146,7 +147,13 @@ export default class DocumentSidebarRelatedResourcesComponent extends Component< ); if (resourceIndex !== -1) { - this.relatedLinks[resourceIndex] = resource; + const linkBeingEdited = this.relatedLinks[resourceIndex]; + assert("linkBeingEdited must exist", linkBeingEdited); + + // We replace the values rather than the object itself. + // This helps Ember Animated recognize the edited sprite as `kept`. + linkBeingEdited.url = resource.url; + linkBeingEdited.name = resource.name; // The getter doesn't update when a new resource is added, so we manually save it. // TODO: Improve this diff --git a/web/app/components/document/sidebar/related-resources/list.hbs b/web/app/components/document/sidebar/related-resources/list.hbs index b9b24fac4..277adc19d 100644 --- a/web/app/components/document/sidebar/related-resources/list.hbs +++ b/web/app/components/document/sidebar/related-resources/list.hbs @@ -6,6 +6,8 @@
{{else}}
    + {{! This is being looped through twice for some reason... }} + {{! On immediate save, the order is wrong - maybe an array push thing? }} {{#animated-each @items rules=this.transitionRules diff --git a/web/app/components/document/sidebar/related-resources/list.ts b/web/app/components/document/sidebar/related-resources/list.ts index cff47067f..2048ed328 100644 --- a/web/app/components/document/sidebar/related-resources/list.ts +++ b/web/app/components/document/sidebar/related-resources/list.ts @@ -71,17 +71,6 @@ export default class DocumentSidebarRelatedResourcesListComponent extends Compon return; } - if (insertedSprites.length === 1 && removedSprites.length === 1) { - /** - * For unknown reasons, Ember Animated sees an updated sprite - * as both inserted and removed, causing unwanted animations. - * This check prevents that from happening. - * - * FIXME: Investigate and fix this. - */ - return; - } - for (let sprite of keptSprites) { void move(sprite, { duration: 250, easing: easeOutQuad }); } diff --git a/web/app/components/related-resources/add-or-edit-external-resource-modal.ts b/web/app/components/related-resources/add-or-edit-external-resource-modal.ts index f77ca3bf5..8ed244eed 100644 --- a/web/app/components/related-resources/add-or-edit-external-resource-modal.ts +++ b/web/app/components/related-resources/add-or-edit-external-resource-modal.ts @@ -130,9 +130,9 @@ export default class RelatedResourcesAddOrEditExternalResourceModalComponent ext const sortOrder = this.args.resource ? this.args.resource.sortOrder : 1; this.args.onSave({ - sortOrder, - name: this.title, url: this.url, + name: this.title, + sortOrder, }); } } From f399aca4b8960204f51a26649d72bc1d03a751a9 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 26 Sep 2023 17:11:44 -0400 Subject: [PATCH 15/15] Remove unnecessary comment --- web/app/components/document/sidebar/related-resources/list.hbs | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/app/components/document/sidebar/related-resources/list.hbs b/web/app/components/document/sidebar/related-resources/list.hbs index 277adc19d..b9b24fac4 100644 --- a/web/app/components/document/sidebar/related-resources/list.hbs +++ b/web/app/components/document/sidebar/related-resources/list.hbs @@ -6,8 +6,6 @@
{{else}}
    - {{! This is being looped through twice for some reason... }} - {{! On immediate save, the order is wrong - maybe an array push thing? }} {{#animated-each @items rules=this.transitionRules