From 01a9f1098ad86be7ec3bbb8f28dae85edca8075b Mon Sep 17 00:00:00 2001
From: "Petter H. Juliussen" <petter.juliussen@origo.oslo.kommune.no>
Date: Fri, 17 Nov 2023 15:40:16 +0100
Subject: [PATCH] Fix permission check for key result value admin

---
 CHANGELOG.md                            |  2 +
 src/components/KeyResultValuesList.vue  | 13 ++++---
 src/components/ObjectiveDetailsCard.vue | 11 +-----
 src/components/panes/KeyResultPane.vue  | 12 ++----
 src/store/index.js                      | 49 ++++++++++++++++---------
 5 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c51323517..4556b3d33 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,8 @@ All notable changes to this project will be documented in this file. The format
   measurement detail pages.
 - Product result indicators and key figures are now correctly included as part
   of parent measurements when switching between items.
+- The edit button for key result values is now only visible to users with the
+  appropriate permissions.
 
 ### Security
 
diff --git a/src/components/KeyResultValuesList.vue b/src/components/KeyResultValuesList.vue
index 2299a5051..e66f5d7d9 100644
--- a/src/components/KeyResultValuesList.vue
+++ b/src/components/KeyResultValuesList.vue
@@ -22,11 +22,11 @@
             </div>
           </td>
           <td>
-            <p v-if="record.comment" :class="{ 'mr-size-40': hasEditRights }">
+            <p v-if="record.comment" :class="{ 'mr-size-40': canEdit }">
               {{ record.comment }}
             </p>
             <pkt-button
-              v-if="hasEditRights"
+              v-if="canEdit"
               v-tooltip="$t('tooltip.editProgress')"
               size="small"
               skin="tertiary"
@@ -42,7 +42,6 @@
 </template>
 
 <script>
-import { mapGetters } from 'vuex';
 import { dateTimeShort } from '@/util';
 import { formatValue } from '@/util/keyResultProgress';
 import { PktButton } from '@oslokommune/punkt-vue2';
@@ -59,10 +58,12 @@ export default {
       type: Array,
       required: true,
     },
-  },
 
-  computed: {
-    ...mapGetters(['hasEditRights']),
+    canEdit: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
 
   methods: {
diff --git a/src/components/ObjectiveDetailsCard.vue b/src/components/ObjectiveDetailsCard.vue
index a856d99f1..12873829a 100644
--- a/src/components/ObjectiveDetailsCard.vue
+++ b/src/components/ObjectiveDetailsCard.vue
@@ -60,7 +60,7 @@
 </template>
 
 <script>
-import { mapState } from 'vuex';
+import { mapGetters, mapState } from 'vuex';
 import { startOfDay } from 'date-fns';
 import { PktButton } from '@oslokommune/punkt-vue2';
 import { periodDates, uniqueBy } from '@/util';
@@ -98,14 +98,7 @@ export default {
 
   computed: {
     ...mapState(['activeItem', 'user']),
-
-    isAdminOfCurrentOrganization() {
-      return this.user.admin?.includes(
-        this.activeItem.organization
-          ? this.activeItem.organization.id
-          : this.activeItem.id
-      );
-    },
+    ...mapGetters(['isAdminOfCurrentOrganization']),
 
     isMemberOfObjectiveParent() {
       return this.objective.parent.team?.includes(`users/${this.user.id}`);
diff --git a/src/components/panes/KeyResultPane.vue b/src/components/panes/KeyResultPane.vue
index e6086721c..b973038ac 100644
--- a/src/components/panes/KeyResultPane.vue
+++ b/src/components/panes/KeyResultPane.vue
@@ -69,6 +69,7 @@
       <key-result-values-list
         v-else-if="progress.length"
         :progress="progress"
+        :can-edit="canEdit"
         class="key-result-pane__table"
         @edit-value="openValueModal"
         @delete-value="(record) => deleteHistoryRecord(record.id)"
@@ -113,7 +114,7 @@
 </template>
 
 <script>
-import { mapActions, mapState } from 'vuex';
+import { mapActions, mapGetters, mapState } from 'vuex';
 import { format } from 'd3-format';
 import { max, min } from 'd3-array';
 import { db } from '@/config/firebaseConfig';
@@ -162,14 +163,7 @@ export default {
   computed: {
     ...mapState(['activeItem', 'activePeriod', 'user']),
     ...mapState('okrs', ['activeObjective', 'activeKeyResult']),
-
-    isAdminOfCurrentOrganization() {
-      return this.user.admin?.includes(
-        this.activeItem.organization
-          ? this.activeItem.organization.id
-          : this.activeItem.id
-      );
-    },
+    ...mapGetters(['isAdminOfCurrentOrganization']),
 
     isMemberOfKeyResultParent() {
       return this.activeKeyResult.parent.team?.includes(`users/${this.user.id}`);
diff --git a/src/store/index.js b/src/store/index.js
index 74bec3372..5890b526f 100644
--- a/src/store/index.js
+++ b/src/store/index.js
@@ -49,24 +49,43 @@ export const storeGetters = {
     return activeItem.team.map(({ id }) => id).includes(user.id);
   },
 
-  hasEditRights: (state) => {
-    // Returns `true` if user has `admin: true` or if user is member of `activeItem`
+  /**
+   * Returns `true` if the current user is an admin of the parent organization
+   * of `activeItem`.
+   */
+  isAdminOfCurrentOrganization: (state) => {
     const { user, activeItem } = state;
-    const { organization } = activeItem;
 
-    const isAdminOfOrganization = organization
-      ? user.admin && user.admin.includes(organization.id)
-      : user.admin && user.admin.includes(activeItem.id);
+    if (!user || !activeItem) {
+      return false;
+    }
 
-    if (user && user.superAdmin) {
-      return true;
+    return (
+      user.admin?.includes(
+        activeItem.organization ? activeItem.organization.id : activeItem.id
+      ) || false
+    );
+  },
+
+  /**
+   * Returns `true` if the current user has admin rights or is member of
+   * `activeItem`.
+   */
+  hasEditRights: (state, getters) => {
+    const { user, activeItem } = state;
+
+    if (!user || !activeItem) {
+      return false;
     }
-    if (isAdminOfOrganization) {
+
+    if (user.superAdmin || getters.isAdminOfCurrentOrganization) {
       return true;
     }
-    if (!user || !activeItem || !activeItem.team) {
+
+    if (!activeItem.team) {
       return false;
     }
+
     return activeItem.team.map(({ id }) => id).includes(user.id);
   },
 
@@ -74,20 +93,14 @@ export const storeGetters = {
    * Return `true` if the current user is an admin of the active item or a
    * member of its parent item.
    */
-  hasParentEditRights: (state) => {
+  hasParentEditRights: (state, getters) => {
     const { user, activeItem } = state;
 
     if (!user || !activeItem) {
       return false;
     }
 
-    const { organization } = activeItem;
-
-    const isAdminOfOrganization = organization
-      ? user.admin && user.admin.includes(organization.id)
-      : user.admin && user.admin.includes(activeItem.id);
-
-    if (user.superAdmin || isAdminOfOrganization) {
+    if (user.superAdmin || getters.isAdminOfCurrentOrganization) {
       return true;
     }