From 4d32b20d28d0c3a89b411e8e1062df3377d552ac Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 10:15:14 -0400 Subject: [PATCH 01/55] Sample json --- notifications.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 notifications.json diff --git a/notifications.json b/notifications.json new file mode 100644 index 0000000000..3dda9c9133 --- /dev/null +++ b/notifications.json @@ -0,0 +1,8 @@ +{ + "notifications": [ + { + "type": "info", + "message": "Editions is here!" + } + ] +} From f2a1ee56a55ef2ede20651d783e10f217add1906 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 10:57:25 -0400 Subject: [PATCH 02/55] Update notifications.json --- notifications.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/notifications.json b/notifications.json index 3dda9c9133..4a2f26fd03 100644 --- a/notifications.json +++ b/notifications.json @@ -2,7 +2,11 @@ "notifications": [ { "type": "info", - "message": "Editions is here!" + "title": "Editions is here!", + "message": "Check out shopify.com/editions/summer2024", + "minDate": "2024-06-27", + "minVersion": "3.62.0", + "commands": ["app:generate:extension"] } ] } From 58e7c0923ab5a8c19164f0581e161c8383cec1ef Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 10:57:55 -0400 Subject: [PATCH 03/55] Show notifications --- .../cli-kit/src/public/node/base-command.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index 02ff639687..a2b17e5f02 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -8,8 +8,10 @@ import {outputContent, outputInfo, outputToken} from './output.js' import {terminalSupportsRawMode} from './system.js' import {hashString} from './crypto.js' import {isTruthy} from './context/utilities.js' +import {versionSatisfies} from './node-package-manager.js' import {JsonMap} from '../../private/common/json.js' import {underscore} from '../common/string.js' +import {CLI_KIT_VERSION} from '../common/version.js' import {Command, Errors} from '@oclif/core' import {FlagOutput, Input, ParserOutput, FlagInput, ArgOutput} from '@oclif/core/lib/interfaces/parser.js' @@ -18,6 +20,23 @@ interface EnvironmentFlags { path?: string } +interface Notifications { + notifications: Notification[] +} + +interface Notification { + message: string + type: 'info' | 'warning' | 'error' + title?: string + minVersion?: string + maxVersion?: string + minDate?: string + maxDate?: string + commands?: string[] + surface?: 'app' | 'theme' | 'hydrogen' | string + frequency?: 'always' | 'once_a_day' | 'once_a_week' +} + abstract class BaseCommand extends Command { // eslint-disable-next-line @typescript-eslint/ban-types static baseFlags: FlagInput<{}> = {} @@ -50,6 +69,7 @@ abstract class BaseCommand extends Command { await registerCleanBugsnagErrorsFromWithinPlugins(this.config) } this.showNpmFlagWarning() + await this.showNotifications() return super.init() } @@ -73,6 +93,36 @@ abstract class BaseCommand extends Command { } } + protected async showNotifications(): Promise { + const response = await fetch( + 'https://raw.githubusercontent.com/Shopify/cli/4d32b20d28d0c3a89b411e8e1062df3377d552ac/notifications.json', + ) + const notifications = await (response.json() as Promise) + const today = new Date() + + const notificationsToShow = notifications.notifications + .filter((notification) => { + const minVersion = !notification.minVersion || versionSatisfies(CLI_KIT_VERSION, `>=${notification.minVersion}`) + const maxVersion = !notification.maxVersion || versionSatisfies(CLI_KIT_VERSION, `<=${notification.maxVersion}`) + return minVersion && maxVersion + }) + .filter((notification) => { + const minDate = !notification.minDate || new Date(notification.minDate) >= today + const maxDate = !notification.maxDate || new Date(notification.maxDate) <= today + return minDate && maxDate + }) + .filter((notification) => { + return !notification.commands || notification.commands?.includes(this.id!) + }) + + notificationsToShow.forEach((notification) => { + renderInfo({ + headline: notification.title, + body: notification.message, + }) + }) + } + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types protected exitWithTimestampWhenEnvVariablePresent() { if (isTruthy(process.env.SHOPIFY_CLI_ENV_STARTUP_PERFORMANCE_RUN)) { From 29cbeb0ab09e6c8bd8d49359454e422614e4841a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 26 Jun 2024 11:12:57 -0400 Subject: [PATCH 04/55] Extract the notification system to its own file --- .../cli-kit/src/public/node/base-command.ts | 52 +------------- .../src/public/node/notifications-system.ts | 72 +++++++++++++++++++ 2 files changed, 74 insertions(+), 50 deletions(-) create mode 100644 packages/cli-kit/src/public/node/notifications-system.ts diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index a2b17e5f02..fadf00934b 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -8,10 +8,9 @@ import {outputContent, outputInfo, outputToken} from './output.js' import {terminalSupportsRawMode} from './system.js' import {hashString} from './crypto.js' import {isTruthy} from './context/utilities.js' -import {versionSatisfies} from './node-package-manager.js' +import {showNotificationsIfNeeded} from './notifications-system.js' import {JsonMap} from '../../private/common/json.js' import {underscore} from '../common/string.js' -import {CLI_KIT_VERSION} from '../common/version.js' import {Command, Errors} from '@oclif/core' import {FlagOutput, Input, ParserOutput, FlagInput, ArgOutput} from '@oclif/core/lib/interfaces/parser.js' @@ -20,23 +19,6 @@ interface EnvironmentFlags { path?: string } -interface Notifications { - notifications: Notification[] -} - -interface Notification { - message: string - type: 'info' | 'warning' | 'error' - title?: string - minVersion?: string - maxVersion?: string - minDate?: string - maxDate?: string - commands?: string[] - surface?: 'app' | 'theme' | 'hydrogen' | string - frequency?: 'always' | 'once_a_day' | 'once_a_week' -} - abstract class BaseCommand extends Command { // eslint-disable-next-line @typescript-eslint/ban-types static baseFlags: FlagInput<{}> = {} @@ -69,7 +51,7 @@ abstract class BaseCommand extends Command { await registerCleanBugsnagErrorsFromWithinPlugins(this.config) } this.showNpmFlagWarning() - await this.showNotifications() + await showNotificationsIfNeeded(this.id!) return super.init() } @@ -93,36 +75,6 @@ abstract class BaseCommand extends Command { } } - protected async showNotifications(): Promise { - const response = await fetch( - 'https://raw.githubusercontent.com/Shopify/cli/4d32b20d28d0c3a89b411e8e1062df3377d552ac/notifications.json', - ) - const notifications = await (response.json() as Promise) - const today = new Date() - - const notificationsToShow = notifications.notifications - .filter((notification) => { - const minVersion = !notification.minVersion || versionSatisfies(CLI_KIT_VERSION, `>=${notification.minVersion}`) - const maxVersion = !notification.maxVersion || versionSatisfies(CLI_KIT_VERSION, `<=${notification.maxVersion}`) - return minVersion && maxVersion - }) - .filter((notification) => { - const minDate = !notification.minDate || new Date(notification.minDate) >= today - const maxDate = !notification.maxDate || new Date(notification.maxDate) <= today - return minDate && maxDate - }) - .filter((notification) => { - return !notification.commands || notification.commands?.includes(this.id!) - }) - - notificationsToShow.forEach((notification) => { - renderInfo({ - headline: notification.title, - body: notification.message, - }) - }) - } - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types protected exitWithTimestampWhenEnvVariablePresent() { if (isTruthy(process.env.SHOPIFY_CLI_ENV_STARTUP_PERFORMANCE_RUN)) { diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts new file mode 100644 index 0000000000..0028740e35 --- /dev/null +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -0,0 +1,72 @@ +import {versionSatisfies} from './node-package-manager.js' +import {renderInfo} from './ui.js' +import {CLI_KIT_VERSION} from '../common/version.js' + +interface Notifications { + notifications: Notification[] +} + +interface Notification { + message: string + type: 'info' | 'warning' | 'error' + title?: string + minVersion?: string + maxVersion?: string + minDate?: string + maxDate?: string + commands?: string[] + surface?: 'app' | 'theme' | 'hydrogen' | string + frequency?: 'always' | 'once_a_day' | 'once_a_week' +} + +/** + * Shows notifications to the user if they meet the criteria specified in the notifications.json file. + * + * @param commandId - The command ID that triggered the notifications. + * @param _surface - The surface that triggered the notifications. + * @returns - A promise that resolves when the notifications have been shown. + */ +export async function showNotificationsIfNeeded(commandId: string, _surface?: string): Promise { + const response = await fetch( + 'https://raw.githubusercontent.com/Shopify/cli/4d32b20d28d0c3a89b411e8e1062df3377d552ac/notifications.json', + ) + const notifications = await (response.json() as Promise) + + const notificationsToShow = notifications.notifications + .filter(filterByVersion) + .filter(filterByDate) + .filter((notification) => { + return !notification.commands || notification.commands?.includes(commandId) + }) + + notificationsToShow.forEach((notification) => { + renderInfo({ + headline: notification.title, + body: notification.message, + }) + }) +} + +/** + * Filters notifications based on the version of the CLI. + * + * @param notification - The notification to filter. + */ +function filterByVersion(notification: Notification) { + const minVersion = !notification.minVersion || versionSatisfies(CLI_KIT_VERSION, `>=${notification.minVersion}`) + const maxVersion = !notification.maxVersion || versionSatisfies(CLI_KIT_VERSION, `<=${notification.maxVersion}`) + return minVersion && maxVersion +} + +const today = new Date() + +/** + * Filters notifications based on the date. + * + * @param notification - The notification to filter. + */ +function filterByDate(notification: Notification) { + const minDate = !notification.minDate || new Date(notification.minDate) >= today + const maxDate = !notification.maxDate || new Date(notification.maxDate) <= today + return minDate && maxDate +} From 92bf53bb57d064dc71ece4bc4489c7c9980a601c Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 11:21:52 -0400 Subject: [PATCH 05/55] Fix date filter and render by type --- .../src/public/node/notifications-system.ts | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 0028740e35..43ddb833a5 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -1,5 +1,5 @@ import {versionSatisfies} from './node-package-manager.js' -import {renderInfo} from './ui.js' +import {renderError, renderInfo, renderWarning} from './ui.js' import {CLI_KIT_VERSION} from '../common/version.js' interface Notifications { @@ -40,10 +40,23 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st }) notificationsToShow.forEach((notification) => { - renderInfo({ + const content = { headline: notification.title, body: notification.message, - }) + } + switch (notification.type) { + case 'info': { + renderInfo(content) + return + } + case 'warning': { + renderWarning(content) + return + } + case 'error': { + renderError(content) + } + } }) } @@ -66,7 +79,7 @@ const today = new Date() * @param notification - The notification to filter. */ function filterByDate(notification: Notification) { - const minDate = !notification.minDate || new Date(notification.minDate) >= today - const maxDate = !notification.maxDate || new Date(notification.maxDate) <= today + const minDate = !notification.minDate || new Date(notification.minDate) <= today + const maxDate = !notification.maxDate || new Date(notification.maxDate) >= today return minDate && maxDate } From e17405bf1140195b2e2f15c9ba7b99a827076473 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 11:23:55 -0400 Subject: [PATCH 06/55] Fix URL --- packages/cli-kit/src/public/node/notifications-system.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 43ddb833a5..05fd34f4c4 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -27,9 +27,7 @@ interface Notification { * @returns - A promise that resolves when the notifications have been shown. */ export async function showNotificationsIfNeeded(commandId: string, _surface?: string): Promise { - const response = await fetch( - 'https://raw.githubusercontent.com/Shopify/cli/4d32b20d28d0c3a89b411e8e1062df3377d552ac/notifications.json', - ) + const response = await fetch('https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json') const notifications = await (response.json() as Promise) const notificationsToShow = notifications.notifications From ed002135a4101e61f328be2b478954e71586878d Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 11:24:39 -0400 Subject: [PATCH 07/55] Update notifications.json --- notifications.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.json b/notifications.json index 4a2f26fd03..271931f12a 100644 --- a/notifications.json +++ b/notifications.json @@ -4,7 +4,7 @@ "type": "info", "title": "Editions is here!", "message": "Check out shopify.com/editions/summer2024", - "minDate": "2024-06-27", + "minDate": "2024-06-25", "minVersion": "3.62.0", "commands": ["app:generate:extension"] } From 1d13c6d270fdd2d944badf3ba1667263bfdda8a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 26 Jun 2024 11:34:09 -0400 Subject: [PATCH 08/55] Extract the filtering function --- .../src/public/node/notifications-system.ts | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 05fd34f4c4..a95ee1454b 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -30,12 +30,7 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st const response = await fetch('https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json') const notifications = await (response.json() as Promise) - const notificationsToShow = notifications.notifications - .filter(filterByVersion) - .filter(filterByDate) - .filter((notification) => { - return !notification.commands || notification.commands?.includes(commandId) - }) + const notificationsToShow = filterNotifications(notifications.notifications, commandId) notificationsToShow.forEach((notification) => { const content = { @@ -58,6 +53,20 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st }) } +/** + * Filters notifications based on the version of the CLI. + * + * @param notifications - The notifications to filter. + * @param commandId - The command ID to filter by. + * @returns - The filtered notifications. + */ +export function filterNotifications(notifications: Notification[], commandId: string): Notification[] { + return notifications + .filter(filterByVersion) + .filter(filterByDate) + .filter((notification) => filterByCommand(notification, commandId)) +} + /** * Filters notifications based on the version of the CLI. * @@ -81,3 +90,14 @@ function filterByDate(notification: Notification) { const maxDate = !notification.maxDate || new Date(notification.maxDate) >= today return minDate && maxDate } + +/** + * Filters notifications based on the command ID. + * + * @param notification - The notification to filter. + * @param commandId - The command ID to filter by. + * @returns - A boolean indicating whether the notification should be shown. + */ +function filterByCommand(notification: Notification, commandId: string) { + return !notification.commands || notification.commands.includes(commandId) +} From beb387080d7860ba05e16a91776145d97e5dd537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 26 Jun 2024 12:21:57 -0400 Subject: [PATCH 09/55] Empty test --- .../public/node/notifications-system.test.ts | 41 +++++++++++++++++++ .../src/public/node/notifications-system.ts | 31 ++++++++------ 2 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 packages/cli-kit/src/public/node/notifications-system.test.ts diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts new file mode 100644 index 0000000000..67edd875ef --- /dev/null +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -0,0 +1,41 @@ +import {Notification, filterNotifications} from './notifications-system.js' +import {describe, expect, test} from 'vitest' + +const notification1: Notification = { + title: 'title', + message: 'message', + type: 'info', + minVersion: '1.0.0', + maxVersion: '2.0.0', + minDate: '2021-01-01', + maxDate: '2021-01-02', + commands: ['commandId'], +} + +interface TestCase { + input: Notification[] + commandId: string + version: string + date: Date + output: Notification[] +} + +const testCases: TestCase[] = [ + { + input: [], + commandId: '', + version: '', + date: new Date(), + output: [], + }, +] + +describe('notifications-system filter notifications', () => { + test.each(testCases)('Filters', ({input, commandId, version, date, output}) => { + // When + const result = filterNotifications(input, commandId, date, version) + + // Then + expect(result).toEqual(output) + }) +}) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index a95ee1454b..0a76ee3cd8 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -6,7 +6,7 @@ interface Notifications { notifications: Notification[] } -interface Notification { +export interface Notification { message: string type: 'info' | 'warning' | 'error' title?: string @@ -30,7 +30,7 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st const response = await fetch('https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json') const notifications = await (response.json() as Promise) - const notificationsToShow = filterNotifications(notifications.notifications, commandId) + const notificationsToShow = filterNotifications(notifications.notifications, commandId, new Date(), CLI_KIT_VERSION) notificationsToShow.forEach((notification) => { const content = { @@ -58,12 +58,19 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st * * @param notifications - The notifications to filter. * @param commandId - The command ID to filter by. + * @param today - The current date. + * @param currentVersion - The current version of the CLI. * @returns - The filtered notifications. */ -export function filterNotifications(notifications: Notification[], commandId: string): Notification[] { +export function filterNotifications( + notifications: Notification[], + commandId: string, + today: Date, + currentVersion: string, +): Notification[] { return notifications - .filter(filterByVersion) - .filter(filterByDate) + .filter((notification) => filterByVersion(notification, currentVersion)) + .filter((notifications) => filterByDate(notifications, today)) .filter((notification) => filterByCommand(notification, commandId)) } @@ -71,28 +78,28 @@ export function filterNotifications(notifications: Notification[], commandId: st * Filters notifications based on the version of the CLI. * * @param notification - The notification to filter. + * @param currentVersion - The current version of the CLI. */ -function filterByVersion(notification: Notification) { - const minVersion = !notification.minVersion || versionSatisfies(CLI_KIT_VERSION, `>=${notification.minVersion}`) - const maxVersion = !notification.maxVersion || versionSatisfies(CLI_KIT_VERSION, `<=${notification.maxVersion}`) +function filterByVersion(notification: Notification, currentVersion: string) { + const minVersion = !notification.minVersion || versionSatisfies(currentVersion, `>=${notification.minVersion}`) + const maxVersion = !notification.maxVersion || versionSatisfies(currentVersion, `<=${notification.maxVersion}`) return minVersion && maxVersion } -const today = new Date() - /** * Filters notifications based on the date. * * @param notification - The notification to filter. + * @param today - The current date. */ -function filterByDate(notification: Notification) { +function filterByDate(notification: Notification, today: Date) { const minDate = !notification.minDate || new Date(notification.minDate) <= today const maxDate = !notification.maxDate || new Date(notification.maxDate) >= today return minDate && maxDate } /** - * Filters notifications based on the command ID. + * Filters notifications based on the command ID. * * @param notification - The notification to filter. * @param commandId - The command ID to filter by. From 1aac63ee0a41f708eff417585772ec20a6cac20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 26 Jun 2024 14:09:35 -0400 Subject: [PATCH 10/55] add test for notification-system --- .../public/node/notifications-system.test.ts | 79 +++++++++++++++---- 1 file changed, 65 insertions(+), 14 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index 67edd875ef..d69f5d1c19 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -1,39 +1,90 @@ import {Notification, filterNotifications} from './notifications-system.js' import {describe, expect, test} from 'vitest' -const notification1: Notification = { - title: 'title', +const betweenVersins1and2: Notification = { message: 'message', type: 'info', - minVersion: '1.0.0', - maxVersion: '2.0.0', - minDate: '2021-01-01', - maxDate: '2021-01-02', - commands: ['commandId'], + minVersion: '1.0', + maxVersion: '2.0', } +const betweenDatesIn2000: Notification = { + message: 'message', + type: 'info', + minDate: '2000-01-01', + maxDate: '2000-12-31', +} + +const fromVersion1: Notification = { + message: 'message', + type: 'info', + minVersion: '1.0', +} + +const upToVersion2: Notification = { + message: 'message', + type: 'info', + maxVersion: '2.0', +} + +const fromDateJan2000: Notification = { + message: 'message', + type: 'info', + minDate: '2000-01-01', +} + +const upToDateDec2000: Notification = { + message: 'message', + type: 'info', + maxDate: '2000-12-31', +} + +const onlyForDevCommand: Notification = { + message: 'message', + type: 'info', + commands: ['app:dev'], +} + +const defaultInput = [ + betweenVersins1and2, + betweenDatesIn2000, + fromVersion1, + upToVersion2, + fromDateJan2000, + upToDateDec2000, + onlyForDevCommand, +] + +/** + * Represents a test case + * @param input - the initial notifications received from remote + * @param comamndId - The current command being executed + * @param veresion - The current version of the CLI + * @param date - The current date for the user + * @param output - The expected filtered notifications + */ interface TestCase { input: Notification[] commandId: string version: string - date: Date + date: string output: Notification[] } const testCases: TestCase[] = [ { - input: [], - commandId: '', - version: '', - date: new Date(), - output: [], + input: defaultInput, + commandId: 'app:info', + version: '1.0', + date: '2024-01-01', + output: [betweenVersins1and2, betweenDatesIn2000], }, ] describe('notifications-system filter notifications', () => { test.each(testCases)('Filters', ({input, commandId, version, date, output}) => { // When - const result = filterNotifications(input, commandId, date, version) + const result = filterNotifications(input, commandId, new Date(date), version) // Then expect(result).toEqual(output) From 02db9ba7b4b5cc4280bf02314bb9204807eaa1e6 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 26 Jun 2024 14:16:00 -0400 Subject: [PATCH 11/55] Add cache --- .../cli-kit/src/private/node/conf-store.ts | 2 ++ .../src/public/node/notifications-system.ts | 30 +++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/cli-kit/src/private/node/conf-store.ts b/packages/cli-kit/src/private/node/conf-store.ts index 61fa23877c..a423830091 100644 --- a/packages/cli-kit/src/private/node/conf-store.ts +++ b/packages/cli-kit/src/private/node/conf-store.ts @@ -8,10 +8,12 @@ interface CacheValue { export type IntrospectionUrlKey = `identity-introspection-url-${string}` export type PackageVersionKey = `npm-package-${string}` +export type NotificationsKey = `notifications-${string}` interface Cache { [introspectionUrlKey: IntrospectionUrlKey]: CacheValue [packageVersionKey: PackageVersionKey]: CacheValue + [notifications: NotificationsKey]: CacheValue } export interface ConfSchema { diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 0a76ee3cd8..6229ee6b00 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -1,6 +1,9 @@ import {versionSatisfies} from './node-package-manager.js' import {renderError, renderInfo, renderWarning} from './ui.js' import {CLI_KIT_VERSION} from '../common/version.js' +import {NotificationsKey, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' + +const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json' interface Notifications { notifications: Notification[] @@ -27,10 +30,8 @@ export interface Notification { * @returns - A promise that resolves when the notifications have been shown. */ export async function showNotificationsIfNeeded(commandId: string, _surface?: string): Promise { - const response = await fetch('https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json') - const notifications = await (response.json() as Promise) - - const notificationsToShow = filterNotifications(notifications.notifications, commandId, new Date(), CLI_KIT_VERSION) + const notifications = await getNotifications() + const notificationsToShow = filterNotifications(notifications.notifications, commandId) notificationsToShow.forEach((notification) => { const content = { @@ -53,6 +54,23 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st }) } +/** + * Get notifications list from cache or fetch it if not present. + */ +async function getNotifications(): Promise { + const cacheKey: NotificationsKey = `notifications-${URL}` + const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, 24 * 3600 * 1000) + return JSON.parse(rawNotifications) +} + +/** + * Fetch notifications from GitHub. + */ +async function fetchNotifications(): Promise { + const response = await fetch(URL) + return response.text() as unknown as string +} + /** * Filters notifications based on the version of the CLI. * @@ -65,8 +83,8 @@ export async function showNotificationsIfNeeded(commandId: string, _surface?: st export function filterNotifications( notifications: Notification[], commandId: string, - today: Date, - currentVersion: string, + today: Date = new Date(), + currentVersion: string = CLI_KIT_VERSION, ): Notification[] { return notifications .filter((notification) => filterByVersion(notification, currentVersion)) From 45f23aa1faf8f4367c586c7b43dbef4deb2cc523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 26 Jun 2024 15:00:13 -0400 Subject: [PATCH 12/55] add more tests --- .../public/node/notifications-system.test.ts | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index d69f5d1c19..7caa9b9cce 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -64,6 +64,7 @@ const defaultInput = [ * @param output - The expected filtered notifications */ interface TestCase { + name: string input: Notification[] commandId: string version: string @@ -73,16 +74,57 @@ interface TestCase { const testCases: TestCase[] = [ { + name: 'all filters pass', + input: defaultInput, + commandId: 'app:dev', + version: '1.0.0', + date: '2000-02-01', + output: defaultInput, + }, + { + name: 'Only for app:info command, excludes notifications for explicit commands', + input: defaultInput, + commandId: 'app:info', + version: '1.0.0', + date: '2000-02-01', + output: [betweenVersins1and2, betweenDatesIn2000, fromVersion1, upToVersion2, fromDateJan2000, upToDateDec2000], + }, + { + name: 'Notifications for version 2.1.0', + input: defaultInput, + commandId: 'app:info', + version: '2.1.0', + date: '2000-02-01', + output: [betweenDatesIn2000, fromVersion1, fromDateJan2000, upToDateDec2000], + }, + { + name: 'Notifications for year 9999', input: defaultInput, commandId: 'app:info', - version: '1.0', - date: '2024-01-01', - output: [betweenVersins1and2, betweenDatesIn2000], + version: '1.0.0', + date: '9999-02-01', + output: [betweenVersins1and2, fromVersion1, upToVersion2, fromDateJan2000], + }, + { + name: 'Notifications for version 1.5, in year 1990', + input: defaultInput, + commandId: 'app:info', + version: '1.5.0', + date: '1990-02-01', + output: [betweenVersins1and2, fromVersion1, upToVersion2, upToDateDec2000], + }, + { + name: 'Notifications for version 2.1, and year 2024 and dev command', + input: defaultInput, + commandId: 'app:dev', + version: '2.1.0', + date: '2024-02-01', + output: [fromVersion1, fromDateJan2000, onlyForDevCommand], }, ] describe('notifications-system filter notifications', () => { - test.each(testCases)('Filters', ({input, commandId, version, date, output}) => { + test.each(testCases)('Filter for %name', ({input, commandId, version, date, output}) => { // When const result = filterNotifications(input, commandId, new Date(date), version) From dd6b5340080b9eb182548e529e169a5e7c98068b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 26 Jun 2024 15:06:00 -0400 Subject: [PATCH 13/55] Filter by surface --- .../public/node/notifications-system.test.ts | 38 +++++++++++++++---- .../src/public/node/notifications-system.ts | 13 +++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index 7caa9b9cce..a79716d891 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -45,6 +45,18 @@ const onlyForDevCommand: Notification = { commands: ['app:dev'], } +const onlyForThemeSurface: Notification = { + message: 'message', + type: 'info', + surface: 'theme', +} + +const unknownSurface: Notification = { + message: 'message', + type: 'info', + surface: 'unknown', +} + const defaultInput = [ betweenVersins1and2, betweenDatesIn2000, @@ -53,6 +65,8 @@ const defaultInput = [ fromDateJan2000, upToDateDec2000, onlyForDevCommand, + onlyForThemeSurface, + unknownSurface, ] /** @@ -73,14 +87,6 @@ interface TestCase { } const testCases: TestCase[] = [ - { - name: 'all filters pass', - input: defaultInput, - commandId: 'app:dev', - version: '1.0.0', - date: '2000-02-01', - output: defaultInput, - }, { name: 'Only for app:info command, excludes notifications for explicit commands', input: defaultInput, @@ -121,6 +127,22 @@ const testCases: TestCase[] = [ date: '2024-02-01', output: [fromVersion1, fromDateJan2000, onlyForDevCommand], }, + { + name: 'Notifications for theme surface', + input: defaultInput, + commandId: 'theme:dev', + version: '2.1.0', + date: '2024-02-01', + output: [fromVersion1, fromDateJan2000, onlyForThemeSurface], + }, + { + name: 'Notifications for unknown surface is never shown', + input: defaultInput, + commandId: 'version', + version: '2.1.0', + date: '2024-02-01', + output: [fromVersion1, fromDateJan2000], + }, ] describe('notifications-system filter notifications', () => { diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 6229ee6b00..dfbbc07e96 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -86,10 +86,12 @@ export function filterNotifications( today: Date = new Date(), currentVersion: string = CLI_KIT_VERSION, ): Notification[] { + const surface = commandId.split(':')[0] ?? 'all' return notifications .filter((notification) => filterByVersion(notification, currentVersion)) .filter((notifications) => filterByDate(notifications, today)) .filter((notification) => filterByCommand(notification, commandId)) + .filter((notification) => filterBySurface(notification, surface)) } /** @@ -126,3 +128,14 @@ function filterByDate(notification: Notification, today: Date) { function filterByCommand(notification: Notification, commandId: string) { return !notification.commands || notification.commands.includes(commandId) } + +/** + * Filters notifications based on the surface. + * + * @param notification - The notification to filter. + * @param surface - The surface to filter by. + * @returns - A boolean indicating whether the notification should be shown. + */ +function filterBySurface(notification: Notification, surface: string) { + return !notification.surface || notification.surface === surface +} From 03fa7cefae863326876e6eaaaa7b5fd99b1b654e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 09:52:22 -0400 Subject: [PATCH 14/55] Filter by surface in app loader --- packages/app/src/cli/models/app/loader.ts | 5 +++++ .../src/public/node/notifications-system.ts | 21 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 3db353645b..26d7b5c866 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -49,6 +49,7 @@ import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array' import {checkIfIgnoredInGitRepository} from '@shopify/cli-kit/node/git' import {renderInfo} from '@shopify/cli-kit/node/ui' import {currentProcessIsGlobal} from '@shopify/cli-kit/node/is-global' +import {showNotificationsIfNeeded} from '@shopify/cli-kit/node/notifications-system' const defaultExtensionDirectory = 'extensions/*' @@ -303,6 +304,10 @@ class AppLoader module.type) + await showNotificationsIfNeeded('app', extensionTypes) + if (!this.errors.isEmpty()) appClass.errors = this.errors await logMetadataForLoadedApp(appClass, { diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index dfbbc07e96..fcaab7a9f2 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -26,12 +26,15 @@ export interface Notification { * Shows notifications to the user if they meet the criteria specified in the notifications.json file. * * @param commandId - The command ID that triggered the notifications. - * @param _surface - The surface that triggered the notifications. + * @param availableSurfaces - The surfaces present in the current project (usually for app extensions). * @returns - A promise that resolves when the notifications have been shown. */ -export async function showNotificationsIfNeeded(commandId: string, _surface?: string): Promise { +export async function showNotificationsIfNeeded( + commandId: string, + availableSurfaces: string[] = ['app', 'theme', 'hydrogen'], +): Promise { const notifications = await getNotifications() - const notificationsToShow = filterNotifications(notifications.notifications, commandId) + const notificationsToShow = filterNotifications(notifications.notifications, commandId, availableSurfaces) notificationsToShow.forEach((notification) => { const content = { @@ -76,6 +79,7 @@ async function fetchNotifications(): Promise { * * @param notifications - The notifications to filter. * @param commandId - The command ID to filter by. + * @param availableSurfaces - The surfaces present in the current project (usually for app extensions). * @param today - The current date. * @param currentVersion - The current version of the CLI. * @returns - The filtered notifications. @@ -83,15 +87,16 @@ async function fetchNotifications(): Promise { export function filterNotifications( notifications: Notification[], commandId: string, + availableSurfaces: string[], today: Date = new Date(), currentVersion: string = CLI_KIT_VERSION, ): Notification[] { - const surface = commandId.split(':')[0] ?? 'all' + const mainSurface = commandId.split(':')[0] ?? 'all' return notifications .filter((notification) => filterByVersion(notification, currentVersion)) .filter((notifications) => filterByDate(notifications, today)) .filter((notification) => filterByCommand(notification, commandId)) - .filter((notification) => filterBySurface(notification, surface)) + .filter((notification) => filterBySurface(notification, availableSurfaces ?? [mainSurface])) } /** @@ -133,9 +138,9 @@ function filterByCommand(notification: Notification, commandId: string) { * Filters notifications based on the surface. * * @param notification - The notification to filter. - * @param surface - The surface to filter by. + * @param availableSurfaces - The surfaces present in the current project (usually for app extensions). * @returns - A boolean indicating whether the notification should be shown. */ -function filterBySurface(notification: Notification, surface: string) { - return !notification.surface || notification.surface === surface +function filterBySurface(notification: Notification, availableSurfaces: string[]) { + return !notification.surface || availableSurfaces.includes(notification.surface) } From 8232c7ded6317c9310fc2beeaed967664b80ae3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 11:03:13 -0400 Subject: [PATCH 15/55] Surface filter for apps --- .../public/node/notifications-system.test.ts | 23 +++++++++++++-- .../src/public/node/notifications-system.ts | 28 ++++++++++--------- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index a79716d891..f84d610386 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -57,6 +57,12 @@ const unknownSurface: Notification = { surface: 'unknown', } +const extensionSurface: Notification = { + message: 'message', + type: 'info', + surface: 'ui-extension', +} + const defaultInput = [ betweenVersins1and2, betweenDatesIn2000, @@ -67,8 +73,11 @@ const defaultInput = [ onlyForDevCommand, onlyForThemeSurface, unknownSurface, + extensionSurface, ] +const defaultSurfaces = ['app', 'theme', 'hydrogen'] + /** * Represents a test case * @param input - the initial notifications received from remote @@ -83,6 +92,7 @@ interface TestCase { commandId: string version: string date: string + surfaces?: string[] output: Notification[] } @@ -143,12 +153,21 @@ const testCases: TestCase[] = [ date: '2024-02-01', output: [fromVersion1, fromDateJan2000], }, + { + name: 'Notifications for extension type surface is shown', + input: defaultInput, + commandId: 'version', + version: '2.1.0', + date: '2024-02-01', + surfaces: ['ui-extension', 'function'], + output: [extensionSurface], + }, ] describe('notifications-system filter notifications', () => { - test.each(testCases)('Filter for %name', ({input, commandId, version, date, output}) => { + test.each(testCases)('Filter for %name', ({input, commandId, version, date, surfaces, output}) => { // When - const result = filterNotifications(input, commandId, new Date(date), version) + const result = filterNotifications(input, commandId, surfaces, new Date(date), version) // Then expect(result).toEqual(output) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index fcaab7a9f2..60aa3d31c8 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -26,15 +26,12 @@ export interface Notification { * Shows notifications to the user if they meet the criteria specified in the notifications.json file. * * @param commandId - The command ID that triggered the notifications. - * @param availableSurfaces - The surfaces present in the current project (usually for app extensions). + * @param currentSurfaces - The surfaces present in the current project (usually for app extensions). * @returns - A promise that resolves when the notifications have been shown. */ -export async function showNotificationsIfNeeded( - commandId: string, - availableSurfaces: string[] = ['app', 'theme', 'hydrogen'], -): Promise { +export async function showNotificationsIfNeeded(commandId: string, currentSurfaces?: string[]): Promise { const notifications = await getNotifications() - const notificationsToShow = filterNotifications(notifications.notifications, commandId, availableSurfaces) + const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) notificationsToShow.forEach((notification) => { const content = { @@ -79,7 +76,7 @@ async function fetchNotifications(): Promise { * * @param notifications - The notifications to filter. * @param commandId - The command ID to filter by. - * @param availableSurfaces - The surfaces present in the current project (usually for app extensions). + * @param currentSurfaces - The surfaces present in the current project (usually for app extensions). * @param today - The current date. * @param currentVersion - The current version of the CLI. * @returns - The filtered notifications. @@ -87,16 +84,15 @@ async function fetchNotifications(): Promise { export function filterNotifications( notifications: Notification[], commandId: string, - availableSurfaces: string[], + currentSurfaces?: string[], today: Date = new Date(), currentVersion: string = CLI_KIT_VERSION, ): Notification[] { - const mainSurface = commandId.split(':')[0] ?? 'all' return notifications .filter((notification) => filterByVersion(notification, currentVersion)) .filter((notifications) => filterByDate(notifications, today)) .filter((notification) => filterByCommand(notification, commandId)) - .filter((notification) => filterBySurface(notification, availableSurfaces ?? [mainSurface])) + .filter((notification) => filterBySurface(notification, commandId, currentSurfaces)) } /** @@ -138,9 +134,15 @@ function filterByCommand(notification: Notification, commandId: string) { * Filters notifications based on the surface. * * @param notification - The notification to filter. - * @param availableSurfaces - The surfaces present in the current project (usually for app extensions). + * @param commandId - The command id. + * @param surfacesFromContext - The surfaces present in the current project (usually for app extensions). * @returns - A boolean indicating whether the notification should be shown. */ -function filterBySurface(notification: Notification, availableSurfaces: string[]) { - return !notification.surface || availableSurfaces.includes(notification.surface) +function filterBySurface(notification: Notification, commandId: string, surfacesFromContext?: string[]) { + const surfaceFromCommand = commandId.split(':')[0] ?? 'all' + const notificationSurface = notification.surface ?? 'all' + + if (surfacesFromContext) return surfacesFromContext.includes(notificationSurface) + + return notificationSurface === surfaceFromCommand || notificationSurface === 'all' } From 8c9aa0566b2f8aedf30705d2718fbdf6b0de18da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 11:07:11 -0400 Subject: [PATCH 16/55] Test with ui-extensino surface --- notifications.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/notifications.json b/notifications.json index 271931f12a..ee2bf7f77c 100644 --- a/notifications.json +++ b/notifications.json @@ -6,7 +6,8 @@ "message": "Check out shopify.com/editions/summer2024", "minDate": "2024-06-25", "minVersion": "3.62.0", - "commands": ["app:generate:extension"] + "commands": ["app:generate:extension"], + "surface": "ui-extension" } ] } From 4c0711a347c0ccc8028c6b9813d2df69a8601c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 11:08:18 -0400 Subject: [PATCH 17/55] update notification json --- notifications.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.json b/notifications.json index ee2bf7f77c..4b450c3b79 100644 --- a/notifications.json +++ b/notifications.json @@ -6,7 +6,7 @@ "message": "Check out shopify.com/editions/summer2024", "minDate": "2024-06-25", "minVersion": "3.62.0", - "commands": ["app:generate:extension"], + "commands": ["app:generate:extension", "app:dev", "app:build"], "surface": "ui-extension" } ] From 6f158596255e59c29c0336d182ebea7714946565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 11:15:06 -0400 Subject: [PATCH 18/55] update notification json --- notifications.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.json b/notifications.json index 4b450c3b79..0749ae1ca5 100644 --- a/notifications.json +++ b/notifications.json @@ -7,7 +7,7 @@ "minDate": "2024-06-25", "minVersion": "3.62.0", "commands": ["app:generate:extension", "app:dev", "app:build"], - "surface": "ui-extension" + "surface": "ui_extension" } ] } From 9d4ec89b8e3a49b4063c773f774a1d4b6e2ca93c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 11:53:04 -0400 Subject: [PATCH 19/55] Ignore commands when running notifications from loader --- packages/app/src/cli/models/app/loader.ts | 2 +- packages/cli-kit/src/public/node/notifications-system.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 26d7b5c866..dc3bec2f0b 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -306,7 +306,7 @@ class AppLoader module.type) - await showNotificationsIfNeeded('app', extensionTypes) + await showNotificationsIfNeeded('', extensionTypes) if (!this.errors.isEmpty()) appClass.errors = this.errors diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 60aa3d31c8..10fe03b7dd 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -127,6 +127,7 @@ function filterByDate(notification: Notification, today: Date) { * @returns - A boolean indicating whether the notification should be shown. */ function filterByCommand(notification: Notification, commandId: string) { + if (commandId === '') return true return !notification.commands || notification.commands.includes(commandId) } From 7747888eeeb6aae6e6f6e0861ca8cd6ef3776fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 14:08:28 -0400 Subject: [PATCH 20/55] Add more tests --- packages/app/src/cli/models/app/loader.ts | 2 +- .../cli-kit/src/public/node/base-command.ts | 4 +- .../cli-kit/src/public/node/global-context.ts | 36 +++++++++++ .../public/node/notifications-system.test.ts | 62 ++++++++++++++++++- .../src/public/node/notifications-system.ts | 5 +- 5 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 packages/cli-kit/src/public/node/global-context.ts diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index dc3bec2f0b..a13ac5675d 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -306,7 +306,7 @@ class AppLoader module.type) - await showNotificationsIfNeeded('', extensionTypes) + await showNotificationsIfNeeded(extensionTypes) if (!this.errors.isEmpty()) appClass.errors = this.errors diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index fadf00934b..9fb97698f5 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -9,6 +9,7 @@ import {terminalSupportsRawMode} from './system.js' import {hashString} from './crypto.js' import {isTruthy} from './context/utilities.js' import {showNotificationsIfNeeded} from './notifications-system.js' +import {setCurrentCommandId} from './global-context.js' import {JsonMap} from '../../private/common/json.js' import {underscore} from '../common/string.js' import {Command, Errors} from '@oclif/core' @@ -46,12 +47,13 @@ abstract class BaseCommand extends Command { // eslint-disable-next-line @typescript-eslint/no-explicit-any protected async init(): Promise { this.exitWithTimestampWhenEnvVariablePresent() + setCurrentCommandId(this.id!) if (!isDevelopment()) { // This function runs just prior to `run` await registerCleanBugsnagErrorsFromWithinPlugins(this.config) } this.showNpmFlagWarning() - await showNotificationsIfNeeded(this.id!) + await showNotificationsIfNeeded() return super.init() } diff --git a/packages/cli-kit/src/public/node/global-context.ts b/packages/cli-kit/src/public/node/global-context.ts new file mode 100644 index 0000000000..e56ed3c330 --- /dev/null +++ b/packages/cli-kit/src/public/node/global-context.ts @@ -0,0 +1,36 @@ +export interface GlobalContext { + currentCommandId: string + [key: string]: string +} + +let _globalContext: GlobalContext | undefined + +/** + * Get the global context. + * + * @returns Global context. + */ +function getGlobalContext(): GlobalContext { + if (!_globalContext) { + _globalContext = {currentCommandId: ''} + } + return _globalContext +} + +/** + * Get the current command ID. + * + * @returns Current command ID. + */ +export function getCurrentCommandId(): string { + return getGlobalContext().currentCommandId +} + +/** + * Set the current command ID. + * + * @param commandId - Command ID. + */ +export function setCurrentCommandId(commandId: string): void { + getGlobalContext().currentCommandId = commandId +} diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index f84d610386..373b024192 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -1,5 +1,10 @@ -import {Notification, filterNotifications} from './notifications-system.js' -import {describe, expect, test} from 'vitest' +import {Notification, filterNotifications, showNotificationsIfNeeded} from './notifications-system.js' +import {renderError, renderInfo, renderWarning} from './ui.js' +import {cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' +import {describe, expect, test, vi} from 'vitest' + +vi.mock('./ui.js') +vi.mock('../../private/node/conf-store.js') const betweenVersins1and2: Notification = { message: 'message', @@ -63,6 +68,21 @@ const extensionSurface: Notification = { surface: 'ui-extension', } +const infoNotification: Notification = { + message: 'message', + type: 'info', +} + +const errorNotification: Notification = { + message: 'message', + type: 'error', +} + +const warningNotification: Notification = { + message: 'message', + type: 'warning', +} + const defaultInput = [ betweenVersins1and2, betweenDatesIn2000, @@ -173,3 +193,41 @@ describe('notifications-system filter notifications', () => { expect(result).toEqual(output) }) }) + +describe('notifications-system', () => { + test('an info notification triggers a renderInfo call', async () => { + // Given + const notifications = [infoNotification] + vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications})) + + // When + await showNotificationsIfNeeded() + + // Then + expect(renderInfo).toHaveBeenCalled() + }) + + test('a warning notification triggers a renderWarning call', async () => { + // Given + const notifications = [warningNotification] + vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications})) + + // When + await showNotificationsIfNeeded() + + // Then + expect(renderWarning).toHaveBeenCalled() + }) + + test('an error notification triggers a renderError call', async () => { + // Given + const notifications = [errorNotification] + vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications})) + + // When + await showNotificationsIfNeeded() + + // Then + expect(renderError).toHaveBeenCalled() + }) +}) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 10fe03b7dd..494ccb1308 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -1,5 +1,6 @@ import {versionSatisfies} from './node-package-manager.js' import {renderError, renderInfo, renderWarning} from './ui.js' +import {getCurrentCommandId} from './global-context.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' @@ -25,12 +26,12 @@ export interface Notification { /** * Shows notifications to the user if they meet the criteria specified in the notifications.json file. * - * @param commandId - The command ID that triggered the notifications. * @param currentSurfaces - The surfaces present in the current project (usually for app extensions). * @returns - A promise that resolves when the notifications have been shown. */ -export async function showNotificationsIfNeeded(commandId: string, currentSurfaces?: string[]): Promise { +export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Promise { const notifications = await getNotifications() + const commandId = getCurrentCommandId() const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) notificationsToShow.forEach((notification) => { From c95253447e1d58e16c84907a69eca2059f91dfc8 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 27 Jun 2024 14:13:02 -0400 Subject: [PATCH 21/55] Add filter by frequency --- notifications.json | 1 + .../cli-kit/src/private/node/conf-store.ts | 13 +++ .../public/node/notifications-system.test.ts | 107 +++++++++++++++++- .../src/public/node/notifications-system.ts | 47 +++++++- 4 files changed, 162 insertions(+), 6 deletions(-) diff --git a/notifications.json b/notifications.json index 0749ae1ca5..7605cb3b73 100644 --- a/notifications.json +++ b/notifications.json @@ -1,6 +1,7 @@ { "notifications": [ { + "id": "editions-summer24", "type": "info", "title": "Editions is here!", "message": "Check out shopify.com/editions/summer2024", diff --git a/packages/cli-kit/src/private/node/conf-store.ts b/packages/cli-kit/src/private/node/conf-store.ts index a423830091..4765db8449 100644 --- a/packages/cli-kit/src/private/node/conf-store.ts +++ b/packages/cli-kit/src/private/node/conf-store.ts @@ -9,11 +9,13 @@ interface CacheValue { export type IntrospectionUrlKey = `identity-introspection-url-${string}` export type PackageVersionKey = `npm-package-${string}` export type NotificationsKey = `notifications-${string}` +export type NotificationKey = `notification-${string}` interface Cache { [introspectionUrlKey: IntrospectionUrlKey]: CacheValue [packageVersionKey: PackageVersionKey]: CacheValue [notifications: NotificationsKey]: CacheValue + [notification: NotificationKey]: CacheValue } export interface ConfSchema { @@ -91,3 +93,14 @@ export async function cacheRetrieveOrRepopulate( config.set('cache', cache) return value } + +export function getCache(key: keyof Cache, config = cliKitStore()): string | undefined { + const cache: Cache = config.get('cache') || {} + return cache[key]?.value +} + +export function setCache(key: keyof Cache, value: string, config = cliKitStore()): void { + const cache: Cache = config.get('cache') || {} + cache[key] = {value, timestamp: Date.now()} + config.set('cache', cache) +} diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index f84d610386..0634f681d9 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -1,7 +1,11 @@ import {Notification, filterNotifications} from './notifications-system.js' -import {describe, expect, test} from 'vitest' +import {getCache} from '../../private/node/conf-store.js' +import {afterEach, describe, expect, test, vi} from 'vitest' + +vi.mock('../../private/node/conf-store.js') const betweenVersins1and2: Notification = { + id: 'betweenVersins1and2', message: 'message', type: 'info', minVersion: '1.0', @@ -9,6 +13,7 @@ const betweenVersins1and2: Notification = { } const betweenDatesIn2000: Notification = { + id: 'betweenDatesIn2000', message: 'message', type: 'info', minDate: '2000-01-01', @@ -16,53 +21,89 @@ const betweenDatesIn2000: Notification = { } const fromVersion1: Notification = { + id: 'fromVersion1', message: 'message', type: 'info', minVersion: '1.0', } const upToVersion2: Notification = { + id: 'upToVersion2', message: 'message', type: 'info', maxVersion: '2.0', } const fromDateJan2000: Notification = { + id: 'fromDateJan2000', message: 'message', type: 'info', minDate: '2000-01-01', } const upToDateDec2000: Notification = { + id: 'upToDateDec2000', message: 'message', type: 'info', maxDate: '2000-12-31', } const onlyForDevCommand: Notification = { + id: 'onlyForDevCommand', message: 'message', type: 'info', commands: ['app:dev'], } const onlyForThemeSurface: Notification = { + id: 'onlyForThemeSurface', message: 'message', type: 'info', surface: 'theme', } const unknownSurface: Notification = { + id: 'unknownSurface', message: 'message', type: 'info', surface: 'unknown', } const extensionSurface: Notification = { + id: 'extensionSurface', message: 'message', type: 'info', surface: 'ui-extension', } +const showOnce: Notification = { + id: 'showOnce', + message: 'message', + type: 'info', + frequency: 'once', +} + +const showOnceADay: Notification = { + id: 'showOnceADay', + message: 'message', + type: 'info', + frequency: 'once_a_day', +} + +const showOnceAWeek: Notification = { + id: 'showOnceAWeek', + message: 'message', + type: 'info', + frequency: 'once_a_week', +} + +const showAlways: Notification = { + id: 'showAlways', + message: 'message', + type: 'info', + frequency: 'always', +} + const defaultInput = [ betweenVersins1and2, betweenDatesIn2000, @@ -164,6 +205,11 @@ const testCases: TestCase[] = [ }, ] +afterEach(() => { + // Restore Date mock + vi.useRealTimers() +}) + describe('notifications-system filter notifications', () => { test.each(testCases)('Filter for %name', ({input, commandId, version, date, surfaces, output}) => { // When @@ -172,4 +218,63 @@ describe('notifications-system filter notifications', () => { // Then expect(result).toEqual(output) }) + + test('Filter for frequency with always', async () => { + // Given + const current = new Date('2020-01-15T00:00:00.000Z') + const yesterday = new Date('2020-01-14T08:00:00.000Z') + vi.setSystemTime(current) + vi.mocked(getCache).mockReturnValue(yesterday.getTime().toString()) + + // When + const result = filterNotifications([showAlways], 'version') + + // Then + expect(result).toEqual([showAlways]) + }) + + test('Filter for frequency with once', async () => { + // Given + const current = new Date('2020-01-15T00:00:00.000Z') + vi.setSystemTime(current) + vi.mocked(getCache).mockReturnValueOnce(undefined) + vi.mocked(getCache).mockReturnValueOnce(current.getTime().toString()) + + // When/Then + const result = filterNotifications([showOnce], 'version') + expect(result).toEqual([showOnce]) + const result2 = filterNotifications([showOnce], 'version') + expect(result2).toEqual([]) + }) + + test('Filter for frequency with once_a_day', async () => { + // Given + const current = new Date('2020-01-15T08:00:00.000Z') + const yesterday = new Date('2020-01-14T00:00:00.000Z') + vi.setSystemTime(current) + vi.mocked(getCache).mockReturnValueOnce(yesterday.getTime().toString()) + vi.mocked(getCache).mockReturnValueOnce(current.getTime().toString()) + + // When/Then + const result = filterNotifications([showOnceADay], 'version') + expect(result).toEqual([showOnceADay]) + const result2 = filterNotifications([showOnceADay], 'version') + expect(result2).toEqual([]) + }) + + test('Filter for frequency with once_a_week', async () => { + // Given + const current = new Date('2020-01-15T08:00:00.000Z') + const yesterday = new Date('2020-01-14T08:00:00.000Z') + const lastWeek = new Date('2020-01-03T00:00:00.000Z') + vi.setSystemTime(current) + vi.mocked(getCache).mockReturnValueOnce(lastWeek.getTime().toString()) + vi.mocked(getCache).mockReturnValueOnce(yesterday.getTime().toString()) + + // When/Then + const result = filterNotifications([showOnceAWeek], 'version') + expect(result).toEqual([showOnceAWeek]) + const result2 = filterNotifications([showOnceAWeek], 'version') + expect(result2).toEqual([]) + }) }) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 10fe03b7dd..a8039ddb64 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -1,7 +1,7 @@ import {versionSatisfies} from './node-package-manager.js' import {renderError, renderInfo, renderWarning} from './ui.js' import {CLI_KIT_VERSION} from '../common/version.js' -import {NotificationsKey, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' +import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json' @@ -10,6 +10,7 @@ interface Notifications { } export interface Notification { + id: string message: string type: 'info' | 'warning' | 'error' title?: string @@ -19,7 +20,7 @@ export interface Notification { maxDate?: string commands?: string[] surface?: 'app' | 'theme' | 'hydrogen' | string - frequency?: 'always' | 'once_a_day' | 'once_a_week' + frequency?: 'always' | 'once' | 'once_a_day' | 'once_a_week' } /** @@ -32,8 +33,15 @@ export interface Notification { export async function showNotificationsIfNeeded(commandId: string, currentSurfaces?: string[]): Promise { const notifications = await getNotifications() const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) + await renderNotifications(notificationsToShow) +} - notificationsToShow.forEach((notification) => { +/** + * + * @param notifications + */ +async function renderNotifications(notifications: Notification[]) { + notifications.forEach((notification) => { const content = { headline: notification.title, body: notification.message, @@ -41,16 +49,17 @@ export async function showNotificationsIfNeeded(commandId: string, currentSurfac switch (notification.type) { case 'info': { renderInfo(content) - return + break } case 'warning': { renderWarning(content) - return + break } case 'error': { renderError(content) } } + setCache(`notification-${notification.id}`, new Date().getTime().toString()) }) } @@ -93,6 +102,7 @@ export function filterNotifications( .filter((notifications) => filterByDate(notifications, today)) .filter((notification) => filterByCommand(notification, commandId)) .filter((notification) => filterBySurface(notification, commandId, currentSurfaces)) + .filter((notification) => filterByFrequency(notification)) } /** @@ -147,3 +157,30 @@ function filterBySurface(notification: Notification, commandId: string, surfaces return notificationSurface === surfaceFromCommand || notificationSurface === 'all' } + +/** + * Filters notifications based on the frequency. + * + * @param notification - The notification to filter. + * @returns - A boolean indicating whether the notification should be shown. + */ +function filterByFrequency(notification: Notification): boolean { + if (!notification.frequency) return true + const lastShown = getCache(`notification-${notification.id}`) as unknown as string + + switch (notification.frequency) { + case 'always': { + return true + } + case 'once': { + const result = !lastShown + return result + } + case 'once_a_day': { + return new Date().getTime() - Number(lastShown) > 24 * 3600 * 1000 + } + case 'once_a_week': { + return new Date().getTime() - Number(lastShown) > 7 * 24 * 3600 * 1000 + } + } +} From 00ccbb1188c526f133f162bebd8ca88d22ce60b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 14:51:48 -0400 Subject: [PATCH 22/55] Add support for CTA links --- notifications.json | 4 ++++ packages/cli-kit/src/public/node/notifications-system.ts | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/notifications.json b/notifications.json index 7605cb3b73..bfefa66811 100644 --- a/notifications.json +++ b/notifications.json @@ -5,6 +5,10 @@ "type": "info", "title": "Editions is here!", "message": "Check out shopify.com/editions/summer2024", + "cta": { + "label": "Learn more", + "url": "https://shopify.com/editions/summer2024" + }, "minDate": "2024-06-25", "minVersion": "3.62.0", "commands": ["app:generate:extension", "app:dev", "app:build"], diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index c1f6075d76..4041e02f67 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -13,6 +13,10 @@ interface Notifications { export interface Notification { id: string message: string + cta?: { + label: string + url: string + } type: 'info' | 'warning' | 'error' title?: string minVersion?: string @@ -38,14 +42,16 @@ export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Pro } /** + * Renders the notifications to the user. * - * @param notifications + * @param notifications - The notifications to render. */ async function renderNotifications(notifications: Notification[]) { notifications.forEach((notification) => { const content = { headline: notification.title, body: notification.message, + link: notification.cta, } switch (notification.type) { case 'info': { From f5a970431c52cd6e9047d32a149429de6fa5195b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 27 Jun 2024 15:02:26 -0400 Subject: [PATCH 23/55] new notification --- notifications.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/notifications.json b/notifications.json index bfefa66811..15a731326d 100644 --- a/notifications.json +++ b/notifications.json @@ -10,9 +10,7 @@ "url": "https://shopify.com/editions/summer2024" }, "minDate": "2024-06-25", - "minVersion": "3.62.0", - "commands": ["app:generate:extension", "app:dev", "app:build"], - "surface": "ui_extension" + "minVersion": "3.62.0" } ] } From 8eb680099cf4790dbabf41ab1ca789c2c59b8b63 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Thu, 27 Jun 2024 16:33:12 -0400 Subject: [PATCH 24/55] List and generate commands --- .../src/public/node/notifications-system.ts | 40 +++++++- packages/cli/oclif.manifest.json | 38 ++++++++ .../cli/commands/notifications/generate.ts | 91 +++++++++++++++++++ .../src/cli/commands/notifications/list.ts | 45 +++++++++ packages/cli/src/index.ts | 4 + 5 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/cli/commands/notifications/generate.ts create mode 100644 packages/cli/src/cli/commands/notifications/list.ts diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 4041e02f67..436e8a5c17 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -1,12 +1,13 @@ import {versionSatisfies} from './node-package-manager.js' import {renderError, renderInfo, renderWarning} from './ui.js' import {getCurrentCommandId} from './global-context.js' +import {fileExists, readFile} from './fs.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json' -interface Notifications { +export interface Notifications { notifications: Notification[] } @@ -72,8 +73,10 @@ async function renderNotifications(notifications: Notification[]) { /** * Get notifications list from cache or fetch it if not present. + * + * @returns A Notifications object. */ -async function getNotifications(): Promise { +export async function getNotifications(): Promise { const cacheKey: NotificationsKey = `notifications-${URL}` const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, 24 * 3600 * 1000) return JSON.parse(rawNotifications) @@ -191,3 +194,36 @@ function filterByFrequency(notification: Notification): boolean { } } } + +/** + * Returns a string with the filters from a notification, one by line. + * + * @param notification - The notification to get the filters from. + * @returns A string with human-readable filters from the notification. + */ +export function stringifyFilters(notification: Notification): string { + const filters = [] + if (notification.minDate) filters.push(`from ${notification.minDate}`) + if (notification.maxDate) filters.push(`to ${notification.maxDate}`) + if (notification.minVersion) filters.push(`from v${notification.minVersion}`) + if (notification.maxVersion) filters.push(`to v${notification.maxVersion}`) + if (notification.frequency === 'once') filters.push('show only once') + if (notification.frequency === 'once_a_day') filters.push('show once a day') + if (notification.frequency === 'once_a_week') filters.push('show once a week') + if (notification.surface) filters.push(`surface = ${notification.surface}`) + if (notification.commands) filters.push(`commands = ${notification.commands.join(', ')}`) + return filters.join('\n') +} + +/** + * Reads the notifications from the local file. + * + * @returns A Notifications object. + */ +export async function getLocalNotifications(): Promise { + const filePath = './notifications.json' + if (!(await fileExists(filePath))) return {notifications: []} + + const rawNotifications = await readFile(filePath) + return JSON.parse(rawNotifications) +} diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 779a56473b..7c04825b3b 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -3890,6 +3890,44 @@ "pluginType": "core", "strict": true }, + "notifications:generate": { + "aliases": [ + ], + "args": { + }, + "description": "Generate a new notification for the the CLI.", + "enableJsonFlag": false, + "flags": { + }, + "hasDynamicHelp": false, + "hidden": true, + "hiddenAliases": [ + ], + "id": "notifications:generate", + "pluginAlias": "@shopify/cli", + "pluginName": "@shopify/cli", + "pluginType": "core", + "strict": true + }, + "notifications:list": { + "aliases": [ + ], + "args": { + }, + "description": "List current notifications configured for the CLI.", + "enableJsonFlag": false, + "flags": { + }, + "hasDynamicHelp": false, + "hidden": true, + "hiddenAliases": [ + ], + "id": "notifications:list", + "pluginAlias": "@shopify/cli", + "pluginName": "@shopify/cli", + "pluginType": "core", + "strict": true + }, "plugins": { "aliases": [ ], diff --git a/packages/cli/src/cli/commands/notifications/generate.ts b/packages/cli/src/cli/commands/notifications/generate.ts new file mode 100644 index 0000000000..5022aae859 --- /dev/null +++ b/packages/cli/src/cli/commands/notifications/generate.ts @@ -0,0 +1,91 @@ +import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' +import Command from '@shopify/cli-kit/node/base-command' +import {writeFile} from '@shopify/cli-kit/node/fs' +import {Notification, Notifications, getLocalNotifications} from '@shopify/cli-kit/node/notifications-system' +import {renderSelectPrompt, renderSuccess, renderTextPrompt} from '@shopify/cli-kit/node/ui' +import {randomUUID} from 'crypto' + +export default class Generate extends Command { + static description = 'Generate a new notification for the the CLI.' + + static hidden = true + + async run(): Promise { + const today = new Date() + const formattedToday = `${today.getFullYear()}-${(today.getMonth() + 1).toString().padStart(2, '0')}-${today + .getDate() + .toString() + .padStart(2, '0')}` + const id = randomUUID() + const type: 'info' | 'warning' | 'error' = await renderSelectPrompt({ + message: 'Type of message?', + choices: [ + {label: 'Info', value: 'info'}, + {label: 'Warning', value: 'warning'}, + {label: 'Error', value: 'error'}, + ], + }) + const title = await renderTextPrompt({ + message: 'Title', + }) + const message = await renderTextPrompt({ + message: 'Message', + }) + const frequency: 'always' | 'once' | 'once_a_day' | 'once_a_week' = await renderSelectPrompt({ + message: 'Frequency', + choices: [ + {label: 'Always', value: 'always'}, + {label: 'Only once', value: 'once'}, + {label: 'Once a day', value: 'once_a_day'}, + {label: 'Once a week', value: 'once_a_week'}, + ], + }) + const minVersion = await renderTextPrompt({ + message: 'Minimum CLI version (optional)', + initialAnswer: CLI_KIT_VERSION, + allowEmpty: true, + }) + const maxVersion = await renderTextPrompt({ + message: 'Maximum CLI version (optional)', + initialAnswer: CLI_KIT_VERSION, + allowEmpty: true, + }) + const minDate = await renderTextPrompt({ + message: 'Minimum date in YYYY-MM-DD format (optional)', + initialAnswer: formattedToday, + allowEmpty: true, + }) + const maxDate = await renderTextPrompt({ + message: 'Maximum date in YYYY-MM-DD format (optional)', + initialAnswer: formattedToday, + allowEmpty: true, + }) + const surface = await renderTextPrompt({ + message: 'Surface. E.g.: app, theme, hydrogen, theme_app_extension... (optional)', + allowEmpty: true, + }) + const commands = await renderTextPrompt({ + message: 'Comma separated list of commands. E.g.: app:generate:extension (optional)', + allowEmpty: true, + }) + const notifications: Notifications = await getLocalNotifications() + + const notification: Notification = { + id, + type, + title, + frequency, + message, + minVersion: minVersion.length === 0 ? undefined : minVersion, + maxVersion: maxVersion.length === 0 ? undefined : maxVersion, + minDate: minDate.length === 0 ? undefined : minDate, + maxDate: maxDate.length === 0 ? undefined : maxDate, + surface: surface.length === 0 ? undefined : surface, + commands: commands.length === 0 ? undefined : commands.split(',').map((command) => command.trim()), + } + notifications.notifications.push(notification) + await writeFile('./notifications.json', JSON.stringify(notifications)) + + renderSuccess({headline: 'notifications.json file updated successfully.'}) + } +} diff --git a/packages/cli/src/cli/commands/notifications/list.ts b/packages/cli/src/cli/commands/notifications/list.ts new file mode 100644 index 0000000000..29a8f2409d --- /dev/null +++ b/packages/cli/src/cli/commands/notifications/list.ts @@ -0,0 +1,45 @@ +import Command from '@shopify/cli-kit/node/base-command' +import {Notifications, getLocalNotifications, stringifyFilters} from '@shopify/cli-kit/node/notifications-system' +import {outputInfo} from '@shopify/cli-kit/node/output' +import {renderTable} from '@shopify/cli-kit/node/ui' + +export default class List extends Command { + static description = 'List current notifications configured for the CLI.' + static hidden = true + + async run(): Promise { + const notifications: Notifications = await getLocalNotifications() + + const rows = notifications.notifications.map((notification) => { + return { + type: notification.type, + title: notification.title, + message: notification.message, + filters: stringifyFilters(notification), + } + }) + + renderTable({ + rows, + columns: { + type: { + header: 'Type', + color: 'dim', + }, + title: { + header: 'Title', + color: 'dim', + }, + message: { + header: 'Message', + color: 'dim', + }, + filters: { + header: 'Filters', + color: 'dim', + }, + }, + }) + outputInfo('\n') + } +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index cc1427c940..4000402f3e 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -14,6 +14,8 @@ import KitchenSinkStatic from './cli/commands/kitchen-sink/static.js' import KitchenSink from './cli/commands/kitchen-sink/index.js' import DocsGenerate from './cli/commands/docs/generate.js' import HelpCommand from './cli/commands/help.js' +import List from './cli/commands/notifications/list.js' +import Generate from './cli/commands/notifications/generate.js' import ThemeCommands from '@shopify/theme' import {COMMANDS as HydrogenCommands, HOOKS as HydrogenHooks} from '@shopify/cli-hydrogen' import {commands as AppCommands} from '@shopify/app' @@ -103,6 +105,8 @@ export const COMMANDS: any = { 'kitchen-sink:prompts': KitchenSinkPrompts, 'kitchen-sink:static': KitchenSinkStatic, 'docs:generate': DocsGenerate, + 'notifications:list': List, + 'notifications:generate': Generate, } export default runShopifyCLI From 017a901ce0a327f3a354a1eecf21a3cc822b7c4b Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 28 Jun 2024 10:06:43 -0400 Subject: [PATCH 25/55] Delete notifications.json --- notifications.json | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 notifications.json diff --git a/notifications.json b/notifications.json deleted file mode 100644 index 15a731326d..0000000000 --- a/notifications.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "notifications": [ - { - "id": "editions-summer24", - "type": "info", - "title": "Editions is here!", - "message": "Check out shopify.com/editions/summer2024", - "cta": { - "label": "Learn more", - "url": "https://shopify.com/editions/summer2024" - }, - "minDate": "2024-06-25", - "minVersion": "3.62.0" - } - ] -} From b0160b8b2e6a589bb72d37c1e87babcfbcf03a81 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 28 Jun 2024 10:12:41 -0400 Subject: [PATCH 26/55] Catch and ignore errors --- .../src/public/node/notifications-system.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 436e8a5c17..c43a39410e 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -2,6 +2,7 @@ import {versionSatisfies} from './node-package-manager.js' import {renderError, renderInfo, renderWarning} from './ui.js' import {getCurrentCommandId} from './global-context.js' import {fileExists, readFile} from './fs.js' +import {outputDebug} from './output.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' @@ -36,10 +37,15 @@ export interface Notification { * @returns - A promise that resolves when the notifications have been shown. */ export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Promise { - const notifications = await getNotifications() - const commandId = getCurrentCommandId() - const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) - await renderNotifications(notificationsToShow) + try { + const notifications = await getNotifications() + const commandId = getCurrentCommandId() + const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) + await renderNotifications(notificationsToShow) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error: unknown) { + outputDebug('Error retrieving notifications') + } } /** From 4e10b2067f64bdbdad9faa149ecbf231923870fc Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 28 Jun 2024 10:14:37 -0400 Subject: [PATCH 27/55] Fix URL --- packages/cli-kit/src/public/node/notifications-system.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index c43a39410e..30224f6b21 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -6,7 +6,7 @@ import {outputDebug} from './output.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' -const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications-sytem/notifications.json' +const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications/notifications.json' export interface Notifications { notifications: Notification[] From c490d08d123e56070060502f7114df31909fc4d6 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 3 Jul 2024 09:48:35 +0200 Subject: [PATCH 28/55] Reuse getCache and setCache --- packages/cli-kit/src/private/node/conf-store.ts | 12 +++++------- .../src/public/node/notifications-system.test.ts | 14 ++++++-------- .../src/public/node/notifications-system.ts | 6 +++--- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/packages/cli-kit/src/private/node/conf-store.ts b/packages/cli-kit/src/private/node/conf-store.ts index 4765db8449..62f4d68a02 100644 --- a/packages/cli-kit/src/private/node/conf-store.ts +++ b/packages/cli-kit/src/private/node/conf-store.ts @@ -9,7 +9,7 @@ interface CacheValue { export type IntrospectionUrlKey = `identity-introspection-url-${string}` export type PackageVersionKey = `npm-package-${string}` export type NotificationsKey = `notifications-${string}` -export type NotificationKey = `notification-${string}` +type NotificationKey = `notification-${string}` interface Cache { [introspectionUrlKey: IntrospectionUrlKey]: CacheValue @@ -81,22 +81,20 @@ export async function cacheRetrieveOrRepopulate( timeout?: number, config = cliKitStore(), ): Promise> { - const cache: Cache = config.get('cache') || {} - const cached = cache[key] + const cached = getCache(key, config) if (cached?.value !== undefined && (timeout === undefined || Date.now() - cached.timestamp < timeout)) { return cached.value } const value = await fn() - cache[key] = {value, timestamp: Date.now()} - config.set('cache', cache) + setCache(key, value, config) return value } -export function getCache(key: keyof Cache, config = cliKitStore()): string | undefined { +export function getCache(key: keyof Cache, config = cliKitStore()): CacheValue | undefined { const cache: Cache = config.get('cache') || {} - return cache[key]?.value + return cache[key] } export function setCache(key: keyof Cache, value: string, config = cliKitStore()): void { diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index 650a3c2f3c..a91ef18184 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -137,8 +137,6 @@ const defaultInput = [ extensionSurface, ] -const defaultSurfaces = ['app', 'theme', 'hydrogen'] - /** * Represents a test case * @param input - the initial notifications received from remote @@ -244,7 +242,7 @@ describe('notifications-system filter notifications', () => { const current = new Date('2020-01-15T00:00:00.000Z') const yesterday = new Date('2020-01-14T08:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValue(yesterday.getTime().toString()) + vi.mocked(getCache).mockReturnValue({value: yesterday.getTime().toString(), timestamp: 0}) // When const result = filterNotifications([showAlways], 'version') @@ -258,7 +256,7 @@ describe('notifications-system filter notifications', () => { const current = new Date('2020-01-15T00:00:00.000Z') vi.setSystemTime(current) vi.mocked(getCache).mockReturnValueOnce(undefined) - vi.mocked(getCache).mockReturnValueOnce(current.getTime().toString()) + vi.mocked(getCache).mockReturnValueOnce({value: current.getTime().toString(), timestamp: 0}) // When/Then const result = filterNotifications([showOnce], 'version') @@ -272,8 +270,8 @@ describe('notifications-system filter notifications', () => { const current = new Date('2020-01-15T08:00:00.000Z') const yesterday = new Date('2020-01-14T00:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValueOnce(yesterday.getTime().toString()) - vi.mocked(getCache).mockReturnValueOnce(current.getTime().toString()) + vi.mocked(getCache).mockReturnValueOnce({value: yesterday.getTime().toString(), timestamp: 0}) + vi.mocked(getCache).mockReturnValueOnce({value: current.getTime().toString(), timestamp: 0}) // When/Then const result = filterNotifications([showOnceADay], 'version') @@ -288,8 +286,8 @@ describe('notifications-system filter notifications', () => { const yesterday = new Date('2020-01-14T08:00:00.000Z') const lastWeek = new Date('2020-01-03T00:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValueOnce(lastWeek.getTime().toString()) - vi.mocked(getCache).mockReturnValueOnce(yesterday.getTime().toString()) + vi.mocked(getCache).mockReturnValueOnce({value: lastWeek.getTime().toString(), timestamp: 0}) + vi.mocked(getCache).mockReturnValueOnce({value: yesterday.getTime().toString(), timestamp: 0}) // When/Then const result = filterNotifications([showOnceAWeek], 'version') diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 30224f6b21..b30365989c 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -182,15 +182,15 @@ function filterBySurface(notification: Notification, commandId: string, surfaces */ function filterByFrequency(notification: Notification): boolean { if (!notification.frequency) return true - const lastShown = getCache(`notification-${notification.id}`) as unknown as string + const lastShown = getCache(`notification-${notification.id}`)?.value as unknown as string + if (!lastShown) return true switch (notification.frequency) { case 'always': { return true } case 'once': { - const result = !lastShown - return result + return false } case 'once_a_day': { return new Date().getTime() - Number(lastShown) > 24 * 3600 * 1000 From 3f537812a9acc26618a3d11cc85c5c2c3af3c198 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 3 Jul 2024 10:09:09 +0200 Subject: [PATCH 29/55] Extract commands logic to a service --- .../cli/commands/notifications/generate.ts | 83 +------------ .../src/cli/commands/notifications/list.ts | 38 +----- .../cli/services/commands/notifications.ts | 114 ++++++++++++++++++ 3 files changed, 118 insertions(+), 117 deletions(-) create mode 100644 packages/cli/src/cli/services/commands/notifications.ts diff --git a/packages/cli/src/cli/commands/notifications/generate.ts b/packages/cli/src/cli/commands/notifications/generate.ts index 5022aae859..cbed8e0ac1 100644 --- a/packages/cli/src/cli/commands/notifications/generate.ts +++ b/packages/cli/src/cli/commands/notifications/generate.ts @@ -1,9 +1,5 @@ -import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' +import {generate} from '../../services/commands/notifications.js' import Command from '@shopify/cli-kit/node/base-command' -import {writeFile} from '@shopify/cli-kit/node/fs' -import {Notification, Notifications, getLocalNotifications} from '@shopify/cli-kit/node/notifications-system' -import {renderSelectPrompt, renderSuccess, renderTextPrompt} from '@shopify/cli-kit/node/ui' -import {randomUUID} from 'crypto' export default class Generate extends Command { static description = 'Generate a new notification for the the CLI.' @@ -11,81 +7,6 @@ export default class Generate extends Command { static hidden = true async run(): Promise { - const today = new Date() - const formattedToday = `${today.getFullYear()}-${(today.getMonth() + 1).toString().padStart(2, '0')}-${today - .getDate() - .toString() - .padStart(2, '0')}` - const id = randomUUID() - const type: 'info' | 'warning' | 'error' = await renderSelectPrompt({ - message: 'Type of message?', - choices: [ - {label: 'Info', value: 'info'}, - {label: 'Warning', value: 'warning'}, - {label: 'Error', value: 'error'}, - ], - }) - const title = await renderTextPrompt({ - message: 'Title', - }) - const message = await renderTextPrompt({ - message: 'Message', - }) - const frequency: 'always' | 'once' | 'once_a_day' | 'once_a_week' = await renderSelectPrompt({ - message: 'Frequency', - choices: [ - {label: 'Always', value: 'always'}, - {label: 'Only once', value: 'once'}, - {label: 'Once a day', value: 'once_a_day'}, - {label: 'Once a week', value: 'once_a_week'}, - ], - }) - const minVersion = await renderTextPrompt({ - message: 'Minimum CLI version (optional)', - initialAnswer: CLI_KIT_VERSION, - allowEmpty: true, - }) - const maxVersion = await renderTextPrompt({ - message: 'Maximum CLI version (optional)', - initialAnswer: CLI_KIT_VERSION, - allowEmpty: true, - }) - const minDate = await renderTextPrompt({ - message: 'Minimum date in YYYY-MM-DD format (optional)', - initialAnswer: formattedToday, - allowEmpty: true, - }) - const maxDate = await renderTextPrompt({ - message: 'Maximum date in YYYY-MM-DD format (optional)', - initialAnswer: formattedToday, - allowEmpty: true, - }) - const surface = await renderTextPrompt({ - message: 'Surface. E.g.: app, theme, hydrogen, theme_app_extension... (optional)', - allowEmpty: true, - }) - const commands = await renderTextPrompt({ - message: 'Comma separated list of commands. E.g.: app:generate:extension (optional)', - allowEmpty: true, - }) - const notifications: Notifications = await getLocalNotifications() - - const notification: Notification = { - id, - type, - title, - frequency, - message, - minVersion: minVersion.length === 0 ? undefined : minVersion, - maxVersion: maxVersion.length === 0 ? undefined : maxVersion, - minDate: minDate.length === 0 ? undefined : minDate, - maxDate: maxDate.length === 0 ? undefined : maxDate, - surface: surface.length === 0 ? undefined : surface, - commands: commands.length === 0 ? undefined : commands.split(',').map((command) => command.trim()), - } - notifications.notifications.push(notification) - await writeFile('./notifications.json', JSON.stringify(notifications)) - - renderSuccess({headline: 'notifications.json file updated successfully.'}) + await generate() } } diff --git a/packages/cli/src/cli/commands/notifications/list.ts b/packages/cli/src/cli/commands/notifications/list.ts index 29a8f2409d..4d251ef63b 100644 --- a/packages/cli/src/cli/commands/notifications/list.ts +++ b/packages/cli/src/cli/commands/notifications/list.ts @@ -1,45 +1,11 @@ +import {list} from '../../services/commands/notifications.js' import Command from '@shopify/cli-kit/node/base-command' -import {Notifications, getLocalNotifications, stringifyFilters} from '@shopify/cli-kit/node/notifications-system' -import {outputInfo} from '@shopify/cli-kit/node/output' -import {renderTable} from '@shopify/cli-kit/node/ui' export default class List extends Command { static description = 'List current notifications configured for the CLI.' static hidden = true async run(): Promise { - const notifications: Notifications = await getLocalNotifications() - - const rows = notifications.notifications.map((notification) => { - return { - type: notification.type, - title: notification.title, - message: notification.message, - filters: stringifyFilters(notification), - } - }) - - renderTable({ - rows, - columns: { - type: { - header: 'Type', - color: 'dim', - }, - title: { - header: 'Title', - color: 'dim', - }, - message: { - header: 'Message', - color: 'dim', - }, - filters: { - header: 'Filters', - color: 'dim', - }, - }, - }) - outputInfo('\n') + await list() } } diff --git a/packages/cli/src/cli/services/commands/notifications.ts b/packages/cli/src/cli/services/commands/notifications.ts new file mode 100644 index 0000000000..352734146b --- /dev/null +++ b/packages/cli/src/cli/services/commands/notifications.ts @@ -0,0 +1,114 @@ +import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' +import {randomUUID} from '@shopify/cli-kit/node/crypto' +import {writeFile} from '@shopify/cli-kit/node/fs' +import { + Notifications, + getLocalNotifications, + Notification, + stringifyFilters, +} from '@shopify/cli-kit/node/notifications-system' +import {outputInfo} from '@shopify/cli-kit/node/output' +import {renderSelectPrompt, renderTextPrompt, renderSuccess, renderTable, TableColumn} from '@shopify/cli-kit/node/ui' + +export async function generate() { + const today = new Date() + const formattedToday = `${today.getFullYear()}-${(today.getMonth() + 1).toString().padStart(2, '0')}-${today + .getDate() + .toString() + .padStart(2, '0')}` + const id = randomUUID() + + const type: 'info' | 'warning' | 'error' = await renderSelectPrompt({ + message: 'Type of message?', + choices: [ + {label: 'Info', value: 'info'}, + {label: 'Warning', value: 'warning'}, + {label: 'Error', value: 'error'}, + ], + }) + const title = await renderTextPrompt({ + message: 'Title', + }) + const message = await renderTextPrompt({ + message: 'Message', + }) + const frequency: 'always' | 'once' | 'once_a_day' | 'once_a_week' = await renderSelectPrompt({ + message: 'Frequency', + choices: [ + {label: 'Always', value: 'always'}, + {label: 'Only once', value: 'once'}, + {label: 'Once a day', value: 'once_a_day'}, + {label: 'Once a week', value: 'once_a_week'}, + ], + }) + const minVersion = await renderTextPrompt({ + message: 'Minimum CLI version (optional)', + initialAnswer: CLI_KIT_VERSION, + allowEmpty: true, + }) + const maxVersion = await renderTextPrompt({ + message: 'Maximum CLI version (optional)', + initialAnswer: CLI_KIT_VERSION, + allowEmpty: true, + }) + const minDate = await renderTextPrompt({ + message: 'Minimum date in YYYY-MM-DD format (optional)', + initialAnswer: formattedToday, + allowEmpty: true, + }) + const maxDate = await renderTextPrompt({ + message: 'Maximum date in YYYY-MM-DD format (optional)', + initialAnswer: formattedToday, + allowEmpty: true, + }) + const surface = await renderTextPrompt({ + message: 'Surface. E.g.: app, theme, hydrogen, theme_app_extension... (optional)', + allowEmpty: true, + }) + const commands = await renderTextPrompt({ + message: 'Comma separated list of commands. E.g.: app:generate:extension (optional)', + allowEmpty: true, + }) + + const notifications: Notifications = await getLocalNotifications() + const notification: Notification = { + id, + type, + title, + frequency, + message, + minVersion: minVersion.length === 0 ? undefined : minVersion, + maxVersion: maxVersion.length === 0 ? undefined : maxVersion, + minDate: minDate.length === 0 ? undefined : minDate, + maxDate: maxDate.length === 0 ? undefined : maxDate, + surface: surface.length === 0 ? undefined : surface, + commands: commands.length === 0 ? undefined : commands.split(',').map((command) => command.trim()), + } + notifications.notifications.push(notification) + await writeFile('./notifications.json', JSON.stringify(notifications)) + + renderSuccess({headline: 'notifications.json file updated successfully.'}) +} + +export async function list() { + const notifications: Notifications = await getLocalNotifications() + + const columns: TableColumn<{type: string; title: string; message: string; filters: string}> = { + type: {header: 'Type', color: 'dim'}, + title: {header: 'Title', color: 'dim'}, + message: {header: 'Message', color: 'dim'}, + filters: {header: 'Filters', color: 'dim'}, + } + + const rows = notifications.notifications.map((notification) => { + return { + type: notification.type, + title: notification.title || '', + message: notification.message, + filters: stringifyFilters(notification), + } + }) + + renderTable({rows, columns}) + outputInfo('\n') +} From 744d482bb4bed5eeb6911ee6a6f06b3f4e28423b Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 15 Jul 2024 16:05:43 +0200 Subject: [PATCH 30/55] Make "Only once" the default frequency --- packages/cli/src/cli/services/commands/notifications.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/cli/services/commands/notifications.ts b/packages/cli/src/cli/services/commands/notifications.ts index 352734146b..129ebe030b 100644 --- a/packages/cli/src/cli/services/commands/notifications.ts +++ b/packages/cli/src/cli/services/commands/notifications.ts @@ -35,10 +35,10 @@ export async function generate() { const frequency: 'always' | 'once' | 'once_a_day' | 'once_a_week' = await renderSelectPrompt({ message: 'Frequency', choices: [ - {label: 'Always', value: 'always'}, {label: 'Only once', value: 'once'}, - {label: 'Once a day', value: 'once_a_day'}, {label: 'Once a week', value: 'once_a_week'}, + {label: 'Once a day', value: 'once_a_day'}, + {label: 'Always', value: 'always'}, ], }) const minVersion = await renderTextPrompt({ From 05d179dd04f203f34928c62e55264cdec0cfcfc7 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 15 Jul 2024 16:14:24 +0200 Subject: [PATCH 31/55] Ask for the slack channel of the owner team --- packages/cli-kit/src/public/node/notifications-system.ts | 3 ++- packages/cli/src/cli/services/commands/notifications.ts | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index b30365989c..c6a130b950 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -15,11 +15,12 @@ export interface Notifications { export interface Notification { id: string message: string + type: 'info' | 'warning' | 'error' + ownerChannel: string cta?: { label: string url: string } - type: 'info' | 'warning' | 'error' title?: string minVersion?: string maxVersion?: string diff --git a/packages/cli/src/cli/services/commands/notifications.ts b/packages/cli/src/cli/services/commands/notifications.ts index 129ebe030b..2141710825 100644 --- a/packages/cli/src/cli/services/commands/notifications.ts +++ b/packages/cli/src/cli/services/commands/notifications.ts @@ -69,6 +69,9 @@ export async function generate() { message: 'Comma separated list of commands. E.g.: app:generate:extension (optional)', allowEmpty: true, }) + const ownerChannel = await renderTextPrompt({ + message: 'Slack channel of the team who will own this notification', + }) const notifications: Notifications = await getLocalNotifications() const notification: Notification = { @@ -83,6 +86,7 @@ export async function generate() { maxDate: maxDate.length === 0 ? undefined : maxDate, surface: surface.length === 0 ? undefined : surface, commands: commands.length === 0 ? undefined : commands.split(',').map((command) => command.trim()), + ownerChannel, } notifications.notifications.push(notification) await writeFile('./notifications.json', JSON.stringify(notifications)) @@ -100,7 +104,7 @@ export async function list() { filters: {header: 'Filters', color: 'dim'}, } - const rows = notifications.notifications.map((notification) => { + const rows = notifications.notifications.map((notification: Notification) => { return { type: notification.type, title: notification.title || '', From 74c097fc2b6f0f04e1b82877641e55e89719b1b6 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 11:01:43 +0200 Subject: [PATCH 32/55] Remove unused field from GlobalContext --- packages/cli-kit/src/public/node/global-context.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/global-context.ts b/packages/cli-kit/src/public/node/global-context.ts index e56ed3c330..fdbb5d87a2 100644 --- a/packages/cli-kit/src/public/node/global-context.ts +++ b/packages/cli-kit/src/public/node/global-context.ts @@ -1,6 +1,5 @@ export interface GlobalContext { currentCommandId: string - [key: string]: string } let _globalContext: GlobalContext | undefined From 70d296ce51b0fbc10ae9780ba72636f68033ee42 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 11:21:14 +0200 Subject: [PATCH 33/55] Add a timeout of 3 seconds --- .../cli-kit/src/public/node/notifications-system.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index c6a130b950..5795575890 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -43,9 +43,13 @@ export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Pro const commandId = getCurrentCommandId() const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) await renderNotifications(notificationsToShow) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (error: unknown) { - outputDebug('Error retrieving notifications') + // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all + } catch (error: any) { + const errorMessage = `Error retrieving notifications: ${error.message}` + outputDebug(errorMessage) + // This is very prone to becoming a circular dependency, so we import it dynamically + const {sendErrorToBugsnag} = await import('./error-handler.js') + await sendErrorToBugsnag(errorMessage, 'unexpected_error') } } @@ -93,7 +97,7 @@ export async function getNotifications(): Promise { * Fetch notifications from GitHub. */ async function fetchNotifications(): Promise { - const response = await fetch(URL) + const response = await fetch(URL, {signal: AbortSignal.timeout(3 * 1000)}) return response.text() as unknown as string } From 2b2eacf0d881214351df319275108f4fe90d848a Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 11:49:13 +0200 Subject: [PATCH 34/55] Add zod schema --- .../src/public/node/notifications-system.ts | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 5795575890..4a64ac3118 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -3,33 +3,36 @@ import {renderError, renderInfo, renderWarning} from './ui.js' import {getCurrentCommandId} from './global-context.js' import {fileExists, readFile} from './fs.js' import {outputDebug} from './output.js' +import {zod} from './schema.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications/notifications.json' -export interface Notifications { - notifications: Notification[] -} - -export interface Notification { - id: string - message: string - type: 'info' | 'warning' | 'error' - ownerChannel: string - cta?: { - label: string - url: string - } - title?: string - minVersion?: string - maxVersion?: string - minDate?: string - maxDate?: string - commands?: string[] - surface?: 'app' | 'theme' | 'hydrogen' | string - frequency?: 'always' | 'once' | 'once_a_day' | 'once_a_week' -} +const NotificationSchema = zod.object({ + id: zod.string(), + message: zod.string(), + type: zod.enum(['info', 'warning', 'error']), + frequency: zod.enum(['always', 'once', 'once_a_day', 'once_a_week']), + ownerChannel: zod.string(), + cta: zod + .object({ + label: zod.string(), + url: zod.string().url(), + }) + .optional(), + title: zod.string().optional(), + minVersion: zod.string().optional(), + maxVersion: zod.string().optional(), + minDate: zod.string().optional(), + maxDate: zod.string().optional(), + commands: zod.array(zod.string()).optional(), + surface: zod.string().optional(), +}) +export type Notification = zod.infer + +const NotificationsSchema = zod.object({notifications: zod.array(NotificationSchema)}) +export type Notifications = zod.infer /** * Shows notifications to the user if they meet the criteria specified in the notifications.json file. @@ -90,7 +93,8 @@ async function renderNotifications(notifications: Notification[]) { export async function getNotifications(): Promise { const cacheKey: NotificationsKey = `notifications-${URL}` const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, 24 * 3600 * 1000) - return JSON.parse(rawNotifications) + const notifications: object = JSON.parse(rawNotifications) + return NotificationsSchema.parse(notifications) } /** @@ -236,5 +240,6 @@ export async function getLocalNotifications(): Promise { if (!(await fileExists(filePath))) return {notifications: []} const rawNotifications = await readFile(filePath) - return JSON.parse(rawNotifications) + const notifications: object = JSON.parse(rawNotifications) + return NotificationsSchema.parse(notifications) } From 6d999ca34cde52dca6d7a32d651283b9d970d3a5 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:24:14 +0200 Subject: [PATCH 35/55] Show notifications when minDate or maxDate is today --- packages/cli-kit/src/public/node/notifications-system.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 4a64ac3118..96215d02bf 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -119,7 +119,7 @@ export function filterNotifications( notifications: Notification[], commandId: string, currentSurfaces?: string[], - today: Date = new Date(), + today: Date = new Date(new Date().setUTCHours(0, 0, 0, 0)), currentVersion: string = CLI_KIT_VERSION, ): Notification[] { return notifications From e96662d31952d418494d64a1aa8a27be2d4e52c3 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:25:19 +0200 Subject: [PATCH 36/55] Show only 2 notifications per command --- packages/cli-kit/src/public/node/notifications-system.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 96215d02bf..3261929b7a 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -128,6 +128,7 @@ export function filterNotifications( .filter((notification) => filterByCommand(notification, commandId)) .filter((notification) => filterBySurface(notification, commandId, currentSurfaces)) .filter((notification) => filterByFrequency(notification)) + .slice(0, 2) } /** From d7c824e967b310a557012f66ab039aef44a6a65a Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:29:55 +0200 Subject: [PATCH 37/55] Upload empty notifications file --- notifications.json | 1 + 1 file changed, 1 insertion(+) create mode 100644 notifications.json diff --git a/notifications.json b/notifications.json new file mode 100644 index 0000000000..8617fa5f2e --- /dev/null +++ b/notifications.json @@ -0,0 +1 @@ +{"notifications":[]} From f7a477bbd3d5c0aaf9b04527854798bc1719fd64 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:30:26 +0200 Subject: [PATCH 38/55] Avoid caching error responses --- packages/cli-kit/src/public/node/notifications-system.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 3261929b7a..6aec7e768a 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -102,6 +102,7 @@ export async function getNotifications(): Promise { */ async function fetchNotifications(): Promise { const response = await fetch(URL, {signal: AbortSignal.timeout(3 * 1000)}) + if (response.status !== 200) throw new Error(`Failed to fetch notifications: ${response.statusText}`) return response.text() as unknown as string } From a567ee2df25a20e2793f076bdc12fc09199b704e Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:36:38 +0200 Subject: [PATCH 39/55] Add some debug log --- packages/cli-kit/src/public/node/notifications-system.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 6aec7e768a..0318bfd84a 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -45,6 +45,7 @@ export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Pro const notifications = await getNotifications() const commandId = getCurrentCommandId() const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) + outputDebug(`Notifications to show: ${notificationsToShow.length}`) await renderNotifications(notificationsToShow) // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all } catch (error: any) { @@ -101,6 +102,7 @@ export async function getNotifications(): Promise { * Fetch notifications from GitHub. */ async function fetchNotifications(): Promise { + outputDebug(`No cached notifications found. Fetching from ${URL}...`) const response = await fetch(URL, {signal: AbortSignal.timeout(3 * 1000)}) if (response.status !== 200) throw new Error(`Failed to fetch notifications: ${response.statusText}`) return response.text() as unknown as string From 09ada76ecb9fde77a490651ea37dfe764f6c2c32 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:45:18 +0200 Subject: [PATCH 40/55] Fix tests --- .../public/node/notifications-system.test.ts | 31 ++++++++++++++++++- .../src/public/node/notifications-system.ts | 4 ++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index a91ef18184..dbe7503ae7 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -10,6 +10,8 @@ const betweenVersins1and2: Notification = { id: 'betweenVersins1and2', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', minVersion: '1.0', maxVersion: '2.0', } @@ -18,6 +20,8 @@ const betweenDatesIn2000: Notification = { id: 'betweenDatesIn2000', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', minDate: '2000-01-01', maxDate: '2000-12-31', } @@ -26,6 +30,8 @@ const fromVersion1: Notification = { id: 'fromVersion1', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', minVersion: '1.0', } @@ -33,6 +39,8 @@ const upToVersion2: Notification = { id: 'upToVersion2', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', maxVersion: '2.0', } @@ -40,6 +48,8 @@ const fromDateJan2000: Notification = { id: 'fromDateJan2000', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', minDate: '2000-01-01', } @@ -47,6 +57,7 @@ const upToDateDec2000: Notification = { id: 'upToDateDec2000', message: 'message', type: 'info', + ownerChannel: 'channel', maxDate: '2000-12-31', } @@ -54,6 +65,8 @@ const onlyForDevCommand: Notification = { id: 'onlyForDevCommand', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', commands: ['app:dev'], } @@ -61,6 +74,8 @@ const onlyForThemeSurface: Notification = { id: 'onlyForThemeSurface', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', surface: 'theme', } @@ -68,6 +83,8 @@ const unknownSurface: Notification = { id: 'unknownSurface', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', surface: 'unknown', } @@ -75,6 +92,8 @@ const extensionSurface: Notification = { id: 'extensionSurface', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', surface: 'ui-extension', } @@ -83,6 +102,7 @@ const showOnce: Notification = { message: 'message', type: 'info', frequency: 'once', + ownerChannel: 'channel', } const showOnceADay: Notification = { @@ -90,6 +110,7 @@ const showOnceADay: Notification = { message: 'message', type: 'info', frequency: 'once_a_day', + ownerChannel: 'channel', } const showOnceAWeek: Notification = { @@ -97,6 +118,7 @@ const showOnceAWeek: Notification = { message: 'message', type: 'info', frequency: 'once_a_week', + ownerChannel: 'channel', } const showAlways: Notification = { @@ -104,24 +126,31 @@ const showAlways: Notification = { message: 'message', type: 'info', frequency: 'always', + ownerChannel: 'channel', } const infoNotification: Notification = { id: 'infoNotification', message: 'message', type: 'info', + frequency: 'always', + ownerChannel: 'channel', } const errorNotification: Notification = { id: 'errorNotification', message: 'message', type: 'error', + frequency: 'always', + ownerChannel: 'channel', } const warningNotification: Notification = { id: 'warningNotification', message: 'message', type: 'warning', + frequency: 'always', + ownerChannel: 'channel', } const defaultInput = [ @@ -231,7 +260,7 @@ afterEach(() => { describe('notifications-system filter notifications', () => { test.each(testCases)('Filter for %name', ({input, commandId, version, date, surfaces, output}) => { // When - const result = filterNotifications(input, commandId, surfaces, new Date(date), version) + const result = filterNotifications(input, commandId, surfaces, new Date(date), version, 10) // Then expect(result).toEqual(output) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 0318bfd84a..80d489f5c4 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -116,6 +116,7 @@ async function fetchNotifications(): Promise { * @param currentSurfaces - The surfaces present in the current project (usually for app extensions). * @param today - The current date. * @param currentVersion - The current version of the CLI. + * @param limit - Number of notifications to show (2 by default). * @returns - The filtered notifications. */ export function filterNotifications( @@ -124,6 +125,7 @@ export function filterNotifications( currentSurfaces?: string[], today: Date = new Date(new Date().setUTCHours(0, 0, 0, 0)), currentVersion: string = CLI_KIT_VERSION, + limit = 2, ): Notification[] { return notifications .filter((notification) => filterByVersion(notification, currentVersion)) @@ -131,7 +133,7 @@ export function filterNotifications( .filter((notification) => filterByCommand(notification, commandId)) .filter((notification) => filterBySurface(notification, commandId, currentSurfaces)) .filter((notification) => filterByFrequency(notification)) - .slice(0, 2) + .slice(0, limit) } /** From 3f0521f9e77e477708a39c1af20014ac4ee654ad Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 12:53:03 +0200 Subject: [PATCH 41/55] Use fetch from cli-kit to get analytics --- packages/cli-kit/src/public/node/notifications-system.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 80d489f5c4..dbd7815608 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -6,6 +6,7 @@ import {outputDebug} from './output.js' import {zod} from './schema.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' +import {fetch} from '@shopify/cli-kit/node/http' const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications/notifications.json' @@ -102,7 +103,7 @@ export async function getNotifications(): Promise { * Fetch notifications from GitHub. */ async function fetchNotifications(): Promise { - outputDebug(`No cached notifications found. Fetching from ${URL}...`) + outputDebug(`No cached notifications found. Fetching them...`) const response = await fetch(URL, {signal: AbortSignal.timeout(3 * 1000)}) if (response.status !== 200) throw new Error(`Failed to fetch notifications: ${response.statusText}`) return response.text() as unknown as string From f72f8c63282f0b3ebcc62c9f3d4d0fe0b92b8517 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 13:03:02 +0200 Subject: [PATCH 42/55] Fix type-check --- packages/cli-kit/src/public/node/notifications-system.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index dbe7503ae7..32e849a54b 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -57,6 +57,7 @@ const upToDateDec2000: Notification = { id: 'upToDateDec2000', message: 'message', type: 'info', + frequency: 'always', ownerChannel: 'channel', maxDate: '2000-12-31', } From 4724740a4acd19f6f487fd380a538bb46c9c4c59 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 13:04:00 +0200 Subject: [PATCH 43/55] Update the notifications.json final URL --- packages/cli-kit/src/public/node/notifications-system.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index dbd7815608..cf2495ac32 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -8,7 +8,7 @@ import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' import {fetch} from '@shopify/cli-kit/node/http' -const URL = 'https://raw.githubusercontent.com/Shopify/cli/notifications/notifications.json' +const URL = 'https://raw.githubusercontent.com/Shopify/cli/main/notifications.json' const NotificationSchema = zod.object({ id: zod.string(), From 4cf02dbf06ed7ace700ba5c7952ad064f021cae8 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 13:10:02 +0200 Subject: [PATCH 44/55] Move the limit to the render method --- .../cli-kit/src/public/node/notifications-system.test.ts | 2 +- packages/cli-kit/src/public/node/notifications-system.ts | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index 32e849a54b..8449d884cd 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -261,7 +261,7 @@ afterEach(() => { describe('notifications-system filter notifications', () => { test.each(testCases)('Filter for %name', ({input, commandId, version, date, surfaces, output}) => { // When - const result = filterNotifications(input, commandId, surfaces, new Date(date), version, 10) + const result = filterNotifications(input, commandId, surfaces, new Date(date), version) // Then expect(result).toEqual(output) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index cf2495ac32..56a101817d 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -59,12 +59,12 @@ export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Pro } /** - * Renders the notifications to the user. + * Renders the first 2 notifications to the user. * * @param notifications - The notifications to render. */ async function renderNotifications(notifications: Notification[]) { - notifications.forEach((notification) => { + notifications.slice(0, 2).forEach((notification) => { const content = { headline: notification.title, body: notification.message, @@ -117,7 +117,6 @@ async function fetchNotifications(): Promise { * @param currentSurfaces - The surfaces present in the current project (usually for app extensions). * @param today - The current date. * @param currentVersion - The current version of the CLI. - * @param limit - Number of notifications to show (2 by default). * @returns - The filtered notifications. */ export function filterNotifications( @@ -126,7 +125,6 @@ export function filterNotifications( currentSurfaces?: string[], today: Date = new Date(new Date().setUTCHours(0, 0, 0, 0)), currentVersion: string = CLI_KIT_VERSION, - limit = 2, ): Notification[] { return notifications .filter((notification) => filterByVersion(notification, currentVersion)) @@ -134,7 +132,6 @@ export function filterNotifications( .filter((notification) => filterByCommand(notification, commandId)) .filter((notification) => filterBySurface(notification, commandId, currentSurfaces)) .filter((notification) => filterByFrequency(notification)) - .slice(0, limit) } /** From 90314e9b9490bb9c2720c04b36e59398d42ad2c5 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 29 Jul 2024 13:29:50 +0200 Subject: [PATCH 45/55] Fix reviewers reminder team name (unrelated) --- .github/workflows/reviewers-reminder.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reviewers-reminder.yml b/.github/workflows/reviewers-reminder.yml index d3f835ec71..09eabe118b 100644 --- a/.github/workflows/reviewers-reminder.yml +++ b/.github/workflows/reviewers-reminder.yml @@ -24,4 +24,4 @@ jobs: - UI extensions: @shopify/ui-extensions-cli - Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship - Hydrogen: @shopify/hydrogen - - Other: @shopify/app-management + - Other: @shopify/app-inner-loop From cf9abdce53bacaceb9caf8e39dbac499221f00df Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 27 Aug 2024 12:08:07 +0200 Subject: [PATCH 46/55] Fix merge --- .../src/private/node/conf-store.test.ts | 7 +++--- .../cli-kit/src/private/node/conf-store.ts | 22 +++++++------------ .../src/public/node/node-package-manager.ts | 2 +- .../public/node/notifications-system.test.ts | 16 +++++++------- .../src/public/node/notifications-system.ts | 13 ++++++++--- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/packages/cli-kit/src/private/node/conf-store.test.ts b/packages/cli-kit/src/private/node/conf-store.test.ts index d67c5b50df..e8a47a6179 100644 --- a/packages/cli-kit/src/private/node/conf-store.test.ts +++ b/packages/cli-kit/src/private/node/conf-store.test.ts @@ -151,14 +151,15 @@ describe('cacheRetrieve', () => { await inTemporaryDirectory(async (cwd) => { // Given const config = new LocalStorage({cwd}) - const cacheValue = {'identity-introspection-url-IDENTITYURL': {value: 'URL1', timestamp: Date.now()}} - config.set('cache', cacheValue) + const cacheValue = {value: 'URL1', timestamp: Date.now()} + const cacheEntry = {'identity-introspection-url-IDENTITYURL': cacheValue} + config.set('cache', cacheEntry) // When const got = cacheRetrieve('identity-introspection-url-IDENTITYURL', config) // Then - expect(got).toEqual('URL1') + expect(got).toEqual(cacheValue) }) }) diff --git a/packages/cli-kit/src/private/node/conf-store.ts b/packages/cli-kit/src/private/node/conf-store.ts index 3317776037..3d7961d8a5 100644 --- a/packages/cli-kit/src/private/node/conf-store.ts +++ b/packages/cli-kit/src/private/node/conf-store.ts @@ -10,10 +10,10 @@ interface CacheValue { export type IntrospectionUrlKey = `identity-introspection-url-${string}` export type PackageVersionKey = `npm-package-${string}` export type NotificationsKey = `notifications-${string}` -type NotificationKey = `notification-${string}` +export type NotificationKey = `notification-${string}` type MostRecentOccurrenceKey = `most-recent-occurrence-${string}` -type ExportedKey = IntrospectionUrlKey | PackageVersionKey +type ExportedKey = IntrospectionUrlKey | PackageVersionKey | NotificationsKey | NotificationKey interface Cache { [introspectionUrlKey: IntrospectionUrlKey]: CacheValue @@ -87,23 +87,18 @@ export async function cacheRetrieveOrRepopulate( timeout?: number, config = cliKitStore(), ): Promise> { - const cached = getCache(key, config) + const cached = cacheRetrieve(key, config) if (cached?.value !== undefined && (timeout === undefined || Date.now() - cached.timestamp < timeout)) { return cached.value } const value = await fn() - setCache(key, value, config) + cacheStore(key, value, config) return value } -export function getCache(key: keyof Cache, config = cliKitStore()): CacheValue | undefined { - const cache: Cache = config.get('cache') || {} - return cache[key] -} - -export function setCache(key: keyof Cache, value: string, config = cliKitStore()): void { +export function cacheStore(key: ExportedKey, value: string, config = cliKitStore()): void { const cache: Cache = config.get('cache') || {} cache[key] = {value, timestamp: Date.now()} config.set('cache', cache) @@ -112,12 +107,11 @@ export function setCache(key: keyof Cache, value: string, config = cliKitStore() /** * Fetch from cache if already populated, otherwise return undefined. * @param key - The key to use for the cache. - * @returns The value from the cache or the result of the function. + * @returns The chache element. */ -export function cacheRetrieve(key: ExportedKey, config = cliKitStore()): CacheValueForKey | undefined { +export function cacheRetrieve(key: ExportedKey, config = cliKitStore()): CacheValue | undefined { const cache: Cache = config.get('cache') || {} - const cached = cache[key] - return cached?.value + return cache[key] } export function cacheClear(config = cliKitStore()): void { diff --git a/packages/cli-kit/src/public/node/node-package-manager.ts b/packages/cli-kit/src/public/node/node-package-manager.ts index a5ac87d3a5..a9be4ef757 100644 --- a/packages/cli-kit/src/public/node/node-package-manager.ts +++ b/packages/cli-kit/src/public/node/node-package-manager.ts @@ -287,7 +287,7 @@ export async function checkForNewVersion( */ export function checkForCachedNewVersion(dependency: string, currentVersion: string): string | undefined { const cacheKey: PackageVersionKey = `npm-package-${dependency}` - const lastVersion = cacheRetrieve(cacheKey) + const lastVersion = cacheRetrieve(cacheKey)?.value if (lastVersion && new SemVer(currentVersion).compare(lastVersion) < 0) { return lastVersion diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index 8449d884cd..efe47b0d47 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -1,6 +1,6 @@ import {Notification, filterNotifications, showNotificationsIfNeeded} from './notifications-system.js' import {renderError, renderInfo, renderWarning} from './ui.js' -import {getCache, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' +import {cacheRetrieve, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js' import {afterEach, describe, expect, test, vi} from 'vitest' vi.mock('./ui.js') @@ -272,7 +272,7 @@ describe('notifications-system filter notifications', () => { const current = new Date('2020-01-15T00:00:00.000Z') const yesterday = new Date('2020-01-14T08:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValue({value: yesterday.getTime().toString(), timestamp: 0}) + vi.mocked(cacheRetrieve).mockReturnValue({value: yesterday.getTime().toString(), timestamp: 0}) // When const result = filterNotifications([showAlways], 'version') @@ -285,8 +285,8 @@ describe('notifications-system filter notifications', () => { // Given const current = new Date('2020-01-15T00:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValueOnce(undefined) - vi.mocked(getCache).mockReturnValueOnce({value: current.getTime().toString(), timestamp: 0}) + vi.mocked(cacheRetrieve).mockReturnValueOnce(undefined) + vi.mocked(cacheRetrieve).mockReturnValueOnce({value: current.getTime().toString(), timestamp: 0}) // When/Then const result = filterNotifications([showOnce], 'version') @@ -300,8 +300,8 @@ describe('notifications-system filter notifications', () => { const current = new Date('2020-01-15T08:00:00.000Z') const yesterday = new Date('2020-01-14T00:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValueOnce({value: yesterday.getTime().toString(), timestamp: 0}) - vi.mocked(getCache).mockReturnValueOnce({value: current.getTime().toString(), timestamp: 0}) + vi.mocked(cacheRetrieve).mockReturnValueOnce({value: yesterday.getTime().toString(), timestamp: 0}) + vi.mocked(cacheRetrieve).mockReturnValueOnce({value: current.getTime().toString(), timestamp: 0}) // When/Then const result = filterNotifications([showOnceADay], 'version') @@ -316,8 +316,8 @@ describe('notifications-system filter notifications', () => { const yesterday = new Date('2020-01-14T08:00:00.000Z') const lastWeek = new Date('2020-01-03T00:00:00.000Z') vi.setSystemTime(current) - vi.mocked(getCache).mockReturnValueOnce({value: lastWeek.getTime().toString(), timestamp: 0}) - vi.mocked(getCache).mockReturnValueOnce({value: yesterday.getTime().toString(), timestamp: 0}) + vi.mocked(cacheRetrieve).mockReturnValueOnce({value: lastWeek.getTime().toString(), timestamp: 0}) + vi.mocked(cacheRetrieve).mockReturnValueOnce({value: yesterday.getTime().toString(), timestamp: 0}) // When/Then const result = filterNotifications([showOnceAWeek], 'version') diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 56a101817d..fca40e8bc8 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -5,7 +5,13 @@ import {fileExists, readFile} from './fs.js' import {outputDebug} from './output.js' import {zod} from './schema.js' import {CLI_KIT_VERSION} from '../common/version.js' -import {NotificationsKey, cacheRetrieveOrRepopulate, getCache, setCache} from '../../private/node/conf-store.js' +import { + NotificationKey, + NotificationsKey, + cacheRetrieve, + cacheRetrieveOrRepopulate, + cacheStore, +} from '../../private/node/conf-store.js' import {fetch} from '@shopify/cli-kit/node/http' const URL = 'https://raw.githubusercontent.com/Shopify/cli/main/notifications.json' @@ -83,7 +89,7 @@ async function renderNotifications(notifications: Notification[]) { renderError(content) } } - setCache(`notification-${notification.id}`, new Date().getTime().toString()) + cacheStore(`notification-${notification.id}`, new Date().getTime().toString()) }) } @@ -195,7 +201,8 @@ function filterBySurface(notification: Notification, commandId: string, surfaces */ function filterByFrequency(notification: Notification): boolean { if (!notification.frequency) return true - const lastShown = getCache(`notification-${notification.id}`)?.value as unknown as string + const cacheKey: NotificationKey = `notification-${notification.id}` + const lastShown = cacheRetrieve(cacheKey)?.value as unknown as string if (!lastShown) return true switch (notification.frequency) { From 105a64bc5c82b4f494666bbd41d686793dd180df Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 27 Aug 2024 17:04:36 +0200 Subject: [PATCH 47/55] Send to Bugsnag only in production --- packages/cli-kit/src/public/node/error-handler.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli-kit/src/public/node/error-handler.ts b/packages/cli-kit/src/public/node/error-handler.ts index 75c21db9d4..f4aa5845e7 100644 --- a/packages/cli-kit/src/public/node/error-handler.ts +++ b/packages/cli-kit/src/public/node/error-handler.ts @@ -259,5 +259,6 @@ function initializeBugsnag() { appVersion: CLI_KIT_VERSION, autoTrackSessions: false, autoDetectErrors: false, + enabledReleaseStages: ['production'], }) } From a99248857fd318101d29f3330813ed3c4b60b2a6 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 16 Sep 2024 09:59:34 +0200 Subject: [PATCH 48/55] Add env variable to override notifications URL --- packages/cli-kit/src/public/node/notifications-system.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index fca40e8bc8..41846e6a63 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -16,6 +16,10 @@ import {fetch} from '@shopify/cli-kit/node/http' const URL = 'https://raw.githubusercontent.com/Shopify/cli/main/notifications.json' +function url(): string { + return process.env.SHOPIFY_CLI_NOTIFICATIONS_URL ?? URL +} + const NotificationSchema = zod.object({ id: zod.string(), message: zod.string(), @@ -99,7 +103,7 @@ async function renderNotifications(notifications: Notification[]) { * @returns A Notifications object. */ export async function getNotifications(): Promise { - const cacheKey: NotificationsKey = `notifications-${URL}` + const cacheKey: NotificationsKey = `notifications-${url()}` const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, 24 * 3600 * 1000) const notifications: object = JSON.parse(rawNotifications) return NotificationsSchema.parse(notifications) @@ -110,7 +114,7 @@ export async function getNotifications(): Promise { */ async function fetchNotifications(): Promise { outputDebug(`No cached notifications found. Fetching them...`) - const response = await fetch(URL, {signal: AbortSignal.timeout(3 * 1000)}) + const response = await fetch(url(), {signal: AbortSignal.timeout(3 * 1000)}) if (response.status !== 200) throw new Error(`Failed to fetch notifications: ${response.statusText}`) return response.text() as unknown as string } From 198e4dad255e9dfdbc9d1b21933bc1117a83d50b Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 16 Sep 2024 11:24:36 +0200 Subject: [PATCH 49/55] Add command to clear the cache --- packages/cli-kit/package.json | 4 +++- packages/cli-kit/src/public/node/cli.ts | 9 ++++++++- packages/cli/oclif.manifest.json | 19 +++++++++++++++++++ packages/cli/src/cli/commands/cache/clear.ts | 11 +++++++++++ packages/cli/src/index.ts | 2 ++ .../specific-imports-in-bootstrap-code.js | 2 +- 6 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/cli/commands/cache/clear.ts diff --git a/packages/cli-kit/package.json b/packages/cli-kit/package.json index 5f40e4315b..4fdc76a0a5 100644 --- a/packages/cli-kit/package.json +++ b/packages/cli-kit/package.json @@ -94,7 +94,9 @@ "static": [ "@oclif/core", "./context/utilities.js", - "../../private/node/demo-recorder.js" + "../../private/node/demo-recorder.js", + "../../private/node/conf-store.js", + "url" ] } ] diff --git a/packages/cli-kit/src/public/node/cli.ts b/packages/cli-kit/src/public/node/cli.ts index 69604dd7d1..54439dafa4 100644 --- a/packages/cli-kit/src/public/node/cli.ts +++ b/packages/cli-kit/src/public/node/cli.ts @@ -1,7 +1,7 @@ import {isTruthy} from './context/utilities.js' import {printEventsJson} from '../../private/node/demo-recorder.js' +import {cacheClear} from '../../private/node/conf-store.js' import {Flags} from '@oclif/core' -// eslint-disable-next-line @shopify/cli/specific-imports-in-bootstrap-code import {fileURLToPath} from 'url' /** @@ -202,3 +202,10 @@ export const globalFlags = { env: 'SHOPIFY_FLAG_VERBOSE', }), } + +/** + * Clear the CLI cache, used to store some API responses and handle notifications status + */ +export function clearCache(): void { + cacheClear() +} diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index dc5e5a9e28..f6343e1a44 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -2104,6 +2104,25 @@ "pluginType": "core", "strict": true }, + "cache:clear": { + "aliases": [ + ], + "args": { + }, + "description": "Clear the CLI cache, used to store some API responses and handle notifications status", + "enableJsonFlag": false, + "flags": { + }, + "hasDynamicHelp": false, + "hidden": true, + "hiddenAliases": [ + ], + "id": "cache:clear", + "pluginAlias": "@shopify/cli", + "pluginName": "@shopify/cli", + "pluginType": "core", + "strict": true + }, "commands": { "aliases": [ ], diff --git a/packages/cli/src/cli/commands/cache/clear.ts b/packages/cli/src/cli/commands/cache/clear.ts new file mode 100644 index 0000000000..ea901b071c --- /dev/null +++ b/packages/cli/src/cli/commands/cache/clear.ts @@ -0,0 +1,11 @@ +import Command from '@shopify/cli-kit/node/base-command' +import {clearCache} from '@shopify/cli-kit/node/cli' + +export default class ClearCache extends Command { + static description = 'Clear the CLI cache, used to store some API responses and handle notifications status' + static hidden = true + + async run(): Promise { + clearCache() + } +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index ae0875a17f..8b214d4edb 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -16,6 +16,7 @@ import DocsGenerate from './cli/commands/docs/generate.js' import HelpCommand from './cli/commands/help.js' import List from './cli/commands/notifications/list.js' import Generate from './cli/commands/notifications/generate.js' +import ClearCache from './cli/commands/cache/clear.js' import ThemeCommands from '@shopify/theme' import {COMMANDS as HydrogenCommands, HOOKS as HydrogenHooks} from '@shopify/cli-hydrogen' import {commands as AppCommands} from '@shopify/app' @@ -143,6 +144,7 @@ export const COMMANDS: any = { 'docs:generate': DocsGenerate, 'notifications:list': List, 'notifications:generate': Generate, + 'cache:clear': ClearCache, } export default runShopifyCLI diff --git a/packages/eslint-plugin-cli/rules/specific-imports-in-bootstrap-code.js b/packages/eslint-plugin-cli/rules/specific-imports-in-bootstrap-code.js index 73bc5aeb57..3efed068f5 100644 --- a/packages/eslint-plugin-cli/rules/specific-imports-in-bootstrap-code.js +++ b/packages/eslint-plugin-cli/rules/specific-imports-in-bootstrap-code.js @@ -16,7 +16,7 @@ function checkImport(allowList, context, node) { const gotMatch = allowList.includes(importTarget) if (!gotMatch) { - context.report(node, `Forbidden import source "${importTarget}", update allow list if required`) + context.report(node, `Forbidden import source "${importTarget}", update allow list if required in the package.json`) } } From a792624926ab585ec1903a90dd35f43e38065c42 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 16 Sep 2024 11:32:45 +0200 Subject: [PATCH 50/55] Abort execution when getting an error notification --- .../cli-kit/src/public/node/notifications-system.test.ts | 4 ++-- packages/cli-kit/src/public/node/notifications-system.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index efe47b0d47..f07b422335 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -352,13 +352,13 @@ describe('notifications-system', () => { expect(renderWarning).toHaveBeenCalled() }) - test('an error notification triggers a renderError call', async () => { + test('an error notification triggers a renderError call and throws an error', async () => { // Given const notifications = [errorNotification] vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications})) // When - await showNotificationsIfNeeded() + await expect(showNotificationsIfNeeded()).rejects.toThrowError() // Then expect(renderError).toHaveBeenCalled() diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 41846e6a63..ba48c8c6a5 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -4,6 +4,7 @@ import {getCurrentCommandId} from './global-context.js' import {fileExists, readFile} from './fs.js' import {outputDebug} from './output.js' import {zod} from './schema.js' +import {AbortSilentError} from './error.js' import {CLI_KIT_VERSION} from '../common/version.js' import { NotificationKey, @@ -58,8 +59,9 @@ export async function showNotificationsIfNeeded(currentSurfaces?: string[]): Pro const notificationsToShow = filterNotifications(notifications.notifications, commandId, currentSurfaces) outputDebug(`Notifications to show: ${notificationsToShow.length}`) await renderNotifications(notificationsToShow) - // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all + // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { + if (error.message === 'abort') throw new AbortSilentError() const errorMessage = `Error retrieving notifications: ${error.message}` outputDebug(errorMessage) // This is very prone to becoming a circular dependency, so we import it dynamically @@ -91,6 +93,7 @@ async function renderNotifications(notifications: Notification[]) { } case 'error': { renderError(content) + throw new Error('abort') } } cacheStore(`notification-${notification.id}`, new Date().getTime().toString()) From 20b976e09f3e3498c8246110c0a9cd65d4eca546 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 16 Sep 2024 11:51:21 +0200 Subject: [PATCH 51/55] Fix merge --- packages/cli-kit/src/private/node/conf-store.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli-kit/src/private/node/conf-store.ts b/packages/cli-kit/src/private/node/conf-store.ts index bdec83a3e5..41150f338c 100644 --- a/packages/cli-kit/src/private/node/conf-store.ts +++ b/packages/cli-kit/src/private/node/conf-store.ts @@ -22,7 +22,6 @@ interface Cache { [notifications: NotificationsKey]: CacheValue [notification: NotificationKey]: CacheValue [MostRecentOccurrenceKey: MostRecentOccurrenceKey]: CacheValue - [mostRecentOccurrenceKey: MostRecentOccurrenceKey]: CacheValue [rateLimitKey: RateLimitKey]: CacheValue } From 3dea6ea88730607cd64259521d6529a15e06739e Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 16 Sep 2024 12:07:19 +0200 Subject: [PATCH 52/55] Properly show line breaks in notifications --- packages/cli-kit/src/public/node/notifications-system.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index ba48c8c6a5..6d959052c0 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -79,7 +79,7 @@ async function renderNotifications(notifications: Notification[]) { notifications.slice(0, 2).forEach((notification) => { const content = { headline: notification.title, - body: notification.message, + body: notification.message.replaceAll('\\n', '\n'), link: notification.cta, } switch (notification.type) { From 9ecc3dc01a507b01a9e27137c09f910fbd4715e4 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 25 Sep 2024 10:50:41 +0200 Subject: [PATCH 53/55] Update cache duration to 1 hour --- packages/cli-kit/src/public/node/notifications-system.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 6d959052c0..dfe3b016bb 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -16,6 +16,7 @@ import { import {fetch} from '@shopify/cli-kit/node/http' const URL = 'https://raw.githubusercontent.com/Shopify/cli/main/notifications.json' +const CACHE_DURATION_IN_MS = 3600 * 1000 function url(): string { return process.env.SHOPIFY_CLI_NOTIFICATIONS_URL ?? URL @@ -101,13 +102,13 @@ async function renderNotifications(notifications: Notification[]) { } /** - * Get notifications list from cache or fetch it if not present. + * Get notifications list from cache (refreshed every hour) or fetch it if not present. * * @returns A Notifications object. */ export async function getNotifications(): Promise { const cacheKey: NotificationsKey = `notifications-${url()}` - const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, 24 * 3600 * 1000) + const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, CACHE_DURATION_IN_MS) const notifications: object = JSON.parse(rawNotifications) return NotificationsSchema.parse(notifications) } From 17440d5bbbd500b1a365c52453f4431a41763db0 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 4 Nov 2024 10:27:29 +0100 Subject: [PATCH 54/55] Fix linter --- packages/cli-kit/src/public/node/base-command.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index 7496c162cb..9adb73b888 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -47,7 +47,7 @@ abstract class BaseCommand extends Command { // eslint-disable-next-line @typescript-eslint/no-explicit-any protected async init(): Promise { this.exitWithTimestampWhenEnvVariablePresent() - setCurrentCommandId(this.id!) + setCurrentCommandId(this.id || '') if (!isDevelopment()) { // This function runs just prior to `run` await registerCleanBugsnagErrorsFromWithinPlugins(this.config) From 247f924b97d7d05268f5d4aad2e27ed021d9b945 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 8 Nov 2024 17:39:43 +0100 Subject: [PATCH 55/55] Add changeset --- .changeset/tough-eels-bow.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/tough-eels-bow.md diff --git a/.changeset/tough-eels-bow.md b/.changeset/tough-eels-bow.md new file mode 100644 index 0000000000..bd628f87ef --- /dev/null +++ b/.changeset/tough-eels-bow.md @@ -0,0 +1,7 @@ +--- +'@shopify/cli-kit': minor +'@shopify/app': minor +'@shopify/cli': minor +--- + +Notification system