Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification system #4149

Merged
merged 63 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
4d32b20
Sample json
gonzaloriestra Jun 26, 2024
f2a1ee5
Update notifications.json
gonzaloriestra Jun 26, 2024
58e7c09
Show notifications
gonzaloriestra Jun 26, 2024
29cbeb0
Extract the notification system to its own file
isaacroldan Jun 26, 2024
92bf53b
Fix date filter and render by type
gonzaloriestra Jun 26, 2024
e17405b
Fix URL
gonzaloriestra Jun 26, 2024
ed00213
Update notifications.json
gonzaloriestra Jun 26, 2024
1d13c6d
Extract the filtering function
isaacroldan Jun 26, 2024
beb3870
Empty test
isaacroldan Jun 26, 2024
1aac63e
add test for notification-system
isaacroldan Jun 26, 2024
02db9ba
Add cache
gonzaloriestra Jun 26, 2024
45f23aa
add more tests
isaacroldan Jun 26, 2024
dd6b534
Filter by surface
isaacroldan Jun 26, 2024
03fa7ce
Filter by surface in app loader
isaacroldan Jun 27, 2024
8232c7d
Surface filter for apps
isaacroldan Jun 27, 2024
8c9aa05
Test with ui-extensino surface
isaacroldan Jun 27, 2024
4c0711a
update notification json
isaacroldan Jun 27, 2024
6f15859
update notification json
isaacroldan Jun 27, 2024
9d4ec89
Ignore commands when running notifications from loader
isaacroldan Jun 27, 2024
7747888
Add more tests
isaacroldan Jun 27, 2024
c952534
Add filter by frequency
gonzaloriestra Jun 27, 2024
7ffb885
Merge branch 'notifications-sytem' of https://github.com/Shopify/cli …
gonzaloriestra Jun 27, 2024
00ccbb1
Add support for CTA links
isaacroldan Jun 27, 2024
f5a9704
new notification
isaacroldan Jun 27, 2024
8eb6800
List and generate commands
gonzaloriestra Jun 27, 2024
017a901
Delete notifications.json
gonzaloriestra Jun 28, 2024
b0160b8
Catch and ignore errors
gonzaloriestra Jun 28, 2024
4e10b20
Fix URL
gonzaloriestra Jun 28, 2024
c490d08
Reuse getCache and setCache
gonzaloriestra Jul 3, 2024
3f53781
Extract commands logic to a service
gonzaloriestra Jul 3, 2024
744d482
Make "Only once" the default frequency
gonzaloriestra Jul 15, 2024
05d179d
Ask for the slack channel of the owner team
gonzaloriestra Jul 15, 2024
74c097f
Remove unused field from GlobalContext
gonzaloriestra Jul 29, 2024
70d296c
Add a timeout of 3 seconds
gonzaloriestra Jul 29, 2024
2b2eacf
Add zod schema
gonzaloriestra Jul 29, 2024
6d999ca
Show notifications when minDate or maxDate is today
gonzaloriestra Jul 29, 2024
e96662d
Show only 2 notifications per command
gonzaloriestra Jul 29, 2024
d7c824e
Upload empty notifications file
gonzaloriestra Jul 29, 2024
f7a477b
Avoid caching error responses
gonzaloriestra Jul 29, 2024
a567ee2
Add some debug log
gonzaloriestra Jul 29, 2024
09ada76
Fix tests
gonzaloriestra Jul 29, 2024
3f0521f
Use fetch from cli-kit to get analytics
gonzaloriestra Jul 29, 2024
9689406
Merge branch 'main' into notifications
gonzaloriestra Jul 29, 2024
f72f8c6
Fix type-check
gonzaloriestra Jul 29, 2024
4724740
Update the notifications.json final URL
gonzaloriestra Jul 29, 2024
4cf02db
Move the limit to the render method
gonzaloriestra Jul 29, 2024
90314e9
Fix reviewers reminder team name (unrelated)
gonzaloriestra Jul 29, 2024
59673e7
Merge branch 'main' into notifications
gonzaloriestra Aug 27, 2024
cf9abdc
Fix merge
gonzaloriestra Aug 27, 2024
105a64b
Send to Bugsnag only in production
gonzaloriestra Aug 27, 2024
a992488
Add env variable to override notifications URL
gonzaloriestra Sep 16, 2024
198e4da
Add command to clear the cache
gonzaloriestra Sep 16, 2024
a792624
Abort execution when getting an error notification
gonzaloriestra Sep 16, 2024
cc44d35
Merge branch 'main' into notifications
gonzaloriestra Sep 16, 2024
c592e8c
Merge branch 'main' into notifications
gonzaloriestra Sep 16, 2024
20b976e
Fix merge
gonzaloriestra Sep 16, 2024
3dea6ea
Properly show line breaks in notifications
gonzaloriestra Sep 16, 2024
0c31a8c
Merge branch 'main' into notifications
gonzaloriestra Sep 25, 2024
9ecc3dc
Update cache duration to 1 hour
gonzaloriestra Sep 25, 2024
53f17bf
Merge branch 'main' into notifications
gonzaloriestra Nov 4, 2024
17440d5
Fix linter
gonzaloriestra Nov 4, 2024
247f924
Add changeset
gonzaloriestra Nov 8, 2024
f0942ec
Merge branch 'main' into notifications
gonzaloriestra Nov 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/tough-eels-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/cli-kit': minor
'@shopify/app': minor
'@shopify/cli': minor
---

Notification system
1 change: 1 addition & 0 deletions notifications.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"notifications":[]}
5 changes: 5 additions & 0 deletions packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
import {globalCLIVersion, localCLIVersion} from '@shopify/cli-kit/node/version'

const defaultExtensionDirectory = 'extensions/*'
Expand Down Expand Up @@ -327,6 +328,10 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
remoteFlags: this.remoteFlags,
})

// Show CLI notifications that are targetted for when your app has specific extension types
const extensionTypes = appClass.realExtensions.map((module) => module.type)
await showNotificationsIfNeeded(extensionTypes)

if (!this.errors.isEmpty()) appClass.errors = this.errors

await logMetadataForLoadedApp(appClass, {
Expand Down
4 changes: 3 additions & 1 deletion packages/cli-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@
"static": [
"@oclif/core",
"./context/utilities.js",
"../../private/node/demo-recorder.js"
"../../private/node/demo-recorder.js",
"../../private/node/conf-store.js",
"url"
]
}
]
Expand Down
7 changes: 4 additions & 3 deletions packages/cli-kit/src/private/node/conf-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,15 @@ describe('cacheRetrieve', () => {
await inTemporaryDirectory(async (cwd) => {
// Given
const config = new LocalStorage<any>({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)
})
})

Expand Down
25 changes: 16 additions & 9 deletions packages/cli-kit/src/private/node/conf-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,19 @@

export type IntrospectionUrlKey = `identity-introspection-url-${string}`
export type PackageVersionKey = `npm-package-${string}`
export type NotificationsKey = `notifications-${string}`
export type NotificationKey = `notification-${string}`
type MostRecentOccurrenceKey = `most-recent-occurrence-${string}`
type RateLimitKey = `rate-limited-occurrences-${string}`

type ExportedKey = IntrospectionUrlKey | PackageVersionKey
type ExportedKey = IntrospectionUrlKey | PackageVersionKey | NotificationsKey | NotificationKey

interface Cache {
[introspectionUrlKey: IntrospectionUrlKey]: CacheValue<string>
[packageVersionKey: PackageVersionKey]: CacheValue<string>
[mostRecentOccurrenceKey: MostRecentOccurrenceKey]: CacheValue<boolean>
[notifications: NotificationsKey]: CacheValue<string>
[notification: NotificationKey]: CacheValue<string>
[MostRecentOccurrenceKey: MostRecentOccurrenceKey]: CacheValue<boolean>
[rateLimitKey: RateLimitKey]: CacheValue<number[]>
}

Expand Down Expand Up @@ -85,28 +89,31 @@
timeout?: number,
config = cliKitStore(),
): Promise<CacheValueForKey<typeof key>> {
const cache: Cache = config.get('cache') || {}
const cached = cache[key]
const cached = cacheRetrieve(key, config)

if (cached?.value !== undefined && (timeout === undefined || Date.now() - cached.timestamp < timeout)) {
return cached.value
}

const value = await fn()
cacheStore(key, value, config)
return value
}

export function cacheStore(key: ExportedKey, value: string, config = cliKitStore()): void {
const cache: Cache = config.get('cache') || {}

Check warning on line 104 in packages/cli-kit/src/private/node/conf-store.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/private/node/conf-store.ts#L104

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
cache[key] = {value, timestamp: Date.now()}
config.set('cache', cache)
return value
}

/**
* 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<typeof key> | undefined {
export function cacheRetrieve(key: ExportedKey, config = cliKitStore()): CacheValue<string> | undefined {
const cache: Cache = config.get('cache') || {}

Check warning on line 115 in packages/cli-kit/src/private/node/conf-store.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/private/node/conf-store.ts#L115

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
const cached = cache[key]
return cached?.value
return cache[key]
}

export function cacheClear(config = cliKitStore()): void {
Expand Down Expand Up @@ -139,7 +146,7 @@
task: () => Promise<void>,
config = cliKitStore(),
): Promise<boolean> {
const cache: Cache = config.get('cache') || {}

Check warning on line 149 in packages/cli-kit/src/private/node/conf-store.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/private/node/conf-store.ts#L149

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
const cacheKey: MostRecentOccurrenceKey = `most-recent-occurrence-${key}`
const cached = cache[cacheKey]

Expand Down Expand Up @@ -190,7 +197,7 @@
*/
export async function runWithRateLimit(options: RunWithRateLimitOptions, config = cliKitStore()): Promise<boolean> {
const {key, limit, timeout, task} = options
const cache: Cache = config.get('cache') || {}

Check warning on line 200 in packages/cli-kit/src/private/node/conf-store.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/private/node/conf-store.ts#L200

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
const cacheKey: RateLimitKey = `rate-limited-occurrences-${key}`
const cached = cache[cacheKey]
const now = Date.now()
Expand Down
4 changes: 4 additions & 0 deletions packages/cli-kit/src/public/node/base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import {terminalSupportsPrompting} 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'
Expand Down Expand Up @@ -45,11 +47,13 @@
// eslint-disable-next-line @typescript-eslint/no-explicit-any
protected async init(): Promise<any> {
this.exitWithTimestampWhenEnvVariablePresent()
setCurrentCommandId(this.id || '')

Check warning on line 50 in packages/cli-kit/src/public/node/base-command.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/public/node/base-command.ts#L50

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
if (!isDevelopment()) {
// This function runs just prior to `run`
await registerCleanBugsnagErrorsFromWithinPlugins(this.config)
}
this.showNpmFlagWarning()
await showNotificationsIfNeeded()
return super.init()
}

Expand Down Expand Up @@ -148,7 +152,7 @@
// Replace the original result with this one.
const result = await super.parse<TFlags, TGlobalFlags, TArgs>(options, [
// Need to specify argv default because we're merging with argsFromEnvironment.
...(argv || this.argv),

Check warning on line 155 in packages/cli-kit/src/public/node/base-command.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/cli-kit/src/public/node/base-command.ts#L155

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
...argsFromEnvironment<TFlags, TGlobalFlags, TArgs>(environment, options, noDefaultsResult),
])

Expand Down
9 changes: 8 additions & 1 deletion packages/cli-kit/src/public/node/cli.ts
Original file line number Diff line number Diff line change
@@ -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'

/**
Expand Down Expand Up @@ -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()
}
35 changes: 35 additions & 0 deletions packages/cli-kit/src/public/node/global-context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
export interface GlobalContext {
currentCommandId: 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
}
2 changes: 1 addition & 1 deletion packages/cli-kit/src/public/node/node-package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,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
Expand Down
Loading
Loading