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.

Sign up for GitHub

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

Prevent Overwriting Files with Same Slug Names #1239

Merged
merged 7 commits into from
May 23, 2018
Merged

Prevent Overwriting Files with Same Slug Names #1239

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
447b473
adds error throwing if a file is in danger of being overwritten
brianlmacdonald Apr 9, 2018
b7898f7
uses implementation.getentry to avoid front-matter error
brianlmacdonald Apr 9, 2018
7e9ba02
removes catch in overwrite so correct error always displays
brianlmacdonald Apr 9, 2018
9d2acc6
uses unpublishedEntry for checkOverwrite
brianlmacdonald Apr 9, 2018
d2c8074
small format tweak
brianlmacdonald Apr 9, 2018
b449760
catch any valid identifier field
erquhart May 14, 2018
3c02916
improve key find logic
erquhart May 23, 2018
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
75 changes: 57 additions & 18 deletions src/backends/backend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import TestRepoBackend from "./test-repo/implementation";
import GitHubBackend from "./github/implementation";
import GitGatewayBackend from "./git-gateway/implementation";
import { registerBackend, getBackend } from 'Lib/registry';
import { EditorialWorkflowError } from "ValueObjects/errors";

/**
* Register internal backends
Expand All @@ -23,7 +24,6 @@ registerBackend('git-gateway', GitGatewayBackend);
registerBackend('github', GitHubBackend);
registerBackend('test-repo', TestRepoBackend);


class LocalStorageAuthStore {
storageKey = "netlify-cms-user";

Expand All @@ -41,23 +41,28 @@ class LocalStorageAuthStore {
}
}

const slugFormatter = (template = "{{slug}}", entryData, slugConfig) => {
const date = new Date();
const validIdentifierFields = ["title", "path"];

const getIdentifier = (entryData) => {
const validIdentifierFields = ["title", "path"];
const identifiers = validIdentifierFields.map((field) =>
entryData.find((_, key) => key.toLowerCase().trim() === field)
);

const identifier = identifiers.find(ident => ident !== undefined);
const getIdentifierKey = entryData => {
const keys = validIdentifierFields.map(field => {
return entryData.findKey((_, key) => {
return key.toLowerCase().trim() === field;
});
});
return keys.find(key => entryData.get(key) !== undefined);
Copy link
Contributor

@tech4him1 tech4him1 May 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Is there a reason we can't use the old logic here, where we return the values (on line 48), instead of having to run entryData.get again here? Maybe I'm mis-reading the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

const keyVals = validIdentifierFields.map(field => {
  return entryData.find((_, key) => {
    return key.toLowerCase().trim() === field;
  });
});
return keyVals.find(key => key !== undefined);

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I believe I was thinking we were lowercasing and trimming the return value for some reason, but that isn't the case.

};

if (identifier === undefined) {
throw new Error("Collection must have a field name that is a valid entry identifier");
}
const getIdentifier = entryData => {
const key = getIdentifierKey(entryData);
const identifier = entryData.get(key);
if (identifier === undefined) {
throw new Error("Collection must have a field name that is a valid entry identifier");
}
return identifier;
};

return identifier;
};
const slugFormatter = (template = "{{slug}}", entryData, slugConfig) => {
const date = new Date();

const slug = template.replace(/\{\{([^\}]+)\}\}/g, (_, field) => {
switch (field) {
Expand Down Expand Up @@ -234,7 +239,37 @@ class Backend {
.then(this.entryWithFormat(collection, slug));
}

persistEntry(config, collection, entryDraft, MediaFiles, integrations, options = {}) {
async checkOverwrite(collection, slug, path, entryData) {
const identifierKey = getIdentifierKey(entryData);
const identifierField = collection.get('fields').find(field => {
return field.get('name') === identifierKey;
});
const identifierLabel = identifierField.get('label');
const errorMessage = `\
Duplicate filename found. Please ensure the ${identifierLabel} field is unique from other entries \
in the ${collection.get('name')} collection.\
`;

const existingEntry = await this.unpublishedEntry(collection, slug).catch(error => {
if (error instanceof EditorialWorkflowError && error.notUnderEditorialWorkflow) {
return Promise.resolve(false);
}
return Promise.reject(error);
});

if (existingEntry) {
throw new Error(errorMessage);
}

const publishedEntry = await this.implementation.getEntry(collection, slug, path)
.then(({ data }) => data);

if (publishedEntry) {
throw new Error(errorMessage);
}
}

async persistEntry(config, collection, entryDraft, MediaFiles, integrations, options = {}) {
const newEntry = entryDraft.getIn(["entry", "newRecord"]) || false;

const parsedData = {
Expand All @@ -248,12 +283,16 @@ class Backend {
if (!selectAllowNewEntries(collection)) {
throw (new Error("Not allowed to create new entries in this collection"));
}
const slug = slugFormatter(collection.get("slug"), entryDraft.getIn(["entry", "data"]), config.get("slug"));
const entryData = entryDraft.getIn(['entry', 'data']);
const slug = slugFormatter(collection.get('slug'), entryData, config.get('slug'));
const path = selectEntryPath(collection, slug);

await this.checkOverwrite(collection, slug, path, entryData);

entryObj = {
path,
slug,
raw: this.entryToRaw(collection, entryDraft.get("entry")),
raw: this.entryToRaw(collection, entryDraft.get('entry')),
};
} else {
const path = entryDraft.getIn(["entry", "path"]);
Expand Down