fix: fix interpretation comment delete (#543)

Before it was always deleting the last added comment, regardless of which delete button was clicked. Fixed by moving the DeleteDialog in the comments list component, we only need one dialog anyway. The comment to delete is set in the state via callback when the Delete button on a comment is used. When the dialog is confirmed the comment to delete is retrieved from the state.
dhis2 · Jan 29, 2020 · 9e38ae4 · 9e38ae4
1 parent 5ee6baa
commit 9e38ae4
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 58 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@dhis2/d2-ui",
-  "version": "6.5.0",
+  "version": "6.5.1",
   "license": "BSD-3-Clause",
   "private": true,
   "scripts": {

diff --git a/packages/interpretations/i18n/en.pot b/packages/interpretations/i18n/en.pot
@@ -5,8 +5,8 @@ msgstr ""
 "Content-Type: text/plain; charset=utf-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=2; plural=(n != 1)\n"
-"POT-Creation-Date: 2020-01-08T11:20:59.368Z\n"
-"PO-Revision-Date: 2020-01-08T11:20:59.368Z\n"
+"POT-Creation-Date: 2020-01-28T10:56:12.426Z\n"
+"PO-Revision-Date: 2020-01-28T10:56:12.426Z\n"
 
 msgid "Pivot Tables"
 msgstr ""
@@ -65,12 +65,6 @@ msgstr ""
 msgid "Interpretations"
 msgstr ""
 
-msgid "Delete comment"
-msgstr ""
-
-msgid "Are you sure you want to delete this comment?"
-msgstr ""
-
 msgid "Save reply"
 msgstr ""
 
@@ -149,6 +143,12 @@ msgstr ""
 msgid "View more replies"
 msgstr ""
 
+msgid "Delete comment"
+msgstr ""
+
+msgid "Are you sure you want to delete this comment?"
+msgstr ""
+
 msgid "Hide"
 msgstr ""
 

diff --git a/packages/interpretations/src/components/Comment/Comment.js b/packages/interpretations/src/components/Comment/Comment.js
@@ -1,13 +1,11 @@
 import React, { Fragment } from 'react';
 import PropTypes from 'prop-types';
 import { withStyles } from '@material-ui/core/styles';
-import i18n from '@dhis2/d2-i18n'
 import ActionButton from '../Buttons/ActionButton';
 import WithAvatar from '../Avatar/WithAvatar';
 import CardHeader from '../Cards/CardHeader';
 import CardText from '../Cards/CardText';
 import CardInfo from '../Cards/CardInfo';
-import DeleteDialog from '../DeleteDialog/DeleteDialog';
 import { formatDate } from '../../dateformats/dateformatter';
 import styles from './styles/Comment.style';
 
@@ -20,9 +18,6 @@ export const Comment = ({
     onEdit, 
     onReply,
     onDelete,
-    dialogIsOpen,
-    onDeleteConfirm,
-    onDeleteCancel, 
 }) => (
     <Fragment>
         <WithAvatar className={classes.comment} key={comment.id} firstName={comment.user.firstName} surname={comment.user.surname}>
@@ -41,7 +36,7 @@ export const Comment = ({
                     />
                     <ActionButton
                         iconType={'delete'} 
-                        onClick={onDelete} 
+                        onClick={() => onDelete(comment)}
                     />
                 </div>
              ) : (
@@ -53,14 +48,6 @@ export const Comment = ({
                 )
              )}
         </WithAvatar>
-        {dialogIsOpen && (
-            <DeleteDialog
-                title={i18n.t('Delete comment')}
-                text={i18n.t('Are you sure you want to delete this comment?')}
-                onDelete={() => onDeleteConfirm(comment)}
-                onCancel={onDeleteCancel}
-            />
-        )}
     </Fragment>
 );
 
@@ -72,9 +59,6 @@ Comment.propTypes = {
     onEdit: PropTypes.func.isRequired,
     onReply: PropTypes.func.isRequired,
     onDelete: PropTypes.func.isRequired,
-    dialogIsOpen: PropTypes.bool.isRequired,
-    onDeleteConfirm: PropTypes.func.isRequired,
-    onDeleteCancel: PropTypes.func.isRequired,
 };
 
 export default withStyles(styles)(Comment);
diff --git a/packages/interpretations/src/components/Comment/__tests__/Comment.spec.js b/packages/interpretations/src/components/Comment/__tests__/Comment.spec.js
@@ -2,7 +2,6 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import { Comment } from '../Comment';
 import ActionButton from '../../Buttons/ActionButton';
-import DeleteDialog from '../../DeleteDialog/DeleteDialog';
 
 let shallowComment;
 
@@ -21,9 +20,6 @@ const baseProps = {
     onEdit: jest.fn(),
     onReply: jest.fn(),
     onDelete: jest.fn(),
-    dialogIsOpen: false,
-    onDeleteConfirm: jest.fn(),
-    onDeleteCancel: jest.fn(),
 };
 
 const comment = (partialProps = {}) => {
@@ -112,20 +108,4 @@ describe('components: Comment -> Comment component ', () => {
         });
     });
 
-    describe('with prop dialogIsOpen as false', () => {
-        it('should not show a DeleteDialog', () => {
-            expect(comment().find(DeleteDialog)).not.toExist();
-        });
-    });
-
-    describe('with prop dialogIsOpen as true', () => {
-        beforeEach(() => {
-            comment({ dialogIsOpen: true })
-        });
-
-        it('should show a DeleteDialog', () => {
-            expect(comment().find(DeleteDialog)).toExist();  
-        });
-    });
-
 });
diff --git a/packages/interpretations/src/components/Lists/CommentsList.js b/packages/interpretations/src/components/Lists/CommentsList.js
@@ -9,6 +9,7 @@ import Link from '../Link/Link';
 import CommentModel from '../../models/comment';
 import { userCanManage } from '../../authorization/auth';
 import styles from './styles/CommentsList.style';
+import DeleteDialog from '../DeleteDialog/DeleteDialog';
 
 const commentsToShowOnInit = 5;
 
@@ -37,6 +38,7 @@ export class CommentsList extends React.Component {
             commentToEdit: null,
             newComment: props.newComment,
             deleteDialogIsOpen: false,
+            commentToDelete: null,
         };
     };
 
@@ -58,8 +60,8 @@ export class CommentsList extends React.Component {
         comment.save(this.context.d2).then(() => this.props.onChange(this.props.interpretation));
     };
 
-    onDeleteComment(comment) {
-        comment.delete(this.context.d2).then(() => this.props.onChange(this.props.interpretation));
+    onDeleteComment() {
+        this.state.commentToDelete.delete(this.context.d2).then(() => this.props.onChange(this.props.interpretation));
         this.onCloseDeleteDialog();
     };
 
@@ -80,11 +82,11 @@ export class CommentsList extends React.Component {
         this.props.onCancel();
     };
 
-    onOpenDeleteDialog = () =>
-        this.setState({ deleteDialogIsOpen: true });
+    onOpenDeleteDialog = comment =>
+        this.setState({ deleteDialogIsOpen: true, commentToDelete: comment });
 
     onCloseDeleteDialog = () =>
-        this.setState({ deleteDialogIsOpen: false });
+        this.setState({ deleteDialogIsOpen: false, commentToDelete: null });
 
     onUpdate(comment) {
         this.onSave(comment);
@@ -98,7 +100,7 @@ export class CommentsList extends React.Component {
 
     getComments = () => {
         const sortedComments = orderBy(["created"], ["asc"], this.props.interpretation.comments);
-        
+
         return !this.state.listIsExpanded 
             ? sortedComments.slice(-commentsToShowOnInit)
             : sortedComments;
@@ -131,9 +133,6 @@ export class CommentsList extends React.Component {
                     onEdit={this.onEdit}
                     onReply={this.onReply}
                     onDelete={this.onOpenDeleteDialog}
-                    dialogIsOpen={this.state.deleteDialogIsOpen}
-                    onDeleteConfirm={this.onDeleteComment}
-                    onDeleteCancel={this.onCloseDeleteDialog}
                 />
             )
         );
@@ -154,9 +153,17 @@ export class CommentsList extends React.Component {
 
         return (
             <div className={this.props.classes.commentSection}>
-                {ViewMoreReplies}
-                {Comments}
-                {InputField}
+            {ViewMoreReplies}
+            {Comments}
+            {InputField}
+            {this.state.deleteDialogIsOpen && (
+                <DeleteDialog
+                    title={i18n.t('Delete comment')}
+                    text={i18n.t('Are you sure you want to delete this comment?')}
+                    onDelete={this.onDeleteComment}
+                    onCancel={this.onCloseDeleteDialog}
+                />
+            )}
             </div>
         );
     }

diff --git a/packages/interpretations/src/components/Lists/__tests__/CommentsList.spec.js b/packages/interpretations/src/components/Lists/__tests__/CommentsList.spec.js
@@ -4,6 +4,7 @@ import merge from 'lodash/merge';
 import { CommentsList } from '../CommentsList';
 import Comment from '../../Comment/Comment';
 import NewCommentField from '../../Comment/NewCommentField';
+import DeleteDialog from '../../DeleteDialog/DeleteDialog';
 import InterpretationModel from '../../../models/interpretation';
 import { getStubContext } from '../../../../config/test-context';
 
@@ -55,6 +56,7 @@ let commentComponents;
 let currentUser;
 let commentComponent;
 let commentToEdit;
+let commentToDelete;
 
 describe('Interpretations: Lists -> CommentsList component', () => {
     beforeEach(() => {
@@ -63,6 +65,7 @@ describe('Interpretations: Lists -> CommentsList component', () => {
             displayName: "John Traore",
         };
         commentList = renderComponent({}, {d2: {currentUser}});
+        commentList.instance().onDeleteComment = jest.fn();
         commentComponents = commentList.find(Comment);
     });
 
@@ -86,6 +89,10 @@ describe('Interpretations: Lists -> CommentsList component', () => {
                 }
             });
         });
+
+        it('should not show the DeleteDialog', () => {
+            expect(commentList.find(DeleteDialog)).not.toExist();
+        });
     });
 
     describe('click on edit link of editable comment', () => {
@@ -103,4 +110,25 @@ describe('Interpretations: Lists -> CommentsList component', () => {
         });
 
     });
+
+    describe('click on delete comment', () => {
+        beforeEach(() => {
+            commentComponent = commentComponents.at(1);
+            commentToDelete = commentComponent.props().comment;
+            commentComponent.props().onDelete(commentToDelete);
+            commentList.update();
+        });
+
+        it('should show the DeleteDialog', () => {
+            expect(commentList.find(DeleteDialog)).toExist();
+        });
+
+        it('should call onDeleteComment when dialog is confirmed', () => {
+            const deleteDialog = commentList.find(DeleteDialog);
+
+            deleteDialog.props().onDelete();
+
+            expect(commentList.instance().onDeleteComment).toHaveBeenCalled();
+        });
+    });
 });