fix: distinguish client controlled features so that server expiration timestamps are ignored (#2022)

packages/features/src/Domain/Feature/FeatureDescription.ts

@@ -23,6 +23,9 @@ export type BaseFeatureDescription = RoleFields & {
   description?: string
   expires_at?: number
 
+  /** Whether the client controls availability of this feature (such as the dark theme) */
+  clientControlled?: boolean
+
   flags?: ComponentFlag[]
   identifier: FeatureIdentifier
   marketing_url?: string

packages/features/src/Domain/Lists/Themes.ts

@@ -10,8 +10,6 @@ export function themes(): ThemeFeatureDescription[] {
     name: 'Midnight',
     identifier: FeatureIdentifier.MidnightTheme,
     permission_name: PermissionName.MidnightTheme,
-    description: 'Elegant utilitarianism.',
-    thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/midnight-with-mobile.jpg',
     isDark: true,
     dock_icon: {
       type: 'circle',
@@ -26,8 +24,6 @@ export function themes(): ThemeFeatureDescription[] {
     name: 'Futura',
     identifier: FeatureIdentifier.FuturaTheme,
     permission_name: PermissionName.FuturaTheme,
-    description: 'Calm and relaxed. Take some time off.',
-    thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/futura-with-mobile.jpg',
     isDark: true,
     dock_icon: {
       type: 'circle',
@@ -42,8 +38,6 @@ export function themes(): ThemeFeatureDescription[] {
     name: 'Solarized Dark',
     identifier: FeatureIdentifier.SolarizedDarkTheme,
     permission_name: PermissionName.SolarizedDarkTheme,
-    description: 'The perfect theme for any time.',
-    thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/solarized-dark.jpg',
     isDark: true,
     dock_icon: {
       type: 'circle',
@@ -58,8 +52,6 @@ export function themes(): ThemeFeatureDescription[] {
     name: 'Autobiography',
     identifier: FeatureIdentifier.AutobiographyTheme,
     permission_name: PermissionName.AutobiographyTheme,
-    description: 'A theme for writers and readers.',
-    thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/autobiography.jpg',
     dock_icon: {
       type: 'circle',
       background_color: '#9D7441',
@@ -73,8 +65,7 @@ export function themes(): ThemeFeatureDescription[] {
     name: 'Dark',
     identifier: FeatureIdentifier.DarkTheme,
     permission_name: PermissionName.FocusedTheme,
-    description: 'For when you need to go in.',
+    clientControlled: true,
-    thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/focus-with-mobile.jpg',
     isDark: true,
     dock_icon: {
       type: 'circle',
@@ -89,8 +80,6 @@ export function themes(): ThemeFeatureDescription[] {
     name: 'Titanium',
     identifier: FeatureIdentifier.TitaniumTheme,
     permission_name: PermissionName.TitaniumTheme,
-    description: 'Light on the eyes, heavy on the spirit.',
-    thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/titanium-with-mobile.jpg',
     dock_icon: {
       type: 'circle',
       background_color: '#6e2b9e',
@@ -106,7 +95,6 @@ export function themes(): ThemeFeatureDescription[] {
     permission_name: PermissionName.ThemeDynamic,
     layerable: true,
     no_mobile: true,
-    description: 'A smart theme that minimizes the tags and notes panels when they are not in use.',
   })
 
   return [midnight, futura, solarizedDark, autobiography, dark, titanium, dynamic]

packages/snjs/lib/Services/Features/FeaturesService.spec.ts

@@ -479,16 +479,35 @@ describe('featuresService', () => {
 
       const featuresService = createService()
       const nativeFeature = featuresService['mapRemoteNativeFeatureToStaticFeature'](remoteFeature)
-      featuresService['mapNativeFeatureToItem'] = jest.fn()
+      featuresService['mapRemoteNativeFeatureToItem'] = jest.fn()
       featuresService.initializeFromDisk()
       await featuresService.updateRolesAndFetchFeatures('123', newRoles)
-      expect(featuresService['mapNativeFeatureToItem']).toHaveBeenCalledWith(
+      expect(featuresService['mapRemoteNativeFeatureToItem']).toHaveBeenCalledWith(
         nativeFeature,
         expect.anything(),
         expect.anything(),
       )
     })
 
+    it('mapRemoteNativeFeatureToItem should throw if called with client controlled feature', async () => {
+      const clientFeature = {
+        identifier: FeatureIdentifier.DarkTheme,
+        content_type: ContentType.Theme,
+        clientControlled: true,
+      } as FeatureDescription
+
+      storageService.getValue = jest.fn().mockReturnValue(roles)
+      apiService.getUserFeatures = jest.fn().mockReturnValue({
+        data: {
+          features: [clientFeature],
+        },
+      })
+
+      const featuresService = createService()
+      featuresService.initializeFromDisk()
+      await expect(() => featuresService['mapRemoteNativeFeatureToItem'](clientFeature, [], [])).rejects.toThrow()
+    })
+
     it('feature status', async () => {
       const featuresService = createService()
 
@@ -649,6 +668,16 @@ describe('featuresService', () => {
       expect(featuresService.getFeatureStatus(FeatureIdentifier.TokenVaultEditor)).toBe(FeatureStatus.NotInCurrentPlan)
     })
 
+    it('didDownloadFeatures should filter out client controlled features', async () => {
+      const featuresService = createService()
+
+      featuresService['mapRemoteNativeFeaturesToItems'] = jest.fn()
+
+      await featuresService.didDownloadFeatures(GetFeatures().filter((f) => f.clientControlled))
+
+      expect(featuresService['mapRemoteNativeFeaturesToItems']).toHaveBeenCalledWith([])
+    })
+
     it('feature status should be dynamic for subscriber if cached features and no successful features request made yet', async () => {
       const featuresService = createService()
 

packages/snjs/lib/Services/Features/FeaturesService.ts

@@ -152,7 +152,7 @@ export class SNFeaturesService
     await super.handleApplicationStage(stage)
 
     if (stage === ApplicationStage.FullSyncCompleted_13) {
-      void this.addDarkTheme()
+      void this.mapClientControlledFeaturesToItems()
 
       if (!this.rolesIncludePaidSubscription()) {
         const offlineRepo = this.getOfflineRepo()
@@ -163,11 +163,32 @@ export class SNFeaturesService
     }
   }
 
-  private async addDarkTheme() {
+  private async mapClientControlledFeaturesToItems() {
-    const darkThemeFeature = FeaturesImports.FindNativeFeature(FeatureIdentifier.DarkTheme)
+    const clientFeatures = FeaturesImports.GetFeatures().filter((feature) => feature.clientControlled)
+    const currentItems = this.itemManager.getItems<Models.SNComponent>([ContentType.Component, ContentType.Theme])
 
-    if (darkThemeFeature) {
+    for (const feature of clientFeatures) {
-      await this.mapRemoteNativeFeaturesToItems([darkThemeFeature])
+      if (!feature.content_type) {
+        continue
+      }
+
+      const existingItem = currentItems.find((item) => item.identifier === feature.identifier)
+      if (existingItem) {
+        const hasChange = JSON.stringify(feature) !== JSON.stringify(existingItem.package_info)
+        if (hasChange) {
+          await this.itemManager.changeComponent(existingItem, (mutator) => {
+            mutator.package_info = feature
+          })
+        }
+
+        continue
+      }
+
+      await this.itemManager.createItem(
+        feature.content_type,
+        this.componentContentForNativeFeatureDescription(feature),
+        true,
+      )
     }
   }
 
@@ -396,7 +417,10 @@ export class SNFeaturesService
 
   public async didDownloadFeatures(features: FeaturesImports.FeatureDescription[]): Promise<void> {
     features = features
-      .filter((feature) => !!FeaturesImports.FindNativeFeature(feature.identifier))
+      .filter((feature) => {
+        const nativeFeature = FeaturesImports.FindNativeFeature(feature.identifier)
+        return nativeFeature != undefined && !nativeFeature.clientControlled
+      })
       .map((feature) => this.mapRemoteNativeFeatureToStaticFeature(feature))
 
     this.features = features
@@ -436,6 +460,7 @@ export class SNFeaturesService
     if (nativeFeatureCopy.expires_at) {
       nativeFeatureCopy.expires_at = convertTimestampToMilliseconds(nativeFeatureCopy.expires_at)
     }
+
     return nativeFeatureCopy
   }
 
@@ -563,32 +588,41 @@ export class SNFeaturesService
     let hasChanges = false
 
     for (const feature of features) {
-      const didChange = await this.mapNativeFeatureToItem(feature, currentItems, itemsToDelete)
+      const didChange = await this.mapRemoteNativeFeatureToItem(feature, currentItems, itemsToDelete)
       if (didChange) {
         hasChanges = true
       }
     }
 
     await this.itemManager.setItemsToBeDeleted(itemsToDelete)
+
     if (hasChanges) {
       void this.syncService.sync()
     }
   }
 
-  private async mapNativeFeatureToItem(
+  private async mapRemoteNativeFeatureToItem(
     feature: FeaturesImports.FeatureDescription,
     currentItems: Models.SNComponent[],
     itemsToDelete: Models.SNComponent[],
   ): Promise<boolean> {
+    if (feature.clientControlled) {
+      throw new Error('Attempted to map client controlled feature as remote item')
+    }
+
     if (!feature.content_type) {
       return false
     }
 
-    if (this.isExperimentalFeature(feature.identifier) && !this.isExperimentalFeatureEnabled(feature.identifier)) {
+    const isDisabledExperimentalFeature =
+      this.isExperimentalFeature(feature.identifier) && !this.isExperimentalFeatureEnabled(feature.identifier)
+
+    if (isDisabledExperimentalFeature) {
       return false
     }
 
     let hasChanges = false
+
     const now = new Date()
     const expired = this.isFreeFeature(feature.identifier)
       ? false
@@ -599,6 +633,7 @@ export class SNFeaturesService
         const itemIdentifier = item.content.package_info.identifier
         return itemIdentifier === feature.identifier
       }
+
       return false
     })
 
@@ -610,14 +645,17 @@ export class SNFeaturesService
 
     if (existingItem) {
       const featureExpiresAt = new Date(feature.expires_at || 0)
-      const hasChange =
+      const hasChangeInPackageInfo = JSON.stringify(feature) !== JSON.stringify(existingItem.package_info)
-        JSON.stringify(feature) !== JSON.stringify(existingItem.package_info) ||
+      const hasChangeInExpiration = featureExpiresAt.getTime() !== existingItem.valid_until.getTime()
-        featureExpiresAt.getTime() !== existingItem.valid_until.getTime()
+
+      const hasChange = hasChangeInPackageInfo || hasChangeInExpiration
+
       if (hasChange) {
         resultingItem = await this.itemManager.changeComponent(existingItem, (mutator) => {
           mutator.package_info = feature
           mutator.valid_until = featureExpiresAt
         })
+
         hasChanges = true
       } else {
         resultingItem = existingItem

packages/ui-services/src/Theme/ThemeManager.ts

@@ -153,21 +153,24 @@ export class ThemeManager extends AbstractService {
 
   private handleFeaturesUpdated(): void {
     let hasChange = false
+
     for (const themeUuid of this.activeThemes) {
       const theme = this.application.items.findItem(themeUuid) as SNTheme
+
       if (!theme) {
         this.deactivateTheme(themeUuid)
         hasChange = true
-      } else {
+        continue
-        const status = this.application.features.getFeatureStatus(theme.identifier)
+      }
-        if (status !== FeatureStatus.Entitled) {
+
-          if (theme.active) {
+      const status = this.application.features.getFeatureStatus(theme.identifier)
-            this.application.mutator.toggleTheme(theme).catch(console.error)
+      if (status !== FeatureStatus.Entitled) {
-          } else {
+        if (theme.active) {
-            this.deactivateTheme(theme.uuid)
+          this.application.mutator.toggleTheme(theme).catch(console.error)
-          }
+        } else {
-          hasChange = true
+          this.deactivateTheme(theme.uuid)
         }
+        hasChange = true
       }
     }