diff --git a/src/platform/updates/common/releaseService.ts b/src/platform/updates/common/releaseService.ts index 6b9bfceed..9ade43baa 100644 --- a/src/platform/updates/common/releaseService.ts +++ b/src/platform/updates/common/releaseService.ts @@ -6,10 +6,22 @@ import { getComfyApiBaseUrl } from '@/config/comfyApi' import type { components, operations } from '@/types/comfyRegistryTypes' import { isAbortError } from '@/utils/typeGuardUtil' -// Use generated types from OpenAPI spec -export type ReleaseNote = components['schemas']['ReleaseNote'] +// Base type from OpenAPI spec +type BaseReleaseNote = components['schemas']['ReleaseNote'] type GetReleasesParams = operations['getReleaseNotes']['parameters']['query'] +/** + * Extended ReleaseNote type with feature flag filtering fields. + * These fields are optional until the backend schema is updated. + * Once comfy-api schema includes these fields, this extension can be removed. + */ +export type ReleaseNote = BaseReleaseNote & { + /** Feature flags that must ALL be enabled for this release to be shown (AND logic) */ + required_feature_flags?: string[] + /** Feature flags that must ALL be disabled for this release to be shown (AND logic) */ + excluded_feature_flags?: string[] +} + // Use generated error response type type ErrorResponse = components['schemas']['ErrorResponse'] diff --git a/src/platform/updates/common/releaseStore.ts b/src/platform/updates/common/releaseStore.ts index 80c2bc05d..4854235f7 100644 --- a/src/platform/updates/common/releaseStore.ts +++ b/src/platform/updates/common/releaseStore.ts @@ -9,6 +9,7 @@ import { useSystemStatsStore } from '@/stores/systemStatsStore' import { isElectron } from '@/utils/envUtil' import { stringToLocale } from '@/utils/formatUtil' +import { useReleaseFeatureFlagFilter } from '../composables/useReleaseFeatureFlagFilter' import { useReleaseService } from './releaseService' import type { ReleaseNote } from './releaseService' @@ -19,6 +20,9 @@ export const useReleaseStore = defineStore('release', () => { const isLoading = ref(false) const error = ref(null) + // Feature flag filtering for "What's New" popup + const { filteredReleases } = useReleaseFeatureFlagFilter({ releases }) + // Services const releaseService = useReleaseService() const systemStatsStore = useSystemStatsStore() @@ -44,14 +48,14 @@ export const useReleaseStore = defineStore('release', () => { settingStore.get('Comfy.Notification.ShowVersionUpdates') ) - // Most recent release + // Most recent release (filtered by feature flags) const recentRelease = computed(() => { - return releases.value[0] ?? null + return filteredReleases.value[0] ?? null }) - // 3 most recent releases + // 3 most recent releases (filtered by feature flags) const recentReleases = computed(() => { - return releases.value.slice(0, 3) + return filteredReleases.value.slice(0, 3) }) // Helper constants diff --git a/src/platform/updates/composables/useReleaseFeatureFlagFilter.test.ts b/src/platform/updates/composables/useReleaseFeatureFlagFilter.test.ts new file mode 100644 index 000000000..089c7afdd --- /dev/null +++ b/src/platform/updates/composables/useReleaseFeatureFlagFilter.test.ts @@ -0,0 +1,349 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { ref } from 'vue' + +import type { ReleaseNote } from '../common/releaseService' +import { useReleaseFeatureFlagFilter } from './useReleaseFeatureFlagFilter' + +// Mock the remoteConfig module +vi.mock('@/platform/remoteConfig/remoteConfig', () => ({ + remoteConfig: ref({}) +})) + +// Mock the API module +vi.mock('@/scripts/api', () => ({ + api: { + getServerFeature: vi.fn() + } +})) + +// Import mocked modules after vi.mock declarations +import { remoteConfig } from '@/platform/remoteConfig/remoteConfig' +import { api } from '@/scripts/api' + +function createMockRelease(overrides: Partial = {}): ReleaseNote { + return { + id: 1, + project: 'comfyui', + version: '1.0.0', + attention: 'medium', + content: 'Test release notes', + published_at: '2024-01-01T00:00:00Z', + ...overrides + } +} + +describe('useReleaseFeatureFlagFilter', () => { + beforeEach(() => { + vi.clearAllMocks() + // Reset mocks to default behavior + remoteConfig.value = {} + vi.mocked(api.getServerFeature).mockImplementation( + (_path, defaultValue) => defaultValue + ) + }) + + describe('shouldShowRelease', () => { + it('shows releases without feature flag requirements (backward compatible)', () => { + const releases = ref([createMockRelease()]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + it('shows releases with empty feature flag arrays', () => { + const releases = ref([ + createMockRelease({ + required_feature_flags: [], + excluded_feature_flags: [] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + describe('required_feature_flags', () => { + it('shows release when all required flags are enabled in remote config', () => { + remoteConfig.value = { + model_upload_button_enabled: true, + asset_deletion_enabled: true + } + + const releases = ref([ + createMockRelease({ + required_feature_flags: [ + 'model_upload_button_enabled', + 'asset_deletion_enabled' + ] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + it('shows release when all required flags are enabled via server feature flags', () => { + vi.mocked(api.getServerFeature).mockImplementation((path) => { + if (path === 'custom_feature_a') return true + if (path === 'custom_feature_b') return true + return false + }) + + const releases = ref([ + createMockRelease({ + required_feature_flags: ['custom_feature_a', 'custom_feature_b'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + it('hides release when any required flag is disabled', () => { + remoteConfig.value = { + model_upload_button_enabled: true, + asset_deletion_enabled: false + } + + const releases = ref([ + createMockRelease({ + required_feature_flags: [ + 'model_upload_button_enabled', + 'asset_deletion_enabled' + ] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(false) + }) + + it('hides release when required flag is not found', () => { + const releases = ref([ + createMockRelease({ + required_feature_flags: ['non_existent_flag'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(false) + }) + }) + + describe('excluded_feature_flags', () => { + it('shows release when all excluded flags are disabled', () => { + remoteConfig.value = { + model_upload_button_enabled: false + } + + const releases = ref([ + createMockRelease({ + excluded_feature_flags: ['model_upload_button_enabled'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + it('shows release when excluded flags are not found (treated as disabled)', () => { + const releases = ref([ + createMockRelease({ + excluded_feature_flags: ['non_existent_flag'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + it('hides release when any excluded flag is enabled', () => { + remoteConfig.value = { + model_upload_button_enabled: true + } + + const releases = ref([ + createMockRelease({ + excluded_feature_flags: ['model_upload_button_enabled'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(false) + }) + }) + + describe('combined required and excluded flags', () => { + it('shows release when required enabled AND excluded disabled', () => { + remoteConfig.value = { + model_upload_button_enabled: true, + asset_deletion_enabled: false + } + + const releases = ref([ + createMockRelease({ + required_feature_flags: ['model_upload_button_enabled'], + excluded_feature_flags: ['asset_deletion_enabled'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(true) + }) + + it('hides release when required enabled but excluded also enabled', () => { + remoteConfig.value = { + model_upload_button_enabled: true, + asset_deletion_enabled: true + } + + const releases = ref([ + createMockRelease({ + required_feature_flags: ['model_upload_button_enabled'], + excluded_feature_flags: ['asset_deletion_enabled'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(false) + }) + + it('hides release when required disabled even if excluded disabled', () => { + remoteConfig.value = { + model_upload_button_enabled: false, + asset_deletion_enabled: false + } + + const releases = ref([ + createMockRelease({ + required_feature_flags: ['model_upload_button_enabled'], + excluded_feature_flags: ['asset_deletion_enabled'] + }) + ]) + const { shouldShowRelease } = useReleaseFeatureFlagFilter({ releases }) + + expect(shouldShowRelease(releases.value[0])).toBe(false) + }) + }) + }) + + describe('evaluateFeatureFlag', () => { + it('prefers remote config over server feature flags', () => { + remoteConfig.value = { + model_upload_button_enabled: true + } + vi.mocked(api.getServerFeature).mockReturnValue(false) + + const releases = ref([]) + const { evaluateFeatureFlag } = useReleaseFeatureFlagFilter({ releases }) + + expect(evaluateFeatureFlag('model_upload_button_enabled')).toBe(true) + // Server feature should not be called when remote config has the value + expect(api.getServerFeature).not.toHaveBeenCalled() + }) + + it('falls back to server feature flags when remote config is undefined', () => { + vi.mocked(api.getServerFeature).mockImplementation((path) => { + if (path === 'server_only_flag') return true + return false + }) + + const releases = ref([]) + const { evaluateFeatureFlag } = useReleaseFeatureFlagFilter({ releases }) + + expect(evaluateFeatureFlag('server_only_flag')).toBe(true) + expect(api.getServerFeature).toHaveBeenCalledWith( + 'server_only_flag', + false + ) + }) + + it('returns false as default when flag not found in either source', () => { + const releases = ref([]) + const { evaluateFeatureFlag } = useReleaseFeatureFlagFilter({ releases }) + + expect(evaluateFeatureFlag('unknown_flag')).toBe(false) + }) + }) + + describe('filteredReleases', () => { + it('filters releases based on feature flags', () => { + remoteConfig.value = { + model_upload_button_enabled: true, + asset_deletion_enabled: false + } + + const releases = ref([ + createMockRelease({ + id: 1, + version: '1.0.0', + required_feature_flags: ['model_upload_button_enabled'] + }), + createMockRelease({ + id: 2, + version: '0.9.0', + required_feature_flags: ['asset_deletion_enabled'] + }), + createMockRelease({ + id: 3, + version: '0.8.0' + }) + ]) + + const { filteredReleases } = useReleaseFeatureFlagFilter({ releases }) + + expect(filteredReleases.value).toHaveLength(2) + expect(filteredReleases.value.map((r) => r.id)).toEqual([1, 3]) + }) + + it('maintains order of releases after filtering', () => { + remoteConfig.value = { + model_upload_button_enabled: true, + linear_toggle_enabled: true + } + + const releases = ref([ + createMockRelease({ + id: 1, + required_feature_flags: ['model_upload_button_enabled'] + }), + createMockRelease({ + id: 2, + required_feature_flags: ['linear_toggle_enabled'] + }), + createMockRelease({ + id: 3, + required_feature_flags: ['model_upload_button_enabled'] + }) + ]) + + const { filteredReleases } = useReleaseFeatureFlagFilter({ releases }) + + expect(filteredReleases.value.map((r) => r.id)).toEqual([1, 2, 3]) + }) + + it('returns empty array when no releases match', () => { + const releases = ref([ + createMockRelease({ + required_feature_flags: ['non_existent_flag'] + }) + ]) + + const { filteredReleases } = useReleaseFeatureFlagFilter({ releases }) + + expect(filteredReleases.value).toEqual([]) + }) + + it('returns all releases when none have feature flag requirements', () => { + const releases = ref([ + createMockRelease({ id: 1 }), + createMockRelease({ id: 2 }), + createMockRelease({ id: 3 }) + ]) + + const { filteredReleases } = useReleaseFeatureFlagFilter({ releases }) + + expect(filteredReleases.value).toHaveLength(3) + }) + }) +}) diff --git a/src/platform/updates/composables/useReleaseFeatureFlagFilter.ts b/src/platform/updates/composables/useReleaseFeatureFlagFilter.ts new file mode 100644 index 000000000..537b54c20 --- /dev/null +++ b/src/platform/updates/composables/useReleaseFeatureFlagFilter.ts @@ -0,0 +1,78 @@ +import type { ComputedRef, Ref } from 'vue' +import { computed } from 'vue' + +import { remoteConfig } from '@/platform/remoteConfig/remoteConfig' +import type { RemoteConfig } from '@/platform/remoteConfig/types' +import { api } from '@/scripts/api' + +import type { ReleaseNote } from '../common/releaseService' + +interface UseReleaseFeatureFlagFilterOptions { + releases: Ref | ComputedRef +} + +/** + * Evaluates a single feature flag by name. + * Checks remote config first, then falls back to server feature flags. + */ +function evaluateFeatureFlag(flagName: string): boolean { + // Check remote config first (keyed by snake_case) + const remoteValue = remoteConfig.value[flagName as keyof RemoteConfig] + if (remoteValue !== undefined) { + return Boolean(remoteValue) + } + + // Fall back to server feature flags + return Boolean(api.getServerFeature(flagName, false)) +} + +/** + * Checks if a release note should be shown based on its feature flag requirements. + * - If no feature flag requirements, the release is shown (backward compatible) + * - required_feature_flags: ALL must be enabled (AND logic) + * - excluded_feature_flags: ALL must be disabled (AND logic) + */ +function shouldShowRelease(release: ReleaseNote): boolean { + const { required_feature_flags, excluded_feature_flags } = release + + // If no feature flag requirements, show the release (backward compatible) + if (!required_feature_flags?.length && !excluded_feature_flags?.length) { + return true + } + + // Check required flags (all must be enabled - AND logic) + if (required_feature_flags?.length) { + const allRequiredEnabled = required_feature_flags.every(evaluateFeatureFlag) + if (!allRequiredEnabled) { + return false + } + } + + // Check excluded flags (all must be disabled - AND logic) + if (excluded_feature_flags?.length) { + const anyExcludedEnabled = excluded_feature_flags.some(evaluateFeatureFlag) + if (anyExcludedEnabled) { + return false + } + } + + return true +} + +/** + * Composable for filtering release notes based on feature flags. + * Used to show/hide "What's New" popup content for specific test cohorts. + */ +export function useReleaseFeatureFlagFilter({ + releases +}: UseReleaseFeatureFlagFilterOptions) { + const filteredReleases = computed(() => { + return releases.value.filter(shouldShowRelease) + }) + + return { + filteredReleases, + evaluateFeatureFlag, + shouldShowRelease + } +}