From 0e1bb3bd44932a36b72065ccfdfbec9fcc924427 Mon Sep 17 00:00:00 2001
From: Andrew Nicolaou <me@andrewnicolaou.co.uk>
Date: Mon, 29 Jun 2020 11:39:35 +0200
Subject: [PATCH] Allow deleted sketches in collections to be removed (fixes
 #1465)

---
 client/modules/User/components/Collection.jsx | 68 +++++++++++--------
 client/styles/components/_sketch-list.scss    |  4 ++
 .../addProjectToCollection.js                 |  2 +-
 .../removeProjectFromCollection.js            |  2 +-
 server/models/collection.js                   |  8 +++
 5 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/client/modules/User/components/Collection.jsx b/client/modules/User/components/Collection.jsx
index 8e707d1d..54e87518 100644
--- a/client/modules/User/components/Collection.jsx
+++ b/client/modules/User/components/Collection.jsx
@@ -71,41 +71,50 @@ ShareURL.propTypes = {
   value: PropTypes.string.isRequired,
 };
 
-class CollectionItemRowBase extends React.Component {
-  handleSketchRemove = () => {
-    if (window.confirm(`Are you sure you want to remove "${this.props.item.project.name}" from this collection?`)) {
-      this.props.removeFromCollection(this.props.collection.id, this.props.item.project.id);
+const CollectionItemRowBase = ({
+  collection, item, isOwner, removeFromCollection
+}) => {
+  const projectIsDeleted = item.isDeleted;
+
+  const handleSketchRemove = () => {
+    const name = projectIsDeleted ? 'deleted sketch' : item.project.name;
+
+    if (window.confirm(`Are you sure you want to remove "${name}" from this collection?`)) {
+      removeFromCollection(collection.id, item.projectId);
     }
-  }
+  };
 
-  render() {
-    const { item } = this.props;
-    const sketchOwnerUsername = item.project.user.username;
-    const sketchUrl = `/${item.project.user.username}/sketches/${item.project.id}`;
+  const name = projectIsDeleted ? <span>Sketch was deleted</span> : (
+    <Link to={`/${item.project.user.username}/sketches/${item.projectId}`}>
+      {item.project.name}
+    </Link>
+  );
 
-    return (
-      <tr
-        className="sketches-table__row"
-      >
-        <th scope="row">
-          <Link to={sketchUrl}>
-            {item.project.name}
-          </Link>
-        </th>
-        <td>{format(new Date(item.createdAt), 'MMM D, YYYY h:mm A')}</td>
-        <td>{sketchOwnerUsername}</td>
-        <td className="collection-row__action-column ">
+  const sketchOwnerUsername = projectIsDeleted ? null : item.project.user.username;
+
+  return (
+    <tr
+      className={`sketches-table__row ${projectIsDeleted ? 'is-deleted' : ''}`}
+    >
+      <th scope="row">
+        {name}
+      </th>
+      <td>{format(new Date(item.createdAt), 'MMM D, YYYY h:mm A')}</td>
+      <td>{sketchOwnerUsername}</td>
+      <td className="collection-row__action-column ">
+        {isOwner &&
           <button
             className="collection-row__remove-button"
-            onClick={this.handleSketchRemove}
+            onClick={handleSketchRemove}
             aria-label="Remove sketch from collection"
           >
             <RemoveIcon focusable="false" aria-hidden="true" />
           </button>
-        </td>
-      </tr>);
-  }
-}
+        }
+      </td>
+    </tr>);
+};
+
 
 CollectionItemRowBase.propTypes = {
   collection: PropTypes.shape({
@@ -114,14 +123,17 @@ CollectionItemRowBase.propTypes = {
   }).isRequired,
   item: PropTypes.shape({
     createdAt: PropTypes.string.isRequired,
+    projectId: PropTypes.string.isRequired,
+    isDeleted: PropTypes.bool.isRequired,
     project: PropTypes.shape({
       id: PropTypes.string.isRequired,
       name: PropTypes.string.isRequired,
       user: PropTypes.shape({
         username: PropTypes.string.isRequired
-      })
+      }),
     }).isRequired,
   }).isRequired,
+  isOwner: PropTypes.bool.isRequired,
   user: PropTypes.shape({
     username: PropTypes.string,
     authenticated: PropTypes.bool.isRequired
@@ -342,6 +354,7 @@ class Collection extends React.Component {
 
   render() {
     const title = this.hasCollection() ? this.getCollectionName() : null;
+    const isOwner = this.isOwner();
 
     return (
       <main className="collection-container" data-has-items={this.hasCollectionItems() ? 'true' : 'false'}>
@@ -372,6 +385,7 @@ class Collection extends React.Component {
                         user={this.props.user}
                         username={this.getUsername()}
                         collection={this.props.collection}
+                        isOwner={isOwner}
                       />))}
                   </tbody>
                 </table>
diff --git a/client/styles/components/_sketch-list.scss b/client/styles/components/_sketch-list.scss
index 4a0f956d..b03381b0 100644
--- a/client/styles/components/_sketch-list.scss
+++ b/client/styles/components/_sketch-list.scss
@@ -85,6 +85,10 @@
   }
 }
 
+.sketches-table__row.is-deleted > * {
+  font-style: italic;
+}
+
 .sketches-table thead {
   font-size: #{12 / $base-font-size}rem;
   @include themify() {
diff --git a/server/controllers/collection.controller/addProjectToCollection.js b/server/controllers/collection.controller/addProjectToCollection.js
index 6abf08bf..9816661f 100644
--- a/server/controllers/collection.controller/addProjectToCollection.js
+++ b/server/controllers/collection.controller/addProjectToCollection.js
@@ -32,7 +32,7 @@ export default function addProjectToCollection(req, res) {
       return null;
     }
 
-    const projectInCollection = collection.items.find(p => p.project._id === project._id);
+    const projectInCollection = collection.items.find(p => p.projectId === project._id);
 
     if (projectInCollection) {
       sendFailure(404, 'Project already in collection');
diff --git a/server/controllers/collection.controller/removeProjectFromCollection.js b/server/controllers/collection.controller/removeProjectFromCollection.js
index 679d0cc0..f2df7bbe 100644
--- a/server/controllers/collection.controller/removeProjectFromCollection.js
+++ b/server/controllers/collection.controller/removeProjectFromCollection.js
@@ -23,7 +23,7 @@ export default function addProjectToCollection(req, res) {
       return null;
     }
 
-    const project = collection.items.find(p => p.project._id === projectId);
+    const project = collection.items.find(p => p.projectId === projectId);
 
     if (project != null) {
       project.remove();
diff --git a/server/models/collection.js b/server/models/collection.js
index 2753496b..bbd40ea4 100644
--- a/server/models/collection.js
+++ b/server/models/collection.js
@@ -15,6 +15,14 @@ collectedProjectSchema.virtual('id').get(function getId() {
   return this._id.toHexString();
 });
 
+collectedProjectSchema.virtual('projectId').get(function projectId() {
+  return this.populated('project');
+});
+
+collectedProjectSchema.virtual('isDeleted').get(function isDeleted() {
+  return this.project == null;
+});
+
 collectedProjectSchema.set('toJSON', {
   virtuals: true
 });