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

Contentful/duplicate assets #893

Merged
merged 6 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ REMOVE_LOCALES=$5

../../node_modules/.bin/ts-node --files src/tools/l10n/locale-migrate.ts \
"$FROM_FILE" "$TO_FILE" "$FROM_LOCALE" "$TO_LOCALE" "$REMOVE_LOCALES"

echo "Change Element.image so that it does not have 1 version per locale"
6 changes: 5 additions & 1 deletion packages/botonic-plugin-contentful/src/cms/cms-dummy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export class DummyCMS implements CMS {
}

asset(id: string, context?: Context): Promise<Asset> {
return Promise.resolve(new Asset(`name for ${id}`, `http://url.${id}`))
return Promise.resolve(new Asset(id, `name for ${id}`, `http://url.${id}`))
}

dateRange(id: string, context?: Context): Promise<DateRangeContent> {
Expand All @@ -156,4 +156,8 @@ export class DummyCMS implements CMS {
contents(contentType: ContentType, context?: Context): Promise<Content[]> {
return Promise.resolve([])
}

assets(context?: Context): Promise<Asset[]> {
return Promise.resolve([])
}
}
9 changes: 5 additions & 4 deletions packages/botonic-plugin-contentful/src/cms/cms-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,16 @@ export class ErrorReportingCMS implements CMS {
.catch(this.handleError('topContents'))
}

contents(
contentType: ContentType,
context?: Context | undefined
): Promise<Content[]> {
contents(contentType: ContentType, context?: Context): Promise<Content[]> {
return this.cms
.contents(contentType, context)
.catch(this.handleError('contents'))
}

assets(context?: Context): Promise<Asset[]> {
return this.cms.assets(context).catch(this.handleError('assets'))
}

schedule(id: string, context?: Context): Promise<ScheduleContent> {
return this.cms
.schedule(id, context)
Expand Down
4 changes: 4 additions & 0 deletions packages/botonic-plugin-contentful/src/cms/cms-multilocale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ export class MultiContextCms implements CMS {
return this.cmsFromContext(context).contents(contentType, context)
}

assets(context?: Context): Promise<Asset[]> {
return this.cmsFromContext(context).assets(context)
}

contentsWithKeywords(context?: Context): Promise<SearchCandidate[]> {
return this.cmsFromContext(context).contentsWithKeywords(context)
}
Expand Down
1 change: 1 addition & 0 deletions packages/botonic-plugin-contentful/src/cms/cms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export interface CMS {
* TODO add filter by id or name
*/
contents(contentType: ContentType, context?: Context): Promise<Content[]>
assets(context?: Context): Promise<Asset[]>

/**
* For contents with 'Searchable by' field (eg. {@link Queue}), it returns one result per each 'Seachable by' entry
Expand Down
1 change: 1 addition & 0 deletions packages/botonic-plugin-contentful/src/cms/contents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export class Asset {
* @param details depends on the type. eg the image size
*/
constructor(
readonly id: string,
readonly name: string,
readonly url: string,
readonly type?: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ export class FilteredCMS implements CMS {
return this.filterContents(contents, context)
}

assets(context?: Context): Promise<Asset[]> {
return this.cms.assets(context)
}

schedule(id: string, context?: Context): Promise<ScheduleContent> {
return this.cms.schedule(id, context)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as cms from '../cms'
import {
Asset,
CommonFields,
Content,
ContentType,
Expand Down Expand Up @@ -220,8 +221,12 @@ export class Contentful implements cms.CMS {
return this._schedule.schedule(id)
}

asset(id: string): Promise<cms.Asset> {
return this._asset.asset(id)
asset(id: string, context = DEFAULT_CONTEXT): Promise<cms.Asset> {
return this._asset.asset(id, context)
}

assets(context = DEFAULT_CONTEXT): Promise<Asset[]> {
return this._asset.assets(context)
}

dateRange(id: string): Promise<DateRangeContent> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
import * as cms from '../../cms'
import { ContentfulEntryUtils, DeliveryApi } from '../delivery-api'
import { Asset } from 'contentful'

export class AssetDelivery {
constructor(protected delivery: DeliveryApi) {}

async asset(id: string): Promise<cms.Asset> {
const asset = await this.delivery.getAsset(id)
async asset(id: string, context: cms.Context): Promise<cms.Asset> {
const asset = await this.delivery.getAsset(id, context)
return this.fromEntry(asset)
}

private fromEntry(asset: Asset) {
return new cms.Asset(
asset.sys.id,
asset.fields.title,
ContentfulEntryUtils.urlFromAsset(asset),
asset.fields.file.contentType,
asset.fields.file.details
)
}

async assets(context: cms.Context): Promise<cms.Asset[]> {
const assets = await this.delivery.getAssets(context)
return assets.items.map(a => this.fromEntry(a))
}
}
19 changes: 16 additions & 3 deletions packages/botonic-plugin-contentful/src/contentful/delivery-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import { ReducedClientApi } from './delivery/client-api'
import { ContentfulOptions } from '../plugin'

export interface DeliveryApi {
getAsset(id: string, query?: any): Promise<contentful.Asset>
getAsset(id: string, context: Context, query?: any): Promise<contentful.Asset>

getAssets(context: Context, query?: any): Promise<contentful.AssetCollection>

getEntry<T>(
id: string,
Expand All @@ -42,8 +44,19 @@ export class AdaptorDeliveryApi implements DeliveryApi {
readonly options: ContentfulOptions
) {}

async getAsset(id: string, query?: any): Promise<contentful.Asset> {
return this.client.getAsset(id, query)
async getAsset(
id: string,
context: Context,
query?: any
): Promise<contentful.Asset> {
return this.client.getAsset(id, this.queryFromContext(context, query))
}

async getAssets(
context: Context,
query?: any
): Promise<contentful.AssetCollection> {
return this.client.getAssets(this.queryFromContext(context, query))
}

async getEntry<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ReducedClientApi } from './client-api'

export class CachedClientApi implements ReducedClientApi {
readonly getAsset: (id: string, query?: any) => Promise<contentful.Asset>
readonly getAssets: (query?: any) => Promise<contentful.AssetCollection>
readonly getEntries: <T>(query: any) => Promise<contentful.EntryCollection<T>>
readonly getEntry: <T>(id: string, query?: any) => Promise<Entry<T>>
readonly getContentType: (id: string) => Promise<ContentType>
Expand All @@ -23,6 +24,7 @@ export class CachedClientApi implements ReducedClientApi {
} as memoize.Options<any>)

this.getAsset = memoize(client.getAsset, options(2))
this.getAssets = memoize(client.getAssets, options(1))
this.getEntries = memoize(client.getEntries, options(1))
this.getEntry = memoize(client.getEntry, options(2))
this.getContentType = memoize(client.getContentType, options(1))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import { ContentfulClientApi } from 'contentful'

export type ReducedClientApi = Pick<
ContentfulClientApi,
'getAsset' | 'getEntries' | 'getEntry' | 'getContentType'
'getAsset' | 'getAssets' | 'getEntries' | 'getEntry' | 'getContentType'
>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
I18nValue,
VisitedField,
} from './traverser'
import * as contentful from 'contentful'

/**
* It requests contentful to deliver all locales for each entry, and we discard all except the one in the context
Expand Down Expand Up @@ -61,8 +62,21 @@ export class IgnoreFallbackDecorator implements DeliveryApi {
)
}

getAsset(id: string, query?: any): Promise<Asset> {
return this.api.getAsset(id, query)
getAsset(id: string, context: Context, query?: any): Promise<Asset> {
console.warn(
'IgnoreFallbackDecorator does not any special treatment for getAsset'
)
return this.api.getAsset(id, context, query)
}

async getAssets(
context: Context,
query?: any
): Promise<contentful.AssetCollection> {
console.warn(
'IgnoreFallbackDecorator does not any special treatment for getAssets'
)
return this.api.getAssets(context, query)
}

private i18nContext(context: Context) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import { ManageCms } from '../../manage-cms/manage-cms'
import * as cms from '../../cms'
import { CmsException, ContentId } from '../../cms'
import * as nlp from '../../nlp'
import { Locale } from '../../nlp'
import { createClient } from 'contentful-management'
// eslint-disable-next-line node/no-missing-import
import { ClientAPI } from 'contentful-management/dist/typings/create-contentful-api'
// eslint-disable-next-line node/no-missing-import
import { Environment } from 'contentful-management/dist/typings/entities/environment'
// eslint-disable-next-line node/no-missing-import
import { Entry } from 'contentful-management/dist/typings/entities/entry'
// eslint-disable-next-line node/no-missing-import
import { Asset } from 'contentful-management/dist/typings/entities/asset'
import { ContentfulOptions } from '../../plugin'
import { ManageContext } from '../../manage-cms/manage-context'
import { CmsException, ContentId } from '../../cms'
import {
CONTENT_FIELDS,
ContentField,
ContentFieldType,
} from '../../manage-cms/fields'
import { Locale } from '../../nlp'

export class ManageContentful implements ManageCms {
readonly manage: ClientAPI
Expand Down Expand Up @@ -55,7 +56,7 @@ export class ManageContentful implements ManageCms {
return this.environment
}

async updateField<T extends cms.Content>(
async updateField(
context: ManageContext,
contentId: ContentId,
fieldType: ContentFieldType,
Expand All @@ -67,7 +68,39 @@ export class ManageContentful implements ManageCms {
oldEntry.fields[field.cmsName][context.locale] = value
// we could use this.deliver.contentFromEntry & IgnoreFallbackDecorator to convert
// the multilocale fields returned by update()
await this.updateEntry(context, oldEntry)
await this.writeEntry(context, oldEntry)
}

async removeAssetFile(
context: ManageContext,
assetId: string
): Promise<void> {
const environment = await this.getEnvironment()
const asset = await environment.getAsset(assetId)
delete asset.fields.file[context.locale]
await this.writeAsset({ ...context, allowOverwrites: true }, asset)
}

async copyAssetFile(
context: ManageContext,
assetId: string,
fromLocale: nlp.Locale
): Promise<void> {
const environment = await this.getEnvironment()
const oldAsset = await environment.getAsset(assetId)
if (!context.allowOverwrites && oldAsset.fields.file[context.locale]) {
throw new Error(
`Cannot overwrite asset '${assetId}' because it's not empty and ManageContext.allowOverwrites is false`
)
}
const fromFile = oldAsset.fields.file[fromLocale]
if (!fromFile) {
throw Error(`Asset '${assetId}' has no file for locale ${fromLocale}`)
}
oldAsset.fields.file[context.locale] = fromFile
// we could use this.deliver.contentFromEntry & IgnoreFallbackDecorator to convert
// the multilocale fields returned by update()
await this.writeAsset(context, oldAsset)
}

private checkOverwrite(
Expand Down Expand Up @@ -111,7 +144,7 @@ export class ManageContentful implements ManageCms {
return field
}

async copyField<T extends cms.Content>(
async copyField(
context: ManageContext,
contentId: ContentId,
fieldType: ContentFieldType,
Expand All @@ -130,10 +163,13 @@ export class ManageContentful implements ManageCms {
return
}
fieldEntry[context.locale] = fieldEntry[fromLocale]
await this.updateEntry(context, oldEntry)
await this.writeEntry(context, oldEntry)
}

async updateEntry(context: ManageContext, entry: Entry): Promise<void> {
private async writeEntry(
context: ManageContext,
entry: Entry
): Promise<void> {
if (context.dryRun) {
console.log('Not updating due to dryRun mode')
return
Expand All @@ -143,4 +179,18 @@ export class ManageContentful implements ManageCms {
await updated.publish()
}
}

private async writeAsset(
context: ManageContext,
asset: Asset
): Promise<void> {
if (context.dryRun) {
console.log('Not updating due to dryRun mode')
return
}
const updated = await asset.update()
if (!context.preview) {
await updated.publish()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class ErrorReportingManageCms implements ManageCms {
value: any
): Promise<void> {
return this.manageCms
.updateField<T>(context, contentId, fieldType, value)
.updateField(context, contentId, fieldType, value)
.catch(this.handleError('updateField', contentId))
}

Expand All @@ -30,7 +30,7 @@ export class ErrorReportingManageCms implements ManageCms {
onlyIfTargetEmpty: boolean
): Promise<void> {
return this.manageCms
.copyField<T>(context, contentId, field, fromLocale, onlyIfTargetEmpty)
.copyField(context, contentId, field, fromLocale, onlyIfTargetEmpty)
.catch(this.handleError('copyField', contentId))
}

Expand All @@ -53,4 +53,20 @@ export class ErrorReportingManageCms implements ManageCms {
.getDefaultLocale()
.catch(this.handleError('defaultLocale'))
}

copyAssetFile(
context: ManageContext,
assetId: string,
fromLocale: Locale
): Promise<void> {
return this.manageCms
.copyAssetFile(context, assetId, fromLocale)
.catch(this.handleError('copyAssetFile'))
}

removeAssetFile(context: ManageContext, assetId: string): Promise<void> {
return this.manageCms
.removeAssetFile(context, assetId)
.catch(this.handleError('removeAssetFile'))
}
}
Loading