From 3623f0d399f9aff221e2681e80badd3a71335823 Mon Sep 17 00:00:00 2001 From: minnakt Date: Thu, 18 Jan 2024 09:51:40 -0500 Subject: [PATCH 1/4] DEVPROD-834: Allow defining multiple metadata links --- .../integration/projectSettings/plugins.ts | 67 +++++++++++----- .../SpruceForm/Widgets/MultiSelect.tsx | 2 +- .../tabs/PluginsTab/PluginsTab.tsx | 42 +++++----- .../tabs/PluginsTab/getFormSchema.tsx | 78 ++++++++++--------- .../tabs/PluginsTab/transformers.test.ts | 44 ++++++++--- .../tabs/PluginsTab/transformers.ts | 20 +++-- .../projectSettings/tabs/PluginsTab/types.ts | 13 ++-- src/pages/projectSettings/tabs/testData.ts | 14 +++- src/pages/version/Metadata.tsx | 7 +- 9 files changed, 179 insertions(+), 108 deletions(-) diff --git a/cypress/integration/projectSettings/plugins.ts b/cypress/integration/projectSettings/plugins.ts index 3ebee64567..6bfeffee0d 100644 --- a/cypress/integration/projectSettings/plugins.ts +++ b/cypress/integration/projectSettings/plugins.ts @@ -3,37 +3,62 @@ import { clickSave } from "../../utils"; describe("Plugins", () => { const patchPage = "version/5ecedafb562343215a7ff297"; - it("Should set an external link to render on patch metadata panel and then unset it to revert the changes", () => { - // Set the external link - cy.visit(getPluginsRoute(projectUseRepoEnabled)); - cy.dataCy("requesters-input").click(); - cy.getInputByLabel("Commits").check({ force: true }); + + const addMetadataLink = (metadataLink: { + displayName: string; + url: string; + }) => { + cy.contains("button", "Add metadata link").scrollIntoView(); + cy.contains("button", "Add metadata link").click(); + cy.dataCy("requesters-input").first().click(); cy.getInputByLabel("Patches").check({ force: true }); - cy.dataCy("requesters-input").click(); - cy.dataCy("display-name-input").type("An external link"); - cy.dataCy("url-template-input").type("https://example.com/{version_id}", { + cy.dataCy("requesters-input").first().click(); + cy.dataCy("display-name-input").first().type(metadataLink.displayName); + cy.dataCy("url-template-input").first().type(metadataLink.url, { parseSpecialCharSequences: false, }); + }; + + it("Should be able to set external links to render on patch metadata panel", () => { + // Add external links. + cy.visit(getPluginsRoute(projectUseRepoEnabled)); + addMetadataLink({ + displayName: "An external link 1", + url: "https://example-1.com/{version_id}", + }); + addMetadataLink({ + displayName: "An external link 2", + url: "https://example-2.com/{version_id}", + }); cy.dataCy("save-settings-button").scrollIntoView(); clickSave(); + cy.visit(patchPage); - cy.dataCy("external-link").contains("An external link"); - cy.dataCy("external-link").should( - "have.attr", - "href", - "https://example.com/5ecedafb562343215a7ff297", - ); + cy.dataCy("external-link").should("have.length", 2); + cy.dataCy("external-link").last().contains("An external link 1"); + cy.dataCy("external-link") + .last() + .should( + "have.attr", + "href", + "https://example-1.com/5ecedafb562343215a7ff297", + ); + cy.dataCy("external-link").first().contains("An external link 2"); + cy.dataCy("external-link") + .first() + .should( + "have.attr", + "href", + "https://example-2.com/5ecedafb562343215a7ff297", + ); - // Unset the external link + // Remove external links. cy.visit(getPluginsRoute(projectUseRepoEnabled)); - cy.dataCy("requesters-input").click(); - cy.getInputByLabel("Commits").uncheck({ force: true }); - cy.getInputByLabel("Patches").uncheck({ force: true }); - cy.dataCy("requesters-input").click(); - cy.dataCy("display-name-input").clear(); - cy.dataCy("url-template-input").clear(); + cy.dataCy("delete-item-button").first().click(); + cy.dataCy("delete-item-button").first().click(); cy.dataCy("save-settings-button").scrollIntoView(); clickSave(); + cy.visit(patchPage); cy.dataCy("external-link").should("not.exist"); }); diff --git a/src/components/SpruceForm/Widgets/MultiSelect.tsx b/src/components/SpruceForm/Widgets/MultiSelect.tsx index ad4c1a5ac0..149f008030 100644 --- a/src/components/SpruceForm/Widgets/MultiSelect.tsx +++ b/src/components/SpruceForm/Widgets/MultiSelect.tsx @@ -56,7 +56,7 @@ export const MultiSelect: React.FC = ({ hasStyling={false} /> - {rawErrors.length > 0 && {rawErrors.join(", ")}} + {rawErrors?.length > 0 && {rawErrors?.join(", ")}} ); diff --git a/src/pages/projectSettings/tabs/PluginsTab/PluginsTab.tsx b/src/pages/projectSettings/tabs/PluginsTab/PluginsTab.tsx index 3cbd5097ad..e2d24fbec5 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/PluginsTab.tsx +++ b/src/pages/projectSettings/tabs/PluginsTab/PluginsTab.tsx @@ -41,7 +41,7 @@ export const PluginsTab: React.FC = ({ const validate = ((formData, errors) => { const { buildBaronSettings: { ticketSearchProjects }, - externalLinks: { metadataPanelLink }, + externalLinks, } = formData; // if a search project is defined, a create project must be defined, and vice versa @@ -63,27 +63,29 @@ const validate = ((formData, errors) => { ); } - const displayNameDefined = metadataPanelLink.displayName.trim() !== ""; - const urlTemplateDefined = metadataPanelLink.urlTemplate.trim() !== ""; - const requestersDefined = metadataPanelLink.requesters.length > 0; + externalLinks.forEach((link, idx) => { + const displayNameDefined = link.displayName.trim() !== ""; + const urlTemplateDefined = link.urlTemplate.trim() !== ""; + const requestersDefined = link.requesters.length > 0; - if (displayNameDefined || urlTemplateDefined || requestersDefined) { - if (!displayNameDefined) { - errors.externalLinks.metadataPanelLink.displayName.addError( - "You must specify a display name.", - ); + if (displayNameDefined || urlTemplateDefined || requestersDefined) { + if (!displayNameDefined) { + errors.externalLinks[idx].displayName.addError( + "You must specify a display name.", + ); + } + if (!urlTemplateDefined) { + errors.externalLinks[idx].urlTemplate.addError( + "You must specify a URL template.", + ); + } + if (!requestersDefined) { + errors.externalLinks[idx].requesters.addError( + "You must specify requesters.", + ); + } } - if (!urlTemplateDefined) { - errors.externalLinks.metadataPanelLink.urlTemplate.addError( - "You must specify a URL template.", - ); - } - if (!requestersDefined) { - errors.externalLinks.metadataPanelLink.requesters.addError( - "You must specify requesters.", - ); - } - } + }); return errors; }) satisfies ValidateProps; diff --git a/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx b/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx index 0a4b09fa42..e629dcea60 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx +++ b/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx @@ -178,38 +178,38 @@ export const getFormSchema = ( }, }, externalLinks: { - type: "object" as "object", - title: "Metadata Link", - properties: { - metadataPanelLink: { - type: "object" as "object", - title: "", - description: - "Add a URL to the metadata panel for versions with the specified requester. Include {version_id} in the URL template and it will be replaced by an actual version ID.", - properties: { - requesters: { - type: "array" as "array", - title: "Requesters", - uniqueItems: true, - items: { - type: "string" as "string", - anyOf: requesters.map((r) => ({ - type: "string" as "string", - title: r.label, - enum: [r.value], - })), - }, - }, - displayName: { - type: "string" as "string", - title: "Display name", - maxLength: 40, - }, - urlTemplate: { + type: "array" as "array", + title: "Metadata Links", + items: { + type: "object" as "object", + properties: { + requesters: { + type: "array" as "array", + title: "Requesters", + uniqueItems: true, + items: { type: "string" as "string", - title: "URL template", - format: "validURLTemplate", + anyOf: requesters.map((r) => ({ + type: "string" as "string", + title: r.label, + enum: [r.value], + })), }, + default: [], + }, + displayName: { + type: "string" as "string", + title: "Display name", + default: "", + minLength: 1, + maxLength: 40, + }, + urlTemplate: { + type: "string" as "string", + title: "URL template", + default: "", + minLength: 1, + format: "validURLTemplate", }, }, }, @@ -292,18 +292,26 @@ export const getFormSchema = ( }, externalLinks: { "ui:rootFieldId": "externalLinks", - "ui:ObjectFieldTemplate": CardFieldTemplate, - metadataPanelLink: { + "ui:placeholder": "No metadata links are defined.", + "ui:description": + "Add URLs to the metadata panel for versions with the specified requester.", + "ui:addButtonText": "Add metadata link", + "ui:orderable": false, + "ui:useExpandableCard": true, + items: { + "ui:displayTitle": "New Metadata Link", requesters: { "ui:widget": widgets.MultiSelectWidget, "ui:data-cy": "requesters-input", }, + displayName: { + "ui:data-cy": "display-name-input", + }, urlTemplate: { "ui:placeholder": "https://example.com/{version_id}", "ui:data-cy": "url-template-input", - }, - displayName: { - "ui:data-cy": "display-name-input", + "ui:description": + "Include {version_id} in the URL template and it will be replaced by an actual version ID.", }, }, }, diff --git a/src/pages/projectSettings/tabs/PluginsTab/transformers.test.ts b/src/pages/projectSettings/tabs/PluginsTab/transformers.test.ts index 7ec2f97134..5b6b35dfaf 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/transformers.test.ts +++ b/src/pages/projectSettings/tabs/PluginsTab/transformers.test.ts @@ -47,13 +47,20 @@ const projectForm: PluginsFormState = { secret: null, }, }, - externalLinks: { - metadataPanelLink: { + externalLinks: [ + { requesters: ["gitter_request", "patch_request"], displayName: "a link display name", - urlTemplate: "https:/a-link-template-{version_id}.com", + displayTitle: "a link display name", + urlTemplate: "https://a-link-template-{version_id}.com", }, - }, + { + requesters: ["ad_hoc"], + displayName: "periodic build link", + displayTitle: "periodic build link", + urlTemplate: "https://periodic-build-{version_id}.com", + }, + ], }; const projectResult: Pick = { @@ -71,7 +78,12 @@ const projectResult: Pick = { { requesters: ["gitter_request", "patch_request"], displayName: "a link display name", - urlTemplate: "https:/a-link-template-{version_id}.com", + urlTemplate: "https://a-link-template-{version_id}.com", + }, + { + requesters: ["ad_hoc"], + displayName: "periodic build link", + urlTemplate: "https://periodic-build-{version_id}.com", }, ], }, @@ -107,13 +119,20 @@ const repoForm: PluginsFormState = { secret: "secret", }, }, - externalLinks: { - metadataPanelLink: { + externalLinks: [ + { requesters: ["gitter_request", "patch_request"], displayName: "a link display name", - urlTemplate: "https:/a-link-template-{version_id}.com", + displayTitle: "a link display name", + urlTemplate: "https://a-link-template-{version_id}.com", }, - }, + { + requesters: ["ad_hoc"], + displayName: "periodic build link", + displayTitle: "periodic build link", + urlTemplate: "https://periodic-build-{version_id}.com", + }, + ], }; const repoResult: Pick = { @@ -136,7 +155,12 @@ const repoResult: Pick = { { requesters: ["gitter_request", "patch_request"], displayName: "a link display name", - urlTemplate: "https:/a-link-template-{version_id}.com", + urlTemplate: "https://a-link-template-{version_id}.com", + }, + { + requesters: ["ad_hoc"], + displayName: "periodic build link", + urlTemplate: "https://periodic-build-{version_id}.com", }, ], }, diff --git a/src/pages/projectSettings/tabs/PluginsTab/transformers.ts b/src/pages/projectSettings/tabs/PluginsTab/transformers.ts index 4c5f9fb3d8..06e73c5e7c 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/transformers.ts +++ b/src/pages/projectSettings/tabs/PluginsTab/transformers.ts @@ -44,13 +44,11 @@ export const gqlToForm = ((data) => { secret: projectRef?.taskAnnotationSettings?.fileTicketWebhook?.secret, }, }, - externalLinks: { - metadataPanelLink: { - requesters: projectRef?.externalLinks?.[0].requesters ?? [], - displayName: projectRef?.externalLinks?.[0].displayName ?? "", - urlTemplate: projectRef?.externalLinks?.[0].urlTemplate ?? "", - }, - }, + externalLinks: + projectRef?.externalLinks?.map((e) => ({ + ...e, + displayTitle: e.displayName, + })) ?? [], }; }) satisfies GqlToFormFunction; @@ -72,7 +70,13 @@ export const formToGql = (( .map(({ displayText, field }) => ({ field, displayText })) .filter((str) => !!str), }, - externalLinks: [externalLinks.metadataPanelLink], + externalLinks: externalLinks.map( + ({ displayName, requesters, urlTemplate }) => ({ + requesters, + displayName, + urlTemplate, + }), + ), }; return { projectRef }; diff --git a/src/pages/projectSettings/tabs/PluginsTab/types.ts b/src/pages/projectSettings/tabs/PluginsTab/types.ts index 5059c03bc3..8cd3971fea 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/types.ts +++ b/src/pages/projectSettings/tabs/PluginsTab/types.ts @@ -24,13 +24,12 @@ export interface PluginsFormState { secret: string; }; }; - externalLinks: { - metadataPanelLink: { - requesters: string[]; - displayName: string; - urlTemplate: string; - }; - }; + externalLinks: Array<{ + displayTitle: string; + requesters: string[]; + displayName: string; + urlTemplate: string; + }>; } export type TabProps = { diff --git a/src/pages/projectSettings/tabs/testData.ts b/src/pages/projectSettings/tabs/testData.ts index 9621b17e2b..386f221cb6 100644 --- a/src/pages/projectSettings/tabs/testData.ts +++ b/src/pages/projectSettings/tabs/testData.ts @@ -14,7 +14,12 @@ const projectBase: ProjectSettingsQuery["projectSettings"] = { { requesters: ["gitter_request", "patch_request"], displayName: "a link display name", - urlTemplate: "https:/a-link-template-{version_id}.com", + urlTemplate: "https://a-link-template-{version_id}.com", + }, + { + requesters: ["ad_hoc"], + displayName: "periodic build link", + urlTemplate: "https://periodic-build-{version_id}.com", }, ], banner: { @@ -158,7 +163,12 @@ const repoBase: RepoSettingsQuery["repoSettings"] = { { requesters: ["gitter_request", "patch_request"], displayName: "a link display name", - urlTemplate: "https:/a-link-template-{version_id}.com", + urlTemplate: "https://a-link-template-{version_id}.com", + }, + { + requesters: ["ad_hoc"], + displayName: "periodic build link", + urlTemplate: "https://periodic-build-{version_id}.com", }, ], id: "123", diff --git a/src/pages/version/Metadata.tsx b/src/pages/version/Metadata.tsx index e9d86855d7..ea3d930bd3 100644 --- a/src/pages/version/Metadata.tsx +++ b/src/pages/version/Metadata.tsx @@ -67,7 +67,6 @@ export const Metadata: React.FC = ({ loading, version }) => { } = upstreamProject || {}; const { owner, repo } = projectMetadata || {}; - const { displayName, url } = externalLinksForMetadata?.[0] || {}; return ( @@ -195,13 +194,13 @@ export const Metadata: React.FC = ({ loading, version }) => { )} - {url && displayName && ( - + {externalLinksForMetadata?.map(({ displayName, url }) => ( + {displayName} - )} + ))} {gitTags && ( {gitTags.map((g) => ( From 24b5b1adc4d4281e320721e33bc9df79fb0d8830 Mon Sep 17 00:00:00 2001 From: minnakt Date: Thu, 18 Jan 2024 09:59:23 -0500 Subject: [PATCH 2/4] Add max items --- src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx b/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx index e629dcea60..d1bc7c337f 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx +++ b/src/pages/projectSettings/tabs/PluginsTab/getFormSchema.tsx @@ -180,6 +180,7 @@ export const getFormSchema = ( externalLinks: { type: "array" as "array", title: "Metadata Links", + maxItems: 5, items: { type: "object" as "object", properties: { From 971efd058b9663d45dab3cc52b68a462ca35e684 Mon Sep 17 00:00:00 2001 From: minnakt Date: Thu, 18 Jan 2024 15:27:24 -0500 Subject: [PATCH 3/4] Ensure better backwards compatibility Old format requires null --- .../tabs/PluginsTab/transformers.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/pages/projectSettings/tabs/PluginsTab/transformers.ts b/src/pages/projectSettings/tabs/PluginsTab/transformers.ts index 06e73c5e7c..b1aa4214e5 100644 --- a/src/pages/projectSettings/tabs/PluginsTab/transformers.ts +++ b/src/pages/projectSettings/tabs/PluginsTab/transformers.ts @@ -70,15 +70,15 @@ export const formToGql = (( .map(({ displayText, field }) => ({ field, displayText })) .filter((str) => !!str), }, - externalLinks: externalLinks.map( - ({ displayName, requesters, urlTemplate }) => ({ - requesters, - displayName, - urlTemplate, - }), - ), + externalLinks: + externalLinks.length > 0 + ? externalLinks.map(({ displayName, requesters, urlTemplate }) => ({ + requesters, + displayName, + urlTemplate, + })) + : null, }; - return { projectRef }; }) satisfies FormToGqlFunction; From 82b57007a06befda305b222623b2a6c0332ce9ff Mon Sep 17 00:00:00 2001 From: minnakt Date: Thu, 18 Jan 2024 16:06:27 -0500 Subject: [PATCH 4/4] Fix lint --- cypress/integration/task/test_table.ts | 2 +- src/pages/task/taskTabs/TestsTable.tsx | 8 ++++---- src/types/task.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cypress/integration/task/test_table.ts b/cypress/integration/task/test_table.ts index 3589f9fa91..4cc5168a33 100644 --- a/cypress/integration/task/test_table.ts +++ b/cypress/integration/task/test_table.ts @@ -106,7 +106,7 @@ describe("Tests Table", () => { }); cy.location().should((loc) => { expect(decodeURIComponent(loc.search)).to.include( - `statuses=${statuses.map(({ key }) => key).join(",")}` + `statuses=${statuses.map(({ key }) => key).join(",")}`, ); }); }); diff --git a/src/pages/task/taskTabs/TestsTable.tsx b/src/pages/task/taskTabs/TestsTable.tsx index 24c66191a5..76785ab09d 100644 --- a/src/pages/task/taskTabs/TestsTable.tsx +++ b/src/pages/task/taskTabs/TestsTable.tsx @@ -117,7 +117,7 @@ export const TestsTable: React.FC = ({ task }) => { const { initialFilters, initialSorting } = useMemo( () => getInitialState(queryParams), - [] // eslint-disable-line react-hooks/exhaustive-deps + [], // eslint-disable-line react-hooks/exhaustive-deps ); const setSorting = (s: SortingState) => @@ -150,11 +150,11 @@ export const TestsTable: React.FC = ({ task }) => { manualPagination: true, onColumnFiltersChange: onChangeHandler( setFilters, - updateFilters + updateFilters, ), onSortingChange: onChangeHandler( setSorting, - tableSortHandler + tableSortHandler, ), }); @@ -215,7 +215,7 @@ const getInitialState = (queryParams: { } return accum; }, - [] + [], ), }; }; diff --git a/src/types/task.ts b/src/types/task.ts index b28d91308a..52500fa81e 100644 --- a/src/types/task.ts +++ b/src/types/task.ts @@ -35,7 +35,7 @@ export const mapIdToFilterParam = Object.entries(mapFilterParamToId).reduce( ...accum, [param]: id, }), - {} + {}, ); export type TableOnChange = TableProps["onChange"];