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

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

Merged
merged 1 commit into from Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/features/src/Domain/Feature/FeatureDescription.ts
Expand Up @@ -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
Expand Down
14 changes: 1 addition & 13 deletions packages/features/src/Domain/Lists/Themes.ts
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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.',
thumbnail_url: 'https://s3.amazonaws.com/standard-notes/screenshots/models/themes/focus-with-mobile.jpg',
clientControlled: true,
isDark: true,
dock_icon: {
type: 'circle',
Expand All @@ -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',
Expand All @@ -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]
Expand Down
33 changes: 31 additions & 2 deletions packages/snjs/lib/Services/Features/FeaturesService.spec.ts
Expand Up @@ -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()

Expand Down Expand Up @@ -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()

Expand Down
62 changes: 50 additions & 12 deletions packages/snjs/lib/Services/Features/FeaturesService.ts
Expand Up @@ -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()
Expand All @@ -163,11 +163,32 @@ export class SNFeaturesService
}
}

private async addDarkTheme() {
const darkThemeFeature = FeaturesImports.FindNativeFeature(FeatureIdentifier.DarkTheme)
private async mapClientControlledFeaturesToItems() {
const clientFeatures = FeaturesImports.GetFeatures().filter((feature) => feature.clientControlled)
const currentItems = this.itemManager.getItems<Models.SNComponent>([ContentType.Component, ContentType.Theme])

for (const feature of clientFeatures) {
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
}

if (darkThemeFeature) {
await this.mapRemoteNativeFeaturesToItems([darkThemeFeature])
await this.itemManager.createItem(
feature.content_type,
this.componentContentForNativeFeatureDescription(feature),
true,
)
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -436,6 +460,7 @@ export class SNFeaturesService
if (nativeFeatureCopy.expires_at) {
nativeFeatureCopy.expires_at = convertTimestampToMilliseconds(nativeFeatureCopy.expires_at)
}

return nativeFeatureCopy
}

Expand Down Expand Up @@ -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
Expand All @@ -599,6 +633,7 @@ export class SNFeaturesService
const itemIdentifier = item.content.package_info.identifier
return itemIdentifier === feature.identifier
}

return false
})

Expand All @@ -610,14 +645,17 @@ export class SNFeaturesService

if (existingItem) {
const featureExpiresAt = new Date(feature.expires_at || 0)
const hasChange =
JSON.stringify(feature) !== JSON.stringify(existingItem.package_info) ||
featureExpiresAt.getTime() !== existingItem.valid_until.getTime()
const hasChangeInPackageInfo = JSON.stringify(feature) !== JSON.stringify(existingItem.package_info)
const hasChangeInExpiration = 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
Expand Down
21 changes: 12 additions & 9 deletions packages/ui-services/src/Theme/ThemeManager.ts
Expand Up @@ -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 {
const status = this.application.features.getFeatureStatus(theme.identifier)
if (status !== FeatureStatus.Entitled) {
if (theme.active) {
this.application.mutator.toggleTheme(theme).catch(console.error)
} else {
this.deactivateTheme(theme.uuid)
}
hasChange = true
continue
}

const status = this.application.features.getFeatureStatus(theme.identifier)
if (status !== FeatureStatus.Entitled) {
if (theme.active) {
this.application.mutator.toggleTheme(theme).catch(console.error)
} else {
this.deactivateTheme(theme.uuid)
}
hasChange = true
}
}

Expand Down