Skip to content

Commit

Permalink
fix: use correct access checks for showing interpretation reply, edit…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
jenniferarnesen authored Jul 7, 2023
1 parent 0fa8310 commit bf70361
Show file tree
Hide file tree
Showing 12 changed files with 383 additions and 63 deletions.
21 changes: 16 additions & 5 deletions i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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"

Expand Down
34 changes: 22 additions & 12 deletions src/components/Interpretations/InterpretationModal/Comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -11,9 +16,12 @@ const Comment = ({
currentUser,
interpretationId,
onThreadUpdated,
canComment,
}) => {
const [isUpdateMode, setIsUpdateMode] = useState(false)

const commentAccess = getCommentAccess(comment, canComment, currentUser)

return isUpdateMode ? (
<CommentUpdateForm
close={() => setIsUpdateMode(false)}
Expand All @@ -29,22 +37,23 @@ const Comment = ({
created={comment.created}
username={comment.createdBy.displayName}
>
<MessageStatsBar>
{comment.access.update && (
{commentAccess.edit && (
<MessageStatsBar>
<MessageIconButton
iconComponent={IconEdit16}
tooltipContent={i18n.t('Edit')}
onClick={() => setIsUpdateMode(true)}
/>
)}
{comment.access.delete && (
<CommentDeleteButton
commentId={comment.id}
interpretationId={interpretationId}
onComplete={() => onThreadUpdated(true)}
/>
)}
</MessageStatsBar>

{commentAccess.delete && (
<CommentDeleteButton
commentId={comment.id}
interpretationId={interpretationId}
onComplete={() => onThreadUpdated(true)}
/>
)}
</MessageStatsBar>
)}
</Message>
)
}
Expand All @@ -54,6 +63,7 @@ Comment.propTypes = {
currentUser: PropTypes.object.isRequired,
interpretationId: PropTypes.string.isRequired,
onThreadUpdated: PropTypes.func.isRequired,
canComment: PropTypes.bool,
}

export { Comment }
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -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 (
<MessageIconButton
tooltipContent={i18n.t('Delete')}
iconComponent={IconDelete16}
onClick={remove}
disabled={loading}
/>
<div className="delete-button-container">
<MessageIconButton
tooltipContent={i18n.t('Delete')}
iconComponent={IconDelete16}
onClick={onDelete}
disabled={loading}
/>
{deleteError && <span className="delete-error">{deleteError}</span>}
<style jsx>{`
.delete-button-container {
display: flex;
align-items: center;
gap: 4px;
}
.delete-error {
color: ${colors.red500};
font-size: 12px;
line-height: 12px;
}
`}</style>
</div>
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div className="message">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]]',
],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -26,6 +26,11 @@ const InterpretationThread = ({
}
}, [initialFocus])

const interpretationAccess = getInterpretationAccess(
interpretation,
currentUser
)

return (
<div className={cx('container', { fetching })}>
<div className={'scrollbox'}>
Expand All @@ -40,9 +45,14 @@ const InterpretationThread = ({
<Interpretation
currentUser={currentUser}
interpretation={interpretation}
onReplyIconClick={() => focusRef.current?.focus()}
onReplyIconClick={
interpretationAccess.comment
? () => focusRef.current?.focus()
: null
}
onUpdated={() => onThreadUpdated(true)}
onDeleted={onInterpretationDeleted}
isInThread={true}
/>
<div className={'comments'}>
{interpretation.comments.map((comment) => (
Expand All @@ -52,15 +62,18 @@ const InterpretationThread = ({
currentUser={currentUser}
interpretationId={interpretation.id}
onThreadUpdated={onThreadUpdated}
canComment={interpretationAccess.comment}
/>
))}
</div>
<CommentAddForm
currentUser={currentUser}
interpretationId={interpretation.id}
onSave={() => onThreadUpdated(true)}
focusRef={focusRef}
/>
{interpretationAccess.comment && (
<CommentAddForm
currentUser={currentUser}
interpretationId={interpretation.id}
onSave={() => onThreadUpdated(true)}
focusRef={focusRef}
/>
)}
</div>
</div>
<style jsx>{`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ const interpretationsQuery = {
resource: 'interpretations',
params: ({ type, id }) => ({
fields: [
'access',
'access[write,manage]',
'id',
'user[displayName]',
'createdBy[id,displayName]',
'created',
'text',
'comments[id]',
Expand Down
Loading

0 comments on commit bf70361

Please sign in to comment.