Merge pull request #1480 from andrewn/bugfix/deleted-collection-sketch

Allow deleted sketches in collections to be removed (fixes #1465)

client/modules/User/components/Collection.jsx

@@ -71,41 +71,50 @@ ShareURL.propTypes = {
   value: PropTypes.string.isRequired,
 };
 
-class CollectionItemRowBase extends React.Component {
+const CollectionItemRowBase = ({
-  handleSketchRemove = () => {
+  collection, item, isOwner, removeFromCollection
-    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 projectIsDeleted = item.isDeleted;
-    }
-  }
 
-  render() {
+  const handleSketchRemove = () => {
-    const { item } = this.props;
+    const name = projectIsDeleted ? 'deleted sketch' : item.project.name;
-    const sketchOwnerUsername = item.project.user.username;
+
-    const sketchUrl = `/${item.project.user.username}/sketches/${item.project.id}`;
+    if (window.confirm(`Are you sure you want to remove "${name}" from this collection?`)) {
+      removeFromCollection(collection.id, item.projectId);
+    }
+  };
+
+  const name = projectIsDeleted ? <span>Sketch was deleted</span> : (
+    <Link to={`/${item.project.user.username}/sketches/${item.projectId}`}>
+      {item.project.name}
+    </Link>
+  );
+
+  const sketchOwnerUsername = projectIsDeleted ? null : item.project.user.username;
 
   return (
     <tr
-        className="sketches-table__row"
+      className={`sketches-table__row ${projectIsDeleted ? 'is-deleted' : ''}`}
     >
       <th scope="row">
-          <Link to={sketchUrl}>
+        {name}
-            {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 ">
+        {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>);
-  }
+};
-}
+
 
 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>

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() {

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');

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();

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
 });