fix: handle both Set and array for currentUser.authorities when check…

…ing interpretations access (DHIS2-15964)
dhis2 · Oct 17, 2023 · d3d45ab · d3d45ab
1 parent 014d43e
commit d3d45ab
Show file tree

Hide file tree

Showing 7 changed files with 311 additions and 95 deletions.
diff --git a/src/components/Interpretations/InterpretationModal/InterpretationModal.js b/src/components/Interpretations/InterpretationModal/InterpretationModal.js
@@ -24,7 +24,7 @@ const modalCSS = css.resolve`
         max-width: calc(100vw - 128px) !important;
         max-height: calc(100vh - 128px) !important;
         width: auto !important;
-        height: calc(100vw - 128px) !important;
+        height: calc(100vh - 128px) !important;
         overflow-y: hidden;
     }
     aside.hidden {

diff --git a/src/components/Interpretations/InterpretationModal/InterpretationThread.js b/src/components/Interpretations/InterpretationModal/InterpretationThread.js
@@ -35,57 +35,47 @@ const InterpretationThread = ({
 
     return (
         <div className={cx('container', { fetching })}>
-            <div className={'scrollbox'}>
-                <div className={'title'}>
-                    <IconClock16 color={colors.grey700} />
-                    {moment(fromServerDate(interpretation.created)).format(
-                        'LLL'
-                    )}
-                </div>
-                {DownloadMenu && (
-                    <DownloadMenu relativePeriodDate={interpretation.created} />
-                )}
-                <div className={'thread'}>
-                    <Interpretation
-                        currentUser={currentUser}
-                        interpretation={interpretation}
-                        onReplyIconClick={
-                            interpretationAccess.comment
-                                ? () => focusRef.current?.focus()
-                                : null
-                        }
-                        onUpdated={() => onThreadUpdated(true)}
-                        onDeleted={onInterpretationDeleted}
-                        isInThread={true}
-                    />
-                    <div className={'comments'}>
-                        {interpretation.comments.map((comment) => (
-                            <Comment
-                                key={comment.id}
-                                comment={comment}
-                                currentUser={currentUser}
-                                interpretationId={interpretation.id}
-                                onThreadUpdated={onThreadUpdated}
-                                canComment={interpretationAccess.comment}
-                            />
-                        ))}
-                    </div>
-                    {interpretationAccess.comment && (
-                        <CommentAddForm
+            <div className={'title'}>
+                <IconClock16 color={colors.grey700} />
+                {moment(fromServerDate(interpretation.created)).format('LLL')}
+            </div>
+            {DownloadMenu && (
+                <DownloadMenu relativePeriodDate={interpretation.created} />
+            )}
+            <div className={'thread'}>
+                <Interpretation
+                    currentUser={currentUser}
+                    interpretation={interpretation}
+                    onReplyIconClick={
+                        interpretationAccess.comment
+                            ? () => focusRef.current?.focus()
+                            : null
+                    }
+                    onUpdated={() => onThreadUpdated(true)}
+                    onDeleted={onInterpretationDeleted}
+                    isInThread={true}
+                />
+                <div className={'comments'}>
+                    {interpretation.comments.map((comment) => (
+                        <Comment
+                            key={comment.id}
+                            comment={comment}
                             currentUser={currentUser}
                             interpretationId={interpretation.id}
-                            onSave={() => onThreadUpdated(true)}
-                            focusRef={focusRef}
+                            onThreadUpdated={onThreadUpdated}
+                            canComment={interpretationAccess.comment}
                         />
-                    )}
+                    ))}
                 </div>
             </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}
+                />
+            )}
             <style jsx>{`
                 .thread {
                     margin-top: var(--spacers-dp16);
@@ -95,7 +85,7 @@ const InterpretationThread = ({
 
                 .container {
                     position: relative;
-                    overflow: hidden;
+                    overflow: auto;
                     max-height: calc(100vh - 285px);
                     display: flex;
                     flex-direction: column;

diff --git a/src/components/Interpretations/common/Message/Message.js b/src/components/Interpretations/common/Message/Message.js
@@ -48,6 +48,7 @@ const Message = ({ children, text, created, username }) => {
                     font-size: 14px;
                     line-height: 19px;
                     color: ${colors.grey900};
+                    word-break: break-word;
                 }
 
                 .content :global(p:first-child) {

diff --git a/src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js b/src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js
@@ -3,21 +3,36 @@ import {
     getCommentAccess,
 } from '../getInterpretationAccess.js'
 
-const superuser = {
+const superuserD2CurrentUser = {
     id: 'iamsuper',
     authorities: new Set(['ALL']),
 }
 
-const userJoe = {
+const userJoeD2CurrentUser = {
     id: 'johndoe',
     authorities: new Set(['Some']),
 }
 
-const userJane = {
+const userJaneD2CurrentUser = {
     id: 'jane',
     authorities: new Set(['Some']),
 }
 
+const superuser = {
+    id: 'iamsuper',
+    authorities: ['ALL'],
+}
+
+const userJoe = {
+    id: 'johndoe',
+    authorities: ['Some'],
+}
+
+const userJane = {
+    id: 'jane',
+    authorities: ['Some'],
+}
+
 describe('interpretation and comment access', () => {
     describe('getInterpretationAccess', () => {
         it('returns true for all accesses for superuser', () => {
@@ -113,6 +128,228 @@ describe('interpretation and comment access', () => {
                 delete: false,
             })
         })
+
+        it('throws an error for all accesses when no currentUser provided', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: false,
+                },
+                createdBy: userJane,
+            }
+
+            expect(() => getInterpretationAccess(interpretation)).toThrow(
+                '"hasAuthority" requires "authorities" to be an array or set of authorities (strings)'
+            )
+        })
+
+        it('throws an error when currentUser is missing authorities', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: false,
+                },
+                createdBy: userJane,
+            }
+
+            expect(() =>
+                getInterpretationAccess(interpretation, {
+                    id: 'usernoauthorties',
+                })
+            ).toThrow(
+                '"hasAuthority" requires "authorities" to be an array or set of authorities (strings)'
+            )
+        })
+    })
+
+    describe('getInterpretationAccess using D2.currentUser', () => {
+        it('returns true for all accesses for superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                    manage: true,
+                },
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, superuserD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: true,
+                edit: true,
+                delete: true,
+            })
+        })
+        it('returns true for all accesses for creator', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                    manage: true,
+                },
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: true,
+                edit: true,
+                delete: true,
+            })
+        })
+
+        it('returns false for edit/delete if user is not creator/superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                    manage: true,
+                },
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: true,
+                edit: false,
+                delete: false,
+            })
+        })
+
+        it('returns false for comment/edit/delete if user is not creator/superuser and no write access', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: true,
+                },
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: false,
+                edit: false,
+                delete: false,
+            })
+        })
+
+        it('returns false for share/comment/edit/delete if user is not creator/superuser and no write or manage access', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: false,
+                },
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: false,
+                comment: false,
+                edit: false,
+                delete: false,
+            })
+        })
+    })
+
+    describe('getCommentAccess using D2.currentUser', () => {
+        it('returns true for all accesses for superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                },
+            }
+
+            const comment = {
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    superuserD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: true,
+                delete: true,
+            })
+        })
+
+        it('returns true for all accesses for creator when interpretation has write access', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                },
+            }
+
+            const comment = {
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    userJoeD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: true,
+                delete: true,
+            })
+        })
+
+        it('returns true for edit and false for delete for creator when interpretation does not have write access', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                },
+            }
+
+            const comment = {
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    userJoeD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: true,
+                delete: false,
+            })
+        })
+
+        it('returns false for edit/delete for user who is not creator or superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                },
+            }
+
+            const comment = {
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    userJoeD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: false,
+                delete: false,
+            })
+        })
     })
 
     describe('getCommentAccess', () => {