Skip to content

Commit

Permalink
fix: don't use getAsset for draft entries (#3403)
Browse files Browse the repository at this point in the history
  • Loading branch information
erezrokah committed Mar 22, 2020
1 parent e92ba41 commit 45a1654
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 120 deletions.
12 changes: 9 additions & 3 deletions packages/netlify-cms-core/src/__tests__/backend.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,17 +349,23 @@ describe('Backend', () => {
init: jest.fn(() => implementation),
unpublishedEntry: jest.fn().mockResolvedValue(unpublishedEntryResult),
};
const config = Map({});
const config = Map({ media_folder: 'static/images' });

const backend = new Backend(implementation, { config, backendName: 'github' });

const collection = Map({
name: 'posts',
});

const state = {
config,
integrations: Map({}),
mediaLibrary: Map({}),
};

const slug = 'slug';

const result = await backend.unpublishedEntry(collection, slug);
const result = await backend.unpublishedEntry(state, collection, slug);
expect(result).toEqual({
collection: 'posts',
slug: '',
Expand All @@ -370,7 +376,7 @@ describe('Backend', () => {
label: null,
metaData: {},
isModification: true,
mediaFiles: [{ id: '1' }],
mediaFiles: [{ id: '1', draft: true }],
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('editorialWorkflow actions', () => {
const { createAssetProxy } = require('ValueObjects/AssetProxy');

const assetProxy = { name: 'name', path: 'path' };
const entry = { mediaFiles: [{ file: { name: 'name' }, id: '1' }] };
const entry = { mediaFiles: [{ file: { name: 'name' }, id: '1', draft: true }] };
const backend = {
unpublishedEntry: jest.fn().mockResolvedValue(entry),
};
Expand Down
34 changes: 13 additions & 21 deletions packages/netlify-cms-core/src/actions/__tests__/entries.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,9 @@ import {
} from '../entries';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import AssetProxy from '../../valueObjects/AssetProxy';

jest.mock('coreSrc/backend');
jest.mock('../media', () => {
const media = jest.requireActual('../media');
return {
...media,
getAsset: jest.fn(),
};
});
jest.mock('netlify-cms-lib-util');
jest.mock('../mediaLibrary');

Expand All @@ -25,6 +19,13 @@ const mockStore = configureMockStore(middlewares);

describe('entries', () => {
describe('createEmptyDraft', () => {
const { currentBackend } = require('coreSrc/backend');
const backend = {
processEntry: jest.fn((_state, _collection, entry) => Promise.resolve(entry)),
};

currentBackend.mockReturnValue(backend);

beforeEach(() => {
jest.clearAllMocks();
});
Expand All @@ -45,7 +46,7 @@ describe('entries', () => {
data: {},
isModification: null,
label: null,
mediaFiles: fromJS([]),
mediaFiles: [],
metaData: null,
partial: false,
path: '',
Expand Down Expand Up @@ -74,7 +75,7 @@ describe('entries', () => {
data: { title: 'title', boolean: true },
isModification: null,
label: null,
mediaFiles: fromJS([]),
mediaFiles: [],
metaData: null,
partial: false,
path: '',
Expand Down Expand Up @@ -105,7 +106,7 @@ describe('entries', () => {
data: { title: '<script>alert('hello')</script>' },
isModification: null,
label: null,
mediaFiles: fromJS([]),
mediaFiles: [],
metaData: null,
partial: false,
path: '',
Expand Down Expand Up @@ -286,20 +287,11 @@ describe('entries', () => {
jest.clearAllMocks();
});

it('should map mediaFiles to assets', async () => {
const { getAsset } = require('../media');
it('should map mediaFiles to assets', () => {
const mediaFiles = fromJS([{ path: 'path1' }, { path: 'path2', draft: true }]);

const asset = { path: 'path1' };

getAsset.mockReturnValue(() => asset);

const collection = Map();
const entry = Map({ mediaFiles });
await expect(getMediaAssets({ entry, collection })).resolves.toEqual([asset]);

expect(getAsset).toHaveBeenCalledTimes(1);
expect(getAsset).toHaveBeenCalledWith({ collection, path: 'path2', entry });
expect(getMediaAssets({ entry })).toEqual([new AssetProxy({ path: 'path2' })]);
});
});
});
32 changes: 12 additions & 20 deletions packages/netlify-cms-core/src/actions/editorialWorkflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,25 +271,20 @@ export function loadUnpublishedEntry(collection: Collection, slug: string) {
dispatch(unpublishedEntryLoading(collection, slug));

try {
const entry = (await backend.unpublishedEntry(collection, slug)) as EntryValue;
const entry = (await backend.unpublishedEntry(state, collection, slug)) as EntryValue;
const assetProxies = await Promise.all(
entry.mediaFiles.map(({ url, file, path }) =>
createAssetProxy({
path,
url,
file,
}),
),
entry.mediaFiles
.filter(file => file.draft)
.map(({ url, file, path }) =>
createAssetProxy({
path,
url,
file,
}),
),
);
dispatch(addAssets(assetProxies));

let mediaFiles: MediaFile[] = entry.mediaFiles.map(file => ({ ...file, draft: true }));
if (!collection.has('media_folder')) {
const libraryFiles = getState().mediaLibrary.get('files') || [];
mediaFiles = mediaFiles.concat(libraryFiles);
}

dispatch(unpublishedEntryLoaded(collection, { ...entry, mediaFiles }));
dispatch(unpublishedEntryLoaded(collection, entry));
dispatch(createDraftFromEntry(entry));
} catch (error) {
if (error.name === EDITORIAL_WORKFLOW_ERROR && error.notUnderEditorialWorkflow) {
Expand Down Expand Up @@ -375,10 +370,7 @@ export function persistUnpublishedEntry(collection: Collection, existingUnpublis
const backend = currentBackend(state.config);
const transactionID = uuid();
const entry = entryDraft.get('entry');
const assetProxies = await getMediaAssets({
getState,
dispatch,
collection,
const assetProxies = getMediaAssets({
entry,
});

Expand Down
47 changes: 17 additions & 30 deletions packages/netlify-cms-core/src/actions/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { createEntry, EntryValue } from '../valueObjects/Entry';
import AssetProxy, { createAssetProxy } from '../valueObjects/AssetProxy';
import ValidationErrorTypes from '../constants/validationErrorTypes';
import { addAssets, getAsset } from './media';
import { Collection, EntryMap, MediaFile, State, EntryFields, EntryField } from '../types/redux';
import { Collection, EntryMap, State, EntryFields, EntryField } from '../types/redux';
import { ThunkDispatch } from 'redux-thunk';
import { AnyAction, Dispatch } from 'redux';
import { AnyAction } from 'redux';
import { waitForMediaLibraryToLoad, loadMedia } from './mediaLibrary';
import { waitUntil } from './waitUntil';

Expand Down Expand Up @@ -524,13 +524,18 @@ export function createEmptyDraft(collection: Collection, search: string) {
const fields = collection.get('fields', List());
const dataFields = createEmptyDraftData(fields);

let mediaFiles = [] as MediaFile[];
const state = getState();
const backend = currentBackend(state.config);

if (!collection.has('media_folder')) {
await waitForMediaLibraryToLoad(dispatch, getState());
mediaFiles = getState().mediaLibrary.get('files');
}

const newEntry = createEntry(collection.get('name'), '', '', { data: dataFields, mediaFiles });
let newEntry = createEntry(collection.get('name'), '', '', {
data: dataFields,
mediaFiles: [],
});
newEntry = await backend.processEntry(state, collection, newEntry);
dispatch(emptyDraftCreated(newEntry));
};
}
Expand Down Expand Up @@ -592,28 +597,13 @@ export function createEmptyDraftData(fields: EntryFields, withNameKey = true) {
);
}

export async function getMediaAssets({
getState,
dispatch,
collection,
entry,
}: {
getState: () => State;
collection: Collection;
entry: EntryMap;
dispatch: Dispatch;
}) {
export function getMediaAssets({ entry }: { entry: EntryMap }) {
const filesArray = entry.get('mediaFiles').toArray();
const assets = await Promise.all(
filesArray
.filter(file => file.get('draft'))
.map(file =>
getAsset({ collection, entry, path: file.get('path'), field: file.get('field') })(
dispatch,
getState,
),
),
);
const assets = filesArray
.filter(file => file.get('draft'))
.map(file =>
createAssetProxy({ path: file.get('path'), file: file.get('file'), url: file.get('url') }),
);

return assets;
}
Expand Down Expand Up @@ -648,10 +638,7 @@ export function persistEntry(collection: Collection) {

const backend = currentBackend(state.config);
const entry = entryDraft.get('entry');
const assetProxies = await getMediaAssets({
getState,
dispatch,
collection,
const assetProxies = getMediaAssets({
entry,
});

Expand Down
1 change: 1 addition & 0 deletions packages/netlify-cms-core/src/actions/mediaLibrary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ function createMediaFileFromAsset({
name: basename(assetProxy.path),
displayURL: assetProxy.url,
draft,
file,
size: file.size,
url: assetProxy.url,
path: assetProxy.path,
Expand Down
66 changes: 37 additions & 29 deletions packages/netlify-cms-core/src/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,27 +494,16 @@ export class Backend {
const path = selectEntryPath(collection, slug) as string;
const label = selectFileEntryLabel(collection, slug);

const integration = selectIntegration(state.integrations, null, 'assetStore');

const loadedEntry = await this.implementation.getEntry(path);
const entry = createEntry(collection.get('name'), slug, loadedEntry.file.path, {
let entry = createEntry(collection.get('name'), slug, loadedEntry.file.path, {
raw: loadedEntry.data,
label,
mediaFiles: [],
});

const entryWithFormat = this.entryWithFormat(collection)(entry);
const mediaFolders = selectMediaFolders(state, collection, fromJS(entryWithFormat));
if (mediaFolders.length > 0 && !integration) {
entry.mediaFiles = [];
for (const folder of mediaFolders) {
entry.mediaFiles = [...entry.mediaFiles, ...(await this.implementation.getMedia(folder))];
}
} else {
entry.mediaFiles = state.mediaLibrary.get('files') || [];
}

return entryWithFormat;
entry = this.entryWithFormat(collection)(entry);
entry = await this.processEntry(state, collection, entry);
return entry;
}

getMedia() {
Expand All @@ -536,9 +525,9 @@ export class Backend {
return Promise.reject(err);
}

entryWithFormat(collectionOrEntity: unknown) {
entryWithFormat(collection: Collection) {
return (entry: EntryValue): EntryValue => {
const format = resolveFormat(collectionOrEntity, entry);
const format = resolveFormat(collection, entry);
if (entry && entry.raw !== undefined) {
const data = (format && attempt(format.fromFile.bind(format, entry.raw))) || {};
if (isError(data)) console.error(data);
Expand Down Expand Up @@ -579,18 +568,37 @@ export class Backend {
}));
}

unpublishedEntry(collection: Collection, slug: string) {
return this.implementation!.unpublishedEntry!(collection.get('name') as string, slug)
.then(loadedEntry => {
const entry = createEntry(collection.get('name'), loadedEntry.slug, loadedEntry.file.path, {
raw: loadedEntry.data,
isModification: loadedEntry.isModification,
metaData: loadedEntry.metaData,
mediaFiles: loadedEntry.mediaFiles,
});
return entry;
})
.then(this.entryWithFormat(collection));
async processEntry(state: State, collection: Collection, entry: EntryValue) {
const integration = selectIntegration(state.integrations, null, 'assetStore');
const mediaFolders = selectMediaFolders(state, collection, fromJS(entry));
if (mediaFolders.length > 0 && !integration) {
const files = await Promise.all(
mediaFolders.map(folder => this.implementation.getMedia(folder)),
);
entry.mediaFiles = entry.mediaFiles.concat(...files);
} else {
entry.mediaFiles = entry.mediaFiles.concat(state.mediaLibrary.get('files') || []);
}

return entry;
}

async unpublishedEntry(state: State, collection: Collection, slug: string) {
const loadedEntry = await this.implementation!.unpublishedEntry!(
collection.get('name') as string,
slug,
);

let entry = createEntry(collection.get('name'), loadedEntry.slug, loadedEntry.file.path, {
raw: loadedEntry.data,
isModification: loadedEntry.isModification,
metaData: loadedEntry.metaData,
mediaFiles: loadedEntry.mediaFiles?.map(file => ({ ...file, draft: true })) || [],
});

entry = this.entryWithFormat(collection)(entry);
entry = await this.processEntry(state, collection, entry);
return entry;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/netlify-cms-core/src/formats/formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ const formatByName = (name, customDelimiter) =>
'yaml-frontmatter': frontmatterYAML(customDelimiter),
}[name]);

export function resolveFormat(collectionOrEntity, entry) {
export function resolveFormat(collection, entry) {
// Check for custom delimiter
const frontmatter_delimiter = collectionOrEntity.get('frontmatter_delimiter');
const frontmatter_delimiter = collection.get('frontmatter_delimiter');
const customDelimiter = List.isList(frontmatter_delimiter)
? frontmatter_delimiter.toArray()
: frontmatter_delimiter;

// If the format is specified in the collection, use that format.
const formatSpecification = collectionOrEntity.get('format');
const formatSpecification = collection.get('format');
if (formatSpecification) {
return formatByName(formatSpecification, customDelimiter);
}
Expand All @@ -62,7 +62,7 @@ export function resolveFormat(collectionOrEntity, entry) {

// If creating a new file, and an `extension` is specified in the
// collection config, infer the format from that extension.
const extension = collectionOrEntity.get('extension');
const extension = collection.get('extension');
if (extension) {
return get(extensionFormatters, extension);
}
Expand Down

0 comments on commit 45a1654

Please sign in to comment.