diff --git a/packages/api/src/feedback.ts b/packages/api/src/feedback.ts index 15daf25c28e..4f8b671959f 100644 --- a/packages/api/src/feedback.ts +++ b/packages/api/src/feedback.ts @@ -18,8 +18,9 @@ export type DirectFeedbackCategory = 'developers' | 'design'; export type GitHubFeedbackCategory = 'feature' | 'bug'; +export type OtherFeedbackCategory = 'other'; -export type FeedbackCategory = DirectFeedbackCategory | GitHubFeedbackCategory; +export type FeedbackCategory = DirectFeedbackCategory | GitHubFeedbackCategory | OtherFeedbackCategory; export interface FeedbackProperties { category: FeedbackCategory; @@ -29,7 +30,7 @@ export interface FeedbackProperties { } export interface GitHubIssue { - category: FeedbackCategory; + category: GitHubFeedbackCategory; title: string; description: string; includeSystemInfo?: boolean; diff --git a/packages/main/src/plugin/feedback-handler.spec.ts b/packages/main/src/plugin/feedback-handler.spec.ts index a1e84beeab3..4d666d6e59c 100644 --- a/packages/main/src/plugin/feedback-handler.spec.ts +++ b/packages/main/src/plugin/feedback-handler.spec.ts @@ -126,6 +126,9 @@ describe('openGitHubIssue', () => { await feedbackHandler.openGitHubIssue(issueProperties); expect(shell.openExternal).toHaveBeenCalledOnce(); + expect(shell.openExternal).toHaveBeenCalledWith( + expect.stringContaining(productJSONFile.GitHubFeedbackLinks.issues), + ); // extract the first argument of the shell.openExternal call const url: string | undefined = vi.mocked(shell.openExternal).mock.calls[0]?.[0]; @@ -136,6 +139,30 @@ describe('openGitHubIssue', () => { }); }); + test('Expect log in the console is there is no GitHub issues link for preview', async () => { + const originalConsoleLog = console.log; + const consoleLogMock = vi.fn(); + console.log = consoleLogMock; + + const originalLink = vi.mocked(productJSONFile).GitHubFeedbackLinks.issues; + + vi.mocked(productJSONFile).GitHubFeedbackLinks.issues = ''; + + const issueProperties: GitHubIssue = { + category: 'feature', + title: 'new feature', + description: 'feature description', + }; + + const feedbackHandler = new FeedbackHandler(extensionLoaderMock); + await feedbackHandler.openGitHubIssue(issueProperties); + + expect(consoleLogMock).toHaveBeenCalledWith('No GitHub issues link found, cannot preview new GitHub issue'); + + console.log = originalConsoleLog; + vi.mocked(productJSONFile).GitHubFeedbackLinks.issues = originalLink; + }); + test('Expect openExternal to be called with queryParams and feature request template', async () => { const issueProperties: GitHubIssue = { category: 'feature', diff --git a/packages/main/src/plugin/feedback-handler.ts b/packages/main/src/plugin/feedback-handler.ts index 7c269a517f1..c27a17166fb 100644 --- a/packages/main/src/plugin/feedback-handler.ts +++ b/packages/main/src/plugin/feedback-handler.ts @@ -43,15 +43,29 @@ export class FeedbackHandler { } async openGitHubIssue(issueProperties: GitHubIssue): Promise { - let additionalContent: string | undefined; - if (issueProperties.includeExtensionInfo) { - const extensions = await this.getStartedExtensions(); - additionalContent = `**Enabled Extensions**\n${extensions}`; - } + if (productJSONFile.GitHubFeedbackLinks?.issues) { + let additionalContent: string | undefined; + if (issueProperties.includeExtensionInfo) { + const extensions = await this.getStartedExtensions(); + additionalContent = `**Enabled Extensions**\n${extensions}`; + } - const urlSearchParams = new URLSearchParams(this.toQueryParameters(issueProperties, additionalContent)).toString(); - const link = `https://github.com/containers/podman-desktop/issues/new?${urlSearchParams}`; - await shell.openExternal(link); + const urlSearchParams = new URLSearchParams( + this.toQueryParameters(issueProperties, additionalContent), + ).toString(); + const link = `${productJSONFile.GitHubFeedbackLinks?.issues}/new?${urlSearchParams}`; + await shell.openExternal(link); + } else { + console.log('No GitHub issues link found, cannot preview new GitHub issue'); + } + } + + getGitHubFeedbackLinks(): { [category: string]: string } | undefined { + return productJSONFile.GitHubFeedbackLinks; + } + + getFeedbackLinks(): { [category: string]: string } | undefined { + return productJSONFile.feedbackLinks; } protected async getStartedExtensions(): Promise { diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index dba492239c2..f33079e37e5 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -3079,6 +3079,20 @@ export class PluginSystem { return feedback.openGitHubIssue(properties); }); + this.ipcHandle( + 'feedback:getGitHubFeedbackLinks', + async (_listener): Promise<{ [category: string]: string } | undefined> => { + return feedback.getGitHubFeedbackLinks(); + }, + ); + + this.ipcHandle( + 'feedback:getFeedbackLinks', + async (_listener): Promise<{ [category: string]: string } | undefined> => { + return feedback.getFeedbackLinks(); + }, + ); + this.ipcHandle('feedback:getFeedbackMessages', async (): Promise => { return feedback.getFeedbackMessages(); }); diff --git a/packages/preload/src/index.ts b/packages/preload/src/index.ts index 08e9601401f..b90db5f8ebf 100644 --- a/packages/preload/src/index.ts +++ b/packages/preload/src/index.ts @@ -2444,6 +2444,17 @@ export function initExposure(): void { return ipcInvoke('feedback:GitHubPreview', feedback); }); + contextBridge.exposeInMainWorld( + 'getGitHubFeedbackLinks', + async (): Promise<{ [category: string]: string } | undefined> => { + return ipcInvoke('feedback:getGitHubFeedbackLinks'); + }, + ); + + contextBridge.exposeInMainWorld('getFeedbackLinks', async (): Promise<{ [category: string]: string } | undefined> => { + return ipcInvoke('feedback:getFeedbackLinks'); + }); + contextBridge.exposeInMainWorld('getFeedbackMessages', async (): Promise => { return ipcInvoke('feedback:getFeedbackMessages'); }); diff --git a/packages/renderer/src/lib/feedback/SendFeedback.spec.ts b/packages/renderer/src/lib/feedback/SendFeedback.spec.ts index 557aa4a92d7..cbe1ee90ad7 100644 --- a/packages/renderer/src/lib/feedback/SendFeedback.spec.ts +++ b/packages/renderer/src/lib/feedback/SendFeedback.spec.ts @@ -44,11 +44,17 @@ beforeAll(() => { beforeEach(() => { vi.resetAllMocks(); + vi.mocked(window.getGitHubFeedbackLinks).mockResolvedValue({ + bug: '/bug/link', + feature: '/feature/link', + }); }); -test('Expect developers feedback form to be rendered by default', () => { +test('Expect developers feedback form to be rendered by default', async () => { render(SendFeedback); + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + expect(DirectFeedback).toHaveBeenCalledOnce(); expect(DirectFeedback).toHaveBeenCalledWith(expect.anything(), { onCloseForm: expect.any(Function), @@ -61,6 +67,8 @@ test('Expect developers feedback form to be rendered by default', () => { test('Expect confirmation dialog to be displayed if content changed', async () => { render(SendFeedback, {}); + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + expect(DirectFeedback).toHaveBeenCalledWith(expect.anything(), { onCloseForm: expect.any(Function), category: 'developers', @@ -87,6 +95,8 @@ test('Expect confirmation dialog to be displayed if content changed', async () = test('Expect no confirmation dialog to be displayed if content has not changed', async () => { render(SendFeedback, {}); + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + expect(DirectFeedback).toHaveBeenCalledWith(expect.anything(), { onCloseForm: expect.any(Function), category: 'developers', @@ -105,6 +115,8 @@ test('Expect no confirmation dialog to be displayed if content has not changed', test('Expect DirectFeedback form to be rendered when design category is selected', async () => { render(SendFeedback, {}); + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ }); expect(categorySelect).toBeInTheDocument(); categorySelect.focus(); @@ -126,6 +138,8 @@ test('Expect DirectFeedback form to be rendered when design category is selected test('Expect GitHubIssue feedback form to be rendered if category is not developers', async () => { render(SendFeedback, {}); + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ }); expect(categorySelect).toBeInTheDocument(); categorySelect.focus(); @@ -138,6 +152,10 @@ test('Expect GitHubIssue feedback form to be rendered if category is not develop // click on a smiley expect(vi.mocked(GitHubIssueFeedback)).toHaveBeenNthCalledWith(1, expect.anything(), { onCloseForm: expect.any(Function), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, category: 'bug', contentChange: expect.any(Function), }); @@ -150,7 +168,71 @@ test('Expect GitHubIssue feedback form to be rendered if category is not develop await fireEvent.click(featureCategory); expect(vi.mocked(GitHubIssueFeedback)).toHaveBeenNthCalledWith(1, expect.anything(), { onCloseForm: expect.any(Function), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, category: 'feature', contentChange: expect.any(Function), }); }); + +test('Expect if there no GitHub links to have a new category other with other feedback links', async () => { + vi.mocked(window.getGitHubFeedbackLinks).mockResolvedValue(undefined); + vi.mocked(window.getFeedbackLinks).mockResolvedValue({ + category1: '/catgory1/link', + category2: '/catgory2/link', + category3: '/catgory3/link', + }); + + render(SendFeedback); + + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + + const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ }); + expect(categorySelect).toBeInTheDocument(); + categorySelect.focus(); + + // open feedback categories dropdown + await userEvent.keyboard('[ArrowDown]'); + + const bugCategory = screen.queryByRole('button', { name: /Bug/ }); + expect(bugCategory).not.toBeInTheDocument(); + + const featureCategory = screen.queryByRole('button', { name: /Feature/ }); + expect(featureCategory).not.toBeInTheDocument(); + + const otherCategory = screen.getByRole('button', { name: /Other/ }); + expect(otherCategory).toBeInTheDocument(); + + await fireEvent.click(otherCategory); + + expect(screen.getByText('category1')).toBeInTheDocument(); + expect(screen.getByText('category2')).toBeInTheDocument(); + expect(screen.getByText('category3')).toBeInTheDocument(); +}); + +test('Expect if there no GitHub links to not have a new category other if there are also no other feedback links', async () => { + vi.mocked(window.getGitHubFeedbackLinks).mockResolvedValue(undefined); + vi.mocked(window.getFeedbackLinks).mockResolvedValue({}); + + render(SendFeedback); + + await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled()); + + const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ }); + expect(categorySelect).toBeInTheDocument(); + categorySelect.focus(); + + // open feedback categories dropdown + await userEvent.keyboard('[ArrowDown]'); + + const bugCategory = screen.queryByRole('button', { name: /Bug/ }); + expect(bugCategory).not.toBeInTheDocument(); + + const featureCategory = screen.queryByRole('button', { name: /Feature/ }); + expect(featureCategory).not.toBeInTheDocument(); + + const otherCategory = screen.queryByRole('button', { name: /Other/ }); + expect(otherCategory).not.toBeInTheDocument(); +}); diff --git a/packages/renderer/src/lib/feedback/SendFeedback.svelte b/packages/renderer/src/lib/feedback/SendFeedback.svelte index 138ee4386e7..ed3ee776d73 100644 --- a/packages/renderer/src/lib/feedback/SendFeedback.svelte +++ b/packages/renderer/src/lib/feedback/SendFeedback.svelte @@ -1,6 +1,10 @@ {#if displayModal} @@ -65,14 +86,26 @@ function handleUpdate(e: boolean): void {
({ value: e[0], label: e[1] }))}> + options={Array.from(feedbackCategories).map(e => ({ value: e[0], label: e[1] }))}>
{#if category === 'developers' || category === 'design'} {:else if category === 'bug' || category === 'feature'} - + + {:else if category === 'other'} + + +

Could not find the right category? Take a look at these additional options:

+ {#each Object.entries(feedbackLinks) as [category, link] (category)} + => window.openExternal(link)}>{category} + {/each} +
+ + + +
{/if} diff --git a/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.spec.ts b/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.spec.ts index d3fc4e0898d..8790064d7d9 100644 --- a/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.spec.ts +++ b/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.spec.ts @@ -104,6 +104,10 @@ test('Expect feedback form to to be GitHub issue feedback form', async () => { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(title).toBeInTheDocument(); @@ -116,6 +120,10 @@ test('Expect Preview on GitHub button to be disabled if there is no title or des category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); // default: disabled @@ -149,6 +157,10 @@ test.each([ category: category as GitHubFeedbackCategory, onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(title).toHaveProperty('placeholder', placeholders.title); @@ -158,17 +170,21 @@ test.each([ test.each([ { category: 'bug', - link: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Fbug%20%F0%9F%90%9E%22', + link: '/bug/link', }, { category: 'feature', - link: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Ffeature%20%F0%9F%92%A1%22', + link: '/feature/link', }, ])('$category should have specific issues link', async ({ category, link }) => { const { getByLabelText } = renderGitHubIssueFeedback({ category: category as GitHubFeedbackCategory, onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); const existingIssues = getByLabelText('GitHub issues'); @@ -187,6 +203,10 @@ test.each([ category: category, onCloseForm: onCloseFormMock, contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); // type dummy data @@ -216,6 +236,10 @@ describe('includeSystemInfo', () => { category: 'feature', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(includeSystemInfo).toBeUndefined(); }); @@ -225,6 +249,10 @@ describe('includeSystemInfo', () => { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(includeSystemInfo).toBeInTheDocument(); @@ -237,6 +265,10 @@ describe('includeSystemInfo', () => { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); if (!includeSystemInfo) throw new Error('includeSystemInfo should be defined'); @@ -265,6 +297,10 @@ describe('includeExtensionInfo', () => { category: 'feature', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(includeExtensionInfo).toBeUndefined(); }); @@ -274,6 +310,10 @@ describe('includeExtensionInfo', () => { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(includeExtensionInfo).toBeInTheDocument(); @@ -286,6 +326,10 @@ describe('includeExtensionInfo', () => { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); if (!includeExtensionInfo) throw new Error('includeExtensionInfo should be defined'); @@ -316,6 +360,10 @@ test.each([ category: category, onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(window.telemetryTrack).toHaveBeenNthCalledWith(1, `feedback.FormOpened`, { feedbackCategory: category }); @@ -338,6 +386,10 @@ test.each([ category: category, onCloseForm: vi.fn(), contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); expect(window.telemetryTrack).toHaveBeenNthCalledWith(1, `feedback.FormOpened`, { feedbackCategory: category }); @@ -360,6 +412,10 @@ test('Expect close confirmation to be true if cancel clicked', async () => { category: 'bug', onCloseForm: closeMock, contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); // click on a cancel @@ -376,6 +432,10 @@ test('Expect opening existing GitHub issues to not close the feedback window', a category: 'bug', onCloseForm: onCloseFormMock, contentChange: vi.fn(), + categoryLinks: { + bug: '/bug/link', + feature: '/feature/link', + }, }); const gitHubLink = getByLabelText('GitHub issues'); diff --git a/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.svelte b/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.svelte index 58b43255aff..f5cf1207b03 100644 --- a/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.svelte +++ b/packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.svelte @@ -7,11 +7,12 @@ import FeedbackForm from '/@/lib/feedback/FeedbackForm.svelte'; interface Props { onCloseForm: (confirm: boolean) => void; + categoryLinks: { [category: string]: string } | undefined; category: GitHubFeedbackCategory; contentChange: (e: boolean) => void; } -let { onCloseForm, category = 'bug', contentChange }: Props = $props(); +let { onCloseForm, categoryLinks = {}, category = 'bug', contentChange }: Props = $props(); let issueTitle = $state(''); let issueDescription = $state(''); @@ -33,13 +34,9 @@ let issueValidationError = $derived.by(() => { } }); -let titlePlaceholder = $state(category === 'bug' ? 'Bug Report Title' : 'Feature name'); -let descriptionPlaceholder = $state(category === 'bug' ? 'Bug description' : 'Feature description'); -let existingIssuesLink = $state( - category === 'bug' - ? 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Fbug%20%F0%9F%90%9E%22' - : 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Ffeature%20%F0%9F%92%A1%22', -); +let titlePlaceholder = $derived(category === 'bug' ? 'Bug Report Title' : 'Feature name'); +let descriptionPlaceholder = $derived(category === 'bug' ? 'Bug description' : 'Feature description'); +let existingIssuesLink = $derived(category === 'bug' ? categoryLinks.bug : categoryLinks.feature); $effect(() => contentChange(Boolean(issueTitle || issueDescription))); @@ -48,7 +45,9 @@ onMount(async () => { }); async function openGitHubIssues(): Promise { - await window.openExternal(existingIssuesLink); + if (existingIssuesLink || categoryLinks.issues) { + await window.openExternal(existingIssuesLink ?? categoryLinks.issues); + } } async function previewOnGitHub(): Promise { @@ -80,7 +79,7 @@ async function previewOnGitHub(): Promise { -
You can search existing {category} issues on github.com/podman-desktop/podman-desktop/issues
+
You can search existing {category} issues on GitHub