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

feat: remove deleted image from content #7310

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions packages/decap-cms-core/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ declare module 'decap-cms-core' {
slug?: CmsSlug;
i18n?: CmsI18nConfig;
local_backend?: boolean | CmsLocalBackend;
remove_empty_image_field?: boolean;
editor?: {
preview?: boolean;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/decap-cms-core/src/actions/editorialWorkflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ export function persistUnpublishedEntry(collection: Collection, existingUnpublis
entry,
});

const serializedEntry = getSerializedEntry(collection, entry);
const serializedEntry = getSerializedEntry(collection, entry, state.config);
const serializedEntryDraft = entryDraft.set('entry', serializedEntry);

dispatch(unpublishedEntryPersisting(collection, entry.get('slug')));
Expand Down
7 changes: 4 additions & 3 deletions packages/decap-cms-core/src/actions/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type {
ViewFilter,
ViewGroup,
Entry,
CmsConfig,
} from '../types/redux';
import type { EntryValue } from '../valueObjects/Entry';
import type { Backend } from '../backend';
Expand Down Expand Up @@ -856,7 +857,7 @@ export function getMediaAssets({ entry }: { entry: EntryMap }) {
return assets;
}

export function getSerializedEntry(collection: Collection, entry: Entry) {
export function getSerializedEntry(collection: Collection, entry: Entry, config: CmsConfig) {
/**
* Serialize the values of any fields with registered serializers, and
* update the entry and entryDraft with the serialized values.
Expand All @@ -865,7 +866,7 @@ export function getSerializedEntry(collection: Collection, entry: Entry) {

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function serializeData(data: any) {
return serializeValues(data, fields);
return serializeValues(data, fields, config);
}

const serializedData = serializeData(entry.get('data'));
Expand Down Expand Up @@ -910,7 +911,7 @@ export function persistEntry(collection: Collection) {
entry,
});

const serializedEntry = getSerializedEntry(collection, entry);
const serializedEntry = getSerializedEntry(collection, entry, state.config);
const serializedEntryDraft = entryDraft.set('entry', serializedEntry);
dispatch(entryPersisting(collection, serializedEntry));
return backend
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,18 @@ describe('config', () => {
}).not.toThrowError();
});

it('should throw if remove_empty_image_field is not a boolean', () => {
expect(() => {
validateConfig(merge({}, validConfig, { remove_empty_image_field: 'false' }));
}).toThrowError("'remove_empty_image_field' must be boolean");
});

it('should not throw if remove_empty_image_field is a boolean', () => {
expect(() => {
validateConfig(merge({}, validConfig, { remove_empty_image_field: false }));
}).not.toThrowError();
});

it('should throw if collection publish is not a boolean', () => {
expect(() => {
validateConfig(merge({}, validConfig, { collections: [{ publish: 'false' }] }));
Expand Down
1 change: 1 addition & 0 deletions packages/decap-cms-core/src/constants/configSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ function getConfigSchema() {
},
],
},
remove_empty_image_field: { type: 'boolean' },
locale: { type: 'string', examples: ['en', 'fr', 'de'] },
i18n: i18nRoot,
site_url: { type: 'string', examples: ['https://example.com'] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@ import { fromJS } from 'immutable';

import { serializeValues, deserializeValues } from '../serializeEntryValues';

const values = fromJS({ title: 'New Post', unknown: 'Unknown Field' });
const fields = fromJS([{ name: 'title', widget: 'string' }]);
const values = fromJS({ title: 'New Post', unknown: 'Unknown Field', removed_image: '' });
const fields = fromJS([
{ name: 'title', widget: 'string' },
{ name: 'removed_image', widget: 'image' },
]);

describe('serializeValues', () => {
it('should retain unknown fields', () => {
expect(serializeValues(values, fields)).toEqual(
fromJS({ title: 'New Post', unknown: 'Unknown Field', removed_image: '' }),
);
});

it('should remove image field', () => {
expect(serializeValues(values, fields, { remove_empty_image_field: true })).toEqual(
fromJS({ title: 'New Post', unknown: 'Unknown Field' }),
);
});
Expand All @@ -16,7 +25,7 @@ describe('serializeValues', () => {
describe('deserializeValues', () => {
it('should retain unknown fields', () => {
expect(deserializeValues(values, fields)).toEqual(
fromJS({ title: 'New Post', unknown: 'Unknown Field' }),
fromJS({ title: 'New Post', unknown: 'Unknown Field', removed_image: '' }),
);
});
});
51 changes: 45 additions & 6 deletions packages/decap-cms-core/src/lib/serializeEntryValues.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { Map, List } from 'immutable';

import { getWidgetValueSerializer } from './registry';

const _pathsToRemove = new Set();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could find a way to make this non-global?


/**
* Methods for serializing/deserializing entry field values. Most widgets don't
* require this for their values, and those that do can typically serialize/
Expand All @@ -21,7 +23,7 @@ import { getWidgetValueSerializer } from './registry';
* registered deserialization handlers run on entry load, and serialization
* handlers run on persist.
*/
function runSerializer(values, fields, method) {
function runSerializer(values, fields, method, config = {}, isRecursive = false, currentPath = '') {
/**
* Reduce the list of fields to a map where keys are field names and values
* are field values, serializing the values of fields whose widgets have
Expand All @@ -33,25 +35,33 @@ function runSerializer(values, fields, method) {
const value = values.get(fieldName);
const serializer = getWidgetValueSerializer(field.get('widget'));
const nestedFields = field.get('fields');
const newPath = currentPath ? `${currentPath}.${fieldName}` : fieldName;

// Call recursively for fields within lists
if (nestedFields && List.isList(value)) {
return acc.set(
fieldName,
value.map(val => runSerializer(val, nestedFields, method)),
value.map((val, index) =>
runSerializer(val, nestedFields, method, config, true, `${newPath}.${index}`),
),
);
}

// Call recursively for fields within objects
if (nestedFields && Map.isMap(value)) {
return acc.set(fieldName, runSerializer(value, nestedFields, method));
return acc.set(fieldName, runSerializer(value, nestedFields, method, config, true, newPath));
}

// Run serialization method on value if not null or undefined
if (serializer && !isNil(value)) {
return acc.set(fieldName, serializer[method](value));
}

// If widget is image with no value set, flag field for removal
if (config.remove_empty_image_field && !value && field.get('widget') === 'image') {
_pathsToRemove.add(newPath);
}

// If no serializer is registered for the field's widget, use the field as is
if (!isNil(value)) {
return acc.set(fieldName, value);
Expand All @@ -60,14 +70,43 @@ function runSerializer(values, fields, method) {
return acc;
}, Map());

//preserve unknown fields value
// preserve unknown fields value
serializedData = values.mergeDeep(serializedData);

// Remove only on the top level, otherwise `mergeDeep` will reinsert them.
if (config.remove_empty_image_field && !isRecursive) {
serializedData = removeEntriesByPaths(serializedData, _pathsToRemove);
_pathsToRemove.clear();
}

return serializedData;
}

export function serializeValues(values, fields) {
return runSerializer(values, fields, 'serialize');
function removeEntriesByPaths(data, paths) {
paths.forEach(path => {
data = removeEntryByPath(data, path.split('.'));
});
return data;
}

function removeEntryByPath(data, keys) {
if (keys.length === 1) {
return data.delete(keys[0]);
}

const [firstKey, ...restKeys] = keys;
const nestedData = data.get(firstKey);

if (nestedData) {
const updatedNestedData = removeEntryByPath(nestedData, restKeys);
return data.set(firstKey, updatedNestedData);
}

return data;
}

export function serializeValues(values, fields, config) {
return runSerializer(values, fields, 'serialize', config);
}

export function deserializeValues(values, fields) {
Expand Down
1 change: 1 addition & 0 deletions packages/decap-cms-core/src/types/redux.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ export interface CmsConfig {
slug?: CmsSlug;
i18n?: CmsI18nConfig;
local_backend?: boolean | CmsLocalBackend;
remove_empty_image_field?: boolean;
editor?: {
preview?: boolean;
};
Expand Down
Loading