Skip to content

Commit

Permalink
feat(feedback): add enabled extensions in additional-context (#10276)
Browse files Browse the repository at this point in the history 
* feat(feedback): add enabled extensions in additional-context

Signed-off-by: axel7083 <[email protected]>

* fix: typo

Co-authored-by: Philippe Martin <[email protected]>
Signed-off-by: axel7083 <[email protected]>

* fix(feedback): sorting extensions

Signed-off-by: axel7083 <[email protected]>

---------

Signed-off-by: axel7083 <[email protected]>
Co-authored-by: Philippe Martin <[email protected]>
  • Loading branch information
@axel7083 @feloy
axel7083 and feloy authored Dec 11, 2024
1 parent cc38f43 commit 4cd79ac
Show file tree
Hide file tree
Showing 6 changed files with 175 additions and 14 deletions.
88 changes: 83 additions & 5 deletions packages/main/src/plugin/feedback-handler.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import { release } from 'node:os';
import { shell } from 'electron';
import { beforeEach, describe, expect, test, vi } from 'vitest';

import type { ExtensionLoader } from '/@/plugin/extension-loader.js';
import { isLinux, isMac, isWindows } from '/@/util.js';
import type { ExtensionInfo } from '/@api/extension-info.js';
import type { GitHubIssue } from '/@api/feedback.js';

import { FeedbackHandler } from './feedback-handler.js';
Expand All @@ -42,13 +44,29 @@ vi.mock('node:os', () => ({
release: vi.fn(),
}));

const extensionLoaderMock: ExtensionLoader = {
listExtensions: vi.fn(),
} as unknown as ExtensionLoader;

const STARTED_EXTENSION: ExtensionInfo = {
state: 'started',
id: 'dummy-started-extension',
} as ExtensionInfo;

const STOPPED_EXTENSION: ExtensionInfo = {
state: 'stopped',
id: 'dummy-stopped-extension',
} as ExtensionInfo;

beforeEach(() => {
vi.resetAllMocks();

// default to linux for testing
vi.mocked(isLinux).mockReturnValue(true);
vi.mocked(isMac).mockReturnValue(false);
vi.mocked(isWindows).mockReturnValue(false);

vi.mocked(extensionLoaderMock.listExtensions).mockResolvedValue([STARTED_EXTENSION, STOPPED_EXTENSION]);
});

/**
Expand All @@ -62,8 +80,8 @@ function containSearchParams(url: string | undefined, params: Record<string, str

const search = new URLSearchParams(new URL(url).search);
Object.entries(params).forEach(([key, value]) => {
expect(search.has(key)).toBeTruthy();
expect(search.get(key)).toBe(value);
expect(search.has(key), `expected url to have ${key} as query parameter`).toBeTruthy();
expect(search.get(key), `expected ${search.get(key)} to be ${value}`).toBe(value);
});
}

Expand All @@ -75,7 +93,7 @@ describe('openGitHubIssue', () => {
description: 'bug description',
};

const feedbackHandler = new FeedbackHandler();
const feedbackHandler = new FeedbackHandler(extensionLoaderMock);
await feedbackHandler.openGitHubIssue(issueProperties);

expect(shell.openExternal).toHaveBeenCalledOnce();
Expand All @@ -96,7 +114,7 @@ describe('openGitHubIssue', () => {
description: 'feature description',
};

const feedbackHandler = new FeedbackHandler();
const feedbackHandler = new FeedbackHandler(extensionLoaderMock);
await feedbackHandler.openGitHubIssue(issueProperties);

expect(shell.openExternal).toHaveBeenCalledOnce();
Expand All @@ -120,7 +138,7 @@ describe('openGitHubIssue', () => {
includeSystemInfo: true,
};

const feedbackHandler = new FeedbackHandler();
const feedbackHandler = new FeedbackHandler(extensionLoaderMock);
await feedbackHandler.openGitHubIssue(issueProperties);

expect(shell.openExternal).toHaveBeenCalledOnce();
Expand All @@ -131,4 +149,64 @@ describe('openGitHubIssue', () => {
os: 'Linux - dummy-release',
});
});

test('expect enabled extensions to be included if includeExtensionInfo is true', async () => {
const issueProperties: GitHubIssue = {
category: 'bug',
title: 'PD is not working',
description: 'bug description',
includeSystemInfo: false,
includeExtensionInfo: true,
};

const feedbackHandler = new FeedbackHandler(extensionLoaderMock);
await feedbackHandler.openGitHubIssue(issueProperties);

expect(shell.openExternal).toHaveBeenCalledOnce();

// extract the first argument of the shell.openExternal call
const url: string | undefined = vi.mocked(shell.openExternal).mock.calls[0]?.[0];
containSearchParams(url, {
'additional-context': '**Enabled Extensions**\n- dummy-started-extension',
});
});

test('expect extensions to be sorted', async () => {
const extensions: ExtensionInfo[] = [
{
id: 'b',
name: 'b',
state: 'started',
},
{
id: 'a',
name: 'a',
state: 'started',
},
{
id: 'c',
name: 'c',
state: 'started',
},
] as ExtensionInfo[];

vi.mocked(extensionLoaderMock.listExtensions).mockResolvedValue(extensions);

const issueProperties: GitHubIssue = {
category: 'bug',
title: 'PD is not working',
description: 'bug description',
includeSystemInfo: false,
includeExtensionInfo: true,
};

const feedbackHandler = new FeedbackHandler(extensionLoaderMock);
await feedbackHandler.openGitHubIssue(issueProperties);

// extract the first argument of the shell.openExternal call
const url: string | undefined = vi.mocked(shell.openExternal).mock.calls[0]?.[0];
containSearchParams(url, {
'additional-context': '**Enabled Extensions**\n- a\n- b\n- c',
});
});
});
25 changes: 22 additions & 3 deletions packages/main/src/plugin/feedback-handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,39 @@

import { shell } from 'electron';

import type { ExtensionLoader } from '/@/plugin/extension-loader.js';
import type { SystemInfo } from '/@/plugin/util/sys-info.js';
import { getSystemInfo } from '/@/plugin/util/sys-info.js';
import type { GitHubIssue } from '/@api/feedback.js';

export class FeedbackHandler {
#systemInfo: SystemInfo;

constructor() {
constructor(protected extensionLoader: ExtensionLoader) {
this.#systemInfo = getSystemInfo();
}

async openGitHubIssue(issueProperties: GitHubIssue): Promise<void> {
const urlSearchParams = new URLSearchParams(this.toQueryParameters(issueProperties)).toString();
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);
}

protected async getStartedExtensions(): Promise<string> {
const extensions = await this.extensionLoader.listExtensions();
return extensions
.filter(extension => extension.state === 'started')
.sort((a, b) => a.name.localeCompare(b.name))
.map(extension => `- ${extension.id}`)
.join('\n');
}

/**
* Generic method to get the system info (system platform, architecture, version)
* @protected
Expand All @@ -43,7 +59,7 @@ export class FeedbackHandler {
return this.#systemInfo.getSystemName();
}

protected toQueryParameters(issue: GitHubIssue): Record<string, string> {
protected toQueryParameters(issue: GitHubIssue, additional?: string): Record<string, string> {
const result: Record<string, string> = {};
result['title'] = issue.title;

Expand All @@ -61,6 +77,9 @@ export class FeedbackHandler {
result['os'] = this.getOsInfo();
}

// append additional context if provided
if (additional) result['additional-context'] = additional;

return result;
default:
throw new Error(`unknown category ${issue.category}`);
Expand Down
5 changes: 2 additions & 3 deletions packages/main/src/plugin/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,6 @@ export class PluginSystem {

const telemetry = new Telemetry(configurationRegistry);
await telemetry.init();

const feedback = new FeedbackHandler();

const exec = new Exec(proxy);

const commandRegistry = new CommandRegistry(apiSender, telemetry);
Expand Down Expand Up @@ -704,6 +701,8 @@ export class PluginSystem {
);
await this.extensionLoader.init();

const feedback = new FeedbackHandler(this.extensionLoader);

const extensionsCatalog = new ExtensionsCatalog(certificates, proxy, configurationRegistry, apiSender);
extensionsCatalog.init();
const featured = new Featured(this.extensionLoader, extensionsCatalog);
Expand Down
10 changes: 7 additions & 3 deletions packages/renderer/src/lib/actions/ContributionActions.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ export let contextUI: ContextUI | undefined = undefined;
let filteredContributions: Menu[] = [];
$: {
console.log('[ContributionActions] contextUI', contextUI?.collectAllValues());
filteredContributions = contributions.reduce((previousValue, currentValue) => {
// Transform the unknown[] args objects as contexts
const argsContexts = args.map(arg => transformObjectToContext(arg, contextPrefix));
console.log('[ContributionActions] argsContexts', argsContexts);
// If no when property is set, we keep all additional menus
if (currentValue.when === undefined) return [...previousValue, currentValue];
Expand All @@ -36,9 +43,6 @@ $: {
return [...previousValue, currentValue];
}
// Transform the unknown[] args objects as contexts
const argsContexts = args.map(arg => transformObjectToContext(arg, contextPrefix));
// Evaluate the arguments as context
for (let argsContext of argsContexts) {
if (whenDeserialized?.evaluate(argsContext)) {
Expand Down
53 changes: 53 additions & 0 deletions packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ function renderGitHubIssueFeedback(props: ComponentProps<typeof GitHubIssueFeedb
description: HTMLTextAreaElement;
preview: HTMLButtonElement;
includeSystemInfo?: HTMLElement;
includeExtensionInfo?: HTMLElement;
} & RenderResult<Component<ComponentProps<typeof GitHubIssueFeedback>>> {
const { getByRole, queryByTitle, ...restResult } = render(GitHubIssueFeedback, props);

Expand All @@ -75,11 +76,14 @@ function renderGitHubIssueFeedback(props: ComponentProps<typeof GitHubIssueFeedb
// checkbox
const includeSystemInfo = queryByTitle('Include system information') ?? undefined;

const includeExtensionInfo = queryByTitle('Include enabled extensions') ?? undefined;

return {
title: title as HTMLInputElement,
description: description as HTMLTextAreaElement,
preview: preview as HTMLButtonElement,
includeSystemInfo,
includeExtensionInfo,
getByRole,
queryByTitle,
...restResult,
Expand Down Expand Up @@ -239,3 +243,52 @@ describe('includeSystemInfo', () => {
);
});
});

describe('includeExtensionInfo', () => {
test('should not be visible on category feature', async () => {
const { includeExtensionInfo } = renderGitHubIssueFeedback({
category: 'feature',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
});
expect(includeExtensionInfo).toBeUndefined();
});

test('should be visible on category bug and enabled by default', async () => {
const { includeExtensionInfo } = renderGitHubIssueFeedback({
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
});
expect(includeExtensionInfo).toBeInTheDocument();

// enabled by default
expect(includeExtensionInfo?.children?.[0]).toHaveClass('text-[var(--pd-input-checkbox-checked)]');
});

test('uncheck the Include system information should set includeSystemInfo to false', async () => {
const { preview, title, description, includeExtensionInfo } = renderGitHubIssueFeedback({
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
});

if (!includeExtensionInfo) throw new Error('includeExtensionInfo should be defined');

// type dummy data
await userEvent.type(title, 'Bug title');
await userEvent.type(description, 'Bug description');

// uncheck
await userEvent.click(includeExtensionInfo);

// open in GitHub
await userEvent.click(preview);

expect(previewOnGitHubMock).toHaveBeenCalledWith(
expect.objectContaining({
includeExtensionInfo: false,
}),
);
});
});
8 changes: 8 additions & 0 deletions packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ let { onCloseForm = () => {}, category = 'bug', contentChange }: Props = $props(
let issueTitle = $state('');
let issueDescription = $state('');
let includeSystemInfo: boolean = $state(true); // default to true
let includeExtensionInfo: boolean = $state(true); // default to true
let issueValidationError = $derived.by(() => {
if (!issueTitle) {
Expand Down Expand Up @@ -52,6 +53,7 @@ async function previewOnGitHub(): Promise<void> {
title: $state.snapshot(issueTitle),
description: $state.snapshot(issueDescription),
includeSystemInfo: $state.snapshot(includeSystemInfo),
includeExtensionInfo: $state.snapshot(includeExtensionInfo),
};
try {
await window.previewOnGitHub(issueProperties);
Expand Down Expand Up @@ -97,6 +99,12 @@ async function previewOnGitHub(): Promise<void> {
bind:checked={includeSystemInfo} />
<div>Include system information (os, architecture etc.)</div>
</div>
<div class="flex flex-row align-items items-center mt-2">
<Checkbox
title="Include enabled extensions"
bind:checked={includeExtensionInfo} />
<div>Include enabled extensions</div>
</div>
{/if}
</svelte:fragment>
<svelte:fragment slot="validation">
Expand Down

0 comments on commit 4cd79ac

Please sign in to comment.