From bf703619cb773354e5665543fda36513f7edc6ce Mon Sep 17 00:00:00 2001 From: Jen Jones Arnesen Date: Fri, 7 Jul 2023 09:59:27 +0200 Subject: [PATCH] fix: use correct access checks for showing interpretation reply, edit and delete buttons (#1531) Implements DHIS2-15510 These are the access checks that are done in the backend: * edit or delete an interpretation: isCreatorOrSuperuser of interpretation * add interpretation comment: interpretation.access.write * update comment: isCreatorOrSuperuser of comment * delete comment: isCreatorOrSuperuser of comment && interpretation.access.write Other changes: * use createdBy instead of user. `user` is only kept for backwards compatibility * don't fetch comment.access because the values can be wrong, and it's not needed anyway * add error handling to the CommentDeleteButton * error message for edit-comment-failure is made generic. Backend may report a misleading message such as "interpretation does not exist" when it does in fact exist, and user can see it, but user doesn't have access to edit. --- i18n/en.pot | 21 +- .../InterpretationModal/Comment.js | 34 +-- .../CommentDeleteButton.js | 44 +++- .../InterpretationModal/CommentUpdateForm.js | 4 +- .../InterpretationModal.js | 6 +- .../InterpretationThread.js | 29 ++- .../InterpretationsUnit.js | 4 +- .../common/Interpretation/Interpretation.js | 71 +++++-- .../common/Message/MessageIconButton.js | 8 +- .../__tests__/getInterpretationAccess.spec.js | 199 ++++++++++++++++++ .../common/getInterpretationAccess.js | 25 +++ .../Interpretations/common/index.js | 1 + 12 files changed, 383 insertions(+), 63 deletions(-) create mode 100644 src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js create mode 100644 src/components/Interpretations/common/getInterpretationAccess.js diff --git a/i18n/en.pot b/i18n/en.pot index 2ceaf2ec9..52645c02d 100644 --- a/i18n/en.pot +++ b/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: 2023-05-24T12:55:52.925Z\n" -"PO-Revision-Date: 2023-05-24T12:55:52.925Z\n" +"POT-Creation-Date: 2023-07-06T08:30:33.216Z\n" +"PO-Revision-Date: 2023-07-06T08:30:33.216Z\n" msgid "view only" msgstr "view only" @@ -343,6 +343,9 @@ msgstr "Write a reply" msgid "Post reply" msgstr "Post reply" +msgid "Delete failed" +msgstr "Delete failed" + msgid "Could not update comment" msgstr "Could not update comment" @@ -377,15 +380,23 @@ msgstr "Post interpretation" msgid "Interpretations" msgstr "Interpretations" +msgid "Reply" +msgstr "Reply" + +msgid "{{count}} replies" +msgid_plural "{{count}} replies" +msgstr[0] "{{count}} reply" +msgstr[1] "{{count}} replies" + +msgid "View replies" +msgstr "View replies" + msgid "Unlike" msgstr "Unlike" msgid "Like" msgstr "Like" -msgid "Reply" -msgstr "Reply" - msgid "Share" msgstr "Share" diff --git a/src/components/Interpretations/InterpretationModal/Comment.js b/src/components/Interpretations/InterpretationModal/Comment.js index 2e7e07637..e1fef4fe1 100644 --- a/src/components/Interpretations/InterpretationModal/Comment.js +++ b/src/components/Interpretations/InterpretationModal/Comment.js @@ -2,7 +2,12 @@ import i18n from '@dhis2/d2-i18n' import { IconEdit16 } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useState } from 'react' -import { Message, MessageIconButton, MessageStatsBar } from '../common/index.js' +import { + Message, + MessageIconButton, + MessageStatsBar, + getCommentAccess, +} from '../common/index.js' import { CommentDeleteButton } from './CommentDeleteButton.js' import { CommentUpdateForm } from './CommentUpdateForm.js' @@ -11,9 +16,12 @@ const Comment = ({ currentUser, interpretationId, onThreadUpdated, + canComment, }) => { const [isUpdateMode, setIsUpdateMode] = useState(false) + const commentAccess = getCommentAccess(comment, canComment, currentUser) + return isUpdateMode ? ( setIsUpdateMode(false)} @@ -29,22 +37,23 @@ const Comment = ({ created={comment.created} username={comment.createdBy.displayName} > - - {comment.access.update && ( + {commentAccess.edit && ( + setIsUpdateMode(true)} /> - )} - {comment.access.delete && ( - onThreadUpdated(true)} - /> - )} - + + {commentAccess.delete && ( + onThreadUpdated(true)} + /> + )} + + )} ) } @@ -54,6 +63,7 @@ Comment.propTypes = { currentUser: PropTypes.object.isRequired, interpretationId: PropTypes.string.isRequired, onThreadUpdated: PropTypes.func.isRequired, + canComment: PropTypes.bool, } export { Comment } diff --git a/src/components/Interpretations/InterpretationModal/CommentDeleteButton.js b/src/components/Interpretations/InterpretationModal/CommentDeleteButton.js index 228896126..363733235 100644 --- a/src/components/Interpretations/InterpretationModal/CommentDeleteButton.js +++ b/src/components/Interpretations/InterpretationModal/CommentDeleteButton.js @@ -1,8 +1,8 @@ import { useDataMutation } from '@dhis2/app-runtime' import i18n from '@dhis2/d2-i18n' -import { IconDelete16 } from '@dhis2/ui' +import { IconDelete16, colors } from '@dhis2/ui' import PropTypes from 'prop-types' -import React from 'react' +import React, { useState } from 'react' import { MessageIconButton } from '../common/index.js' const mutation = { @@ -13,17 +13,43 @@ const mutation = { } const CommentDeleteButton = ({ commentId, interpretationId, onComplete }) => { + const [deleteError, setDeleteError] = useState(null) const [remove, { loading }] = useDataMutation(mutation, { - onComplete, + onComplete: () => { + setDeleteError(null) + onComplete() + }, + onError: () => setDeleteError(i18n.t('Delete failed')), variables: { commentId, interpretationId }, }) + + const onDelete = () => { + setDeleteError(null) + remove() + } + return ( - +
+ + {deleteError && {deleteError}} + +
) } diff --git a/src/components/Interpretations/InterpretationModal/CommentUpdateForm.js b/src/components/Interpretations/InterpretationModal/CommentUpdateForm.js index 1b1dec89c..ea9d3812e 100644 --- a/src/components/Interpretations/InterpretationModal/CommentUpdateForm.js +++ b/src/components/Interpretations/InterpretationModal/CommentUpdateForm.js @@ -33,9 +33,7 @@ export const CommentUpdateForm = ({ }, } ) - const errorText = error - ? error.message || i18n.t('Could not update comment') - : '' + const errorText = error ? i18n.t('Could not update comment') : '' return (
diff --git a/src/components/Interpretations/InterpretationModal/InterpretationModal.js b/src/components/Interpretations/InterpretationModal/InterpretationModal.js index f2d693838..4f02eaefa 100644 --- a/src/components/Interpretations/InterpretationModal/InterpretationModal.js +++ b/src/components/Interpretations/InterpretationModal/InterpretationModal.js @@ -49,14 +49,14 @@ const query = { id: ({ id }) => id, params: { fields: [ - 'access', + 'access[write,manage]', 'id', 'text', 'created', - 'user[id,displayName]', + 'createdBy[id,displayName]', 'likes', 'likedBy', - 'comments[access,id,text,created,createdBy[id,displayName]]', + 'comments[id,text,created,createdBy[id,displayName]]', ], }, }, diff --git a/src/components/Interpretations/InterpretationModal/InterpretationThread.js b/src/components/Interpretations/InterpretationModal/InterpretationThread.js index cf352317b..03540290d 100644 --- a/src/components/Interpretations/InterpretationModal/InterpretationThread.js +++ b/src/components/Interpretations/InterpretationModal/InterpretationThread.js @@ -3,7 +3,7 @@ import cx from 'classnames' import moment from 'moment' import PropTypes from 'prop-types' import React, { useRef, useEffect } from 'react' -import { Interpretation } from '../common/index.js' +import { Interpretation, getInterpretationAccess } from '../common/index.js' import { Comment } from './Comment.js' import { CommentAddForm } from './CommentAddForm.js' @@ -26,6 +26,11 @@ const InterpretationThread = ({ } }, [initialFocus]) + const interpretationAccess = getInterpretationAccess( + interpretation, + currentUser + ) + return (
@@ -40,9 +45,14 @@ const InterpretationThread = ({ focusRef.current?.focus()} + onReplyIconClick={ + interpretationAccess.comment + ? () => focusRef.current?.focus() + : null + } onUpdated={() => onThreadUpdated(true)} onDeleted={onInterpretationDeleted} + isInThread={true} />
{interpretation.comments.map((comment) => ( @@ -52,15 +62,18 @@ const InterpretationThread = ({ currentUser={currentUser} interpretationId={interpretation.id} onThreadUpdated={onThreadUpdated} + canComment={interpretationAccess.comment} /> ))}
- onThreadUpdated(true)} - focusRef={focusRef} - /> + {interpretationAccess.comment && ( + onThreadUpdated(true)} + focusRef={focusRef} + /> + )}