Entry deletion for the simple workflow (#485)

src/actions/entries.js

@@ -30,6 +30,10 @@ export const ENTRY_PERSIST_REQUEST = 'ENTRY_PERSIST_REQUEST';
 export const ENTRY_PERSIST_SUCCESS = 'ENTRY_PERSIST_SUCCESS';
 export const ENTRY_PERSIST_FAILURE = 'ENTRY_PERSIST_FAILURE';
 
+export const ENTRY_DELETE_REQUEST = 'ENTRY_DELETE_REQUEST';
+export const ENTRY_DELETE_SUCCESS = 'ENTRY_DELETE_SUCCESS';
+export const ENTRY_DELETE_FAILURE = 'ENTRY_DELETE_FAILURE';
+
 /*
  * Simple Action Creators (Internal)
  * We still need to export them for tests
@@ -126,6 +130,37 @@ export function entryPersistFail(collection, entry, error) {
   };
 }
 
+export function entryDeleting(collection, slug) {
+  return {
+    type: ENTRY_DELETE_REQUEST,
+    payload: {
+      collectionName: collection.get('name'),
+      entrySlug: slug,
+    },
+  };
+}
+
+export function entryDeleted(collection, slug) {
+  return {
+    type: ENTRY_DELETE_SUCCESS,
+    payload: {
+      collectionName: collection.get('name'),
+      entrySlug: slug,
+    },
+  };
+}
+
+export function entryDeleteFail(collection, slug, error) {
+  return {
+    type: ENTRY_DELETE_FAILURE,
+    payload: {
+      collectionName: collection.get('name'),
+      entrySlug: slug,
+      error: error.toString(),
+    },
+  };
+}
+
 export function emptyDraftCreated(entry) {
   return {
     type: DRAFT_CREATE_EMPTY,
@@ -229,13 +264,13 @@ export function persistEntry(collection) {
     const entryDraft = state.entryDraft;
 
     // Early return if draft contains validation errors
-    if (!entryDraft.get('fieldsErrors').isEmpty()) return;
+    if (!entryDraft.get('fieldsErrors').isEmpty()) return Promise.reject();
     
     const backend = currentBackend(state.config);
     const assetProxies = entryDraft.get('mediaFiles').map(path => getAsset(state, path));
     const entry = entryDraft.get('entry');
     dispatch(entryPersisting(collection, entry));
-    backend
+    return backend
       .persistEntry(state.config, collection, entryDraft, assetProxies.toJS())
       .then(() => {
         dispatch(notifSend({
@@ -243,8 +278,7 @@ export function persistEntry(collection) {
           kind: 'success',
           dismissAfter: 4000,
         }));
-        dispatch(entryPersisted(collection, entry));
-        dispatch(closeEntry(collection));
+        return dispatch(entryPersisted(collection, entry));
       })
       .catch((error) => {
         dispatch(notifSend({
@@ -252,7 +286,29 @@ export function persistEntry(collection) {
           kind: 'danger',
           dismissAfter: 8000,
         }));
-        dispatch(entryPersistFail(collection, entry, error));
+        return dispatch(entryPersistFail(collection, entry, error));
       });
   };
 }
+
+export function deleteEntry(collection, slug) {
+  return (dispatch, getState) => {
+    const state = getState();
+    const backend = currentBackend(state.config);
+
+    dispatch(entryDeleting(collection, slug));
+    return backend.deleteEntry(state.config, collection, slug)
+    .then(() => {
+      return dispatch(entryDeleted(collection, slug));
+    })
+    .catch((error) => {
+      dispatch(notifSend({
+        message: `Failed to delete entry: ${ error }`,
+        kind: 'danger',
+        dismissAfter: 8000,
+      }));
+      console.error(error);
+      return dispatch(entryDeleteFail(collection, slug, error));
+    });
+  };
+}

src/backends/backend.js

@@ -230,6 +230,12 @@ class Backend {
     });
   }
 
+  deleteEntry(config, collection, slug) {
+    const path = selectEntryPath(collection, slug);
+    const commitMessage = `Delete ${ collection.get('label') } “${ slug }”`;
+    return this.implementation.deleteFile(path, commitMessage);
+  }
+
   persistUnpublishedEntry(config, collection, entryDraft, MediaFiles) {
     return this.persistEntry(config, collection, entryDraft, MediaFiles, { unpublished: true });
   }

src/backends/github/API.js

@@ -201,7 +201,12 @@ export default class API {
       // Get PRs with a `head` of `branchName`. Note that this is a
       // substring match, so we need to check that the `head.ref` of
       // at least one of the returned objects matches `branchName`.
-      return this.request(`${ this.repoURL }/pulls?head=${ branchName }&state=open`)
+      return this.request(`${ this.repoURL }/pulls`, {
+        params: {
+          head: branchName,
+          state: 'open',
+        },
+      })
         .then(prs => prs.some(pr => pr.head.ref === branchName));
     }))
     .catch((error) => {
@@ -258,6 +263,21 @@ export default class API {
     });
   }
 
+  deleteFile(path, message, options={}) {
+    const branch = options.branch || this.branch;
+    const fileURL = `${ this.repoURL }/contents/${ path }`;
+    // We need to request the file first to get the SHA
+    return this.request(fileURL)
+    .then(({ sha }) => this.request(fileURL, {
+      method: "DELETE",
+      params: {
+        sha,
+        message,
+        branch,
+      },
+    }));
+  }
+
   editorialWorkflowGit(fileTree, entry, filesList, options) {
     const contentKey = entry.slug;
     const branchName = `cms/${ contentKey }`;
@@ -343,10 +363,18 @@ export default class API {
 
   deleteUnpublishedEntry(collection, slug) {
     const contentKey = slug;
-    let prNumber; 
     return this.retrieveMetadata(contentKey)
     .then(metadata => this.closePR(metadata.pr, metadata.objects))
-    .then(() => this.deleteBranch(`cms/${ contentKey }`));
+    .then(() => this.deleteBranch(`cms/${ contentKey }`))
+    // If the PR doesn't exist, then this has already been deleted -
+    // deletion should be idempotent, so we can consider this a
+    // success.
+    .catch((err) => {
+      if (err.message === "Reference does not exist") {
+        return Promise.resolve();
+      }
+      return Promise.reject(err);
+    });
   }
 
   publishUnpublishedEntry(collection, slug) {
@@ -374,7 +402,7 @@ export default class API {
 
   deleteRef(type, name, sha) {
     return this.request(`${ this.repoURL }/git/refs/${ type }/${ encodeURIComponent(name) }`, {
-      method: "DELETE",
+      method: 'DELETE',
     });
   }
 

src/backends/github/implementation.js

@@ -83,6 +83,10 @@ export default class GitHub {
     return this.api.persistFiles(entry, mediaFiles, options);
   }
 
+  deleteFile(path, commitMessage, options) {
+    return this.api.deleteFile(path, commitMessage, options);
+  }
+
   unpublishedEntries() {
     return this.api.listUnpublishedBranches().then((branches) => {
       const sem = semaphore(MAX_CONCURRENT_DOWNLOADS);

src/backends/test-repo/implementation.js

@@ -93,4 +93,10 @@ export default class TestRepo {
     return Promise.resolve();
   }
 
+  deleteEntry(path, commitMessage) {
+    const folder = path.substring(0, path.lastIndexOf('/'));
+    const fileName = path.substring(path.lastIndexOf('/') + 1);
+    window.repoFiles[folder][fileName] = undefined;
+    return Promise.resolve();
+  }
 }

src/components/EntryEditor/EntryEditor.js

@@ -47,6 +47,8 @@ class EntryEditor extends Component {
         fieldsErrors,
         getAsset,
         onChange,
+        showDelete,
+        onDelete,
         onValidate,
         onAddAsset,
         onRemoveAsset,
@@ -127,6 +129,8 @@ class EntryEditor extends Component {
             isPersisting={entry.get('isPersisting')}
             onPersist={this.handleOnPersist}
             onCancelEdit={onCancelEdit}
+            onDelete={onDelete}
+            showDelete={showDelete}
           />
         </div>
       </div>
@@ -145,6 +149,8 @@ EntryEditor.propTypes = {
   onChange: PropTypes.func.isRequired,
   onValidate: PropTypes.func.isRequired,
   onPersist: PropTypes.func.isRequired,
+  showDelete: PropTypes.bool.isRequired,
+  onDelete: PropTypes.func.isRequired,
   onRemoveAsset: PropTypes.func.isRequired,
   onCancelEdit: PropTypes.func.isRequired,
 };

src/components/EntryEditor/EntryEditorToolbar.js

@@ -5,6 +5,8 @@ const EntryEditorToolbar = (
   {
     isPersisting,
     onPersist,
+    showDelete,
+    onDelete,
     onCancelEdit,
   }) => {
   const disabled = isPersisting;
@@ -19,6 +21,12 @@ const EntryEditorToolbar = (
         { isPersisting ? 'Saving...' : 'Save' }
       </Button>
       {' '}
+      { showDelete
+        ? (<Button accent onClick={onDelete}>
+            Delete
+           </Button>)
+        : '' }
+      { showDelete ? ' ' : '' }
       <Button onClick={onCancelEdit}>
         Cancel
       </Button>
@@ -29,6 +37,8 @@ const EntryEditorToolbar = (
 EntryEditorToolbar.propTypes = {
   isPersisting: PropTypes.bool,
   onPersist: PropTypes.func.isRequired,
+  showDelete: PropTypes.bool.isRequired,
+  onDelete: PropTypes.func.isRequired,
   onCancelEdit: PropTypes.func.isRequired,
 };
 

src/components/EntryEditor/__tests__/EntryEditorToolbar.spec.js

@@ -8,6 +8,7 @@ describe('EntryEditorToolbar', () => {
       <EntryEditorToolbar
         onPersist={() => {}}
         onCancelEdit={() => {}}
+        onDelete={() => {}}
       />
     );
     const tree = component.html();
@@ -20,6 +21,7 @@ describe('EntryEditorToolbar', () => {
         isPersisting
         onPersist={() => {}}
         onCancelEdit={() => {}}
+        onDelete={() => {}}
       />
     );
     const tree = component.html();

src/containers/EntryPage.js

@@ -10,6 +10,7 @@ import {
   changeDraftField,
   changeDraftFieldValidation,
   persistEntry,
+  deleteEntry,
 } from '../actions/entries';
 import { closeEntry } from '../actions/editor';
 import { addAsset, removeAsset } from '../actions/media';
@@ -34,6 +35,8 @@ class EntryPage extends React.Component {
     entryDraft: ImmutablePropTypes.map.isRequired,
     loadEntry: PropTypes.func.isRequired,
     persistEntry: PropTypes.func.isRequired,
+    deleteEntry: PropTypes.func.isRequired,
+    showDelete: PropTypes.bool.isRequired,
     removeAsset: PropTypes.func.isRequired,
     closeEntry: PropTypes.func.isRequired,
     openSidebar: PropTypes.func.isRequired,
@@ -79,16 +82,29 @@ class EntryPage extends React.Component {
   };
 
   handleCloseEntry = () => {
-    this.props.closeEntry();
+    return this.props.closeEntry();
   };
 
   handlePersistEntry = () => {
     const { persistEntry, collection } = this.props;
     setTimeout(() => {
-      persistEntry(collection);  
+      persistEntry(collection).then(() => this.handleCloseEntry());
     }, 0);
   };
 
+  handleDeleteEntry = () => {
+    if (!window.confirm('Are you sure you want to delete this entry?')) { return; }
+    if (this.props.newEntry) {
+      return this.handleCloseEntry();
+    }
+
+    const { deleteEntry, entry, collection } = this.props;
+    const slug = entry.get('slug');
+    setTimeout(() => {
+      deleteEntry(collection, slug).then(() => this.handleCloseEntry());
+    }, 0);
+  }
+
   render() {
     const {
       entry,
@@ -124,6 +140,8 @@ class EntryPage extends React.Component {
         onAddAsset={addAsset}
         onRemoveAsset={removeAsset}
         onPersist={this.handlePersistEntry}
+        onDelete={this.handleDeleteEntry}
+        showDelete={this.props.showDelete}
         onCancelEdit={this.handleCloseEntry}
       />
     );
@@ -162,6 +180,7 @@ export default connect(
     createEmptyDraft,
     discardDraft,
     persistEntry,
+    deleteEntry,
     closeEntry,
     openSidebar,
   }

src/containers/editorialWorkflow/EntryPageHOC.js

@@ -15,8 +15,9 @@ export default function EntryPageHOC(EntryPage) {
   function mapStateToProps(state, ownProps) {
     const { collections } = state;
     const isEditorialWorkflow = (state.config.get('publish_mode') === EDITORIAL_WORKFLOW);
-    const returnObj = { isEditorialWorkflow };
+    const returnObj = { isEditorialWorkflow, showDelete: !ownProps.newEntry };
     if (isEditorialWorkflow) {
+      returnObj.showDelete = false;
       const slug = ownProps.params.slug;
       const collection = collections.get(ownProps.params.name);
       const unpublishedEntry = selectUnpublishedEntry(state, collection.get('name'), slug);
@@ -35,14 +36,16 @@ export default function EntryPageHOC(EntryPage) {
 
     if (isEditorialWorkflow) {
       // Overwrite loadEntry to loadUnpublishedEntry
-      returnObj.loadEntry = (collection, slug) => {
+      returnObj.loadEntry = (collection, slug) =>
         dispatch(loadUnpublishedEntry(collection, slug));
-      };
-      
+
       // Overwrite persistEntry to persistUnpublishedEntry
-      returnObj.persistEntry = (collection) => {
+      returnObj.persistEntry = collection =>
         dispatch(persistUnpublishedEntry(collection, unpublishedEntry));
-      };
+
+      // Overwrite deleteEntry to a noop (deletion is currently
+      // disabled in the editorial workflow)
+      returnObj.deleteEntry = () => undefined;
     }
 
     return {

src/reducers/entries.js

@@ -6,6 +6,7 @@ import {
   ENTRIES_REQUEST,
   ENTRIES_SUCCESS,
   ENTRIES_FAILURE,
+  ENTRY_DELETE_SUCCESS,
 } from '../actions/entries';
 
 import { SEARCH_ENTRIES_SUCCESS } from '../actions/search';
@@ -61,6 +62,13 @@ const entries = (state = Map({ entities: Map(), pages: Map() }), action) => {
         ));
       });
 
+    case ENTRY_DELETE_SUCCESS:
+      return state.withMutations((map) => {
+        map.deleteIn(['entities', `${ action.payload.collectionName }.${ action.payload.entrySlug }`]);
+        map.updateIn(['pages', action.payload.collectionName, 'ids'],
+          ids => ids.filter(id => id !== action.payload.entrySlug));
+      });
+
     default:
       return state;
   }

src/reducers/entryDraft.js

@@ -8,6 +8,7 @@ import {
   ENTRY_PERSIST_REQUEST,
   ENTRY_PERSIST_SUCCESS,
   ENTRY_PERSIST_FAILURE,
+  ENTRY_DELETE_SUCCESS,
 } from '../actions/entries';
 import {
   UNPUBLISHED_ENTRY_PERSIST_REQUEST,
@@ -76,6 +77,7 @@ const entryDraftReducer = (state = Map(), action) => {
     }
 
     case ENTRY_PERSIST_SUCCESS:
+    case ENTRY_DELETE_SUCCESS:
     case UNPUBLISHED_ENTRY_PERSIST_SUCCESS:
       return state.withMutations((state) => {
         state.deleteIn(['entry', 'isPersisting']);