From 298f573113ba393794b7b30ea65a3c5d09da52f4 Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Tue, 31 May 2022 15:41:47 -0400 Subject: [PATCH] fix: use `originalUserIsStaff` to gate bulk email tool over `isStaff` [MICROBA-1822][CR-4822] * use `originalUserIsStaff` to gate bulk email tool access over `isStaff` Additional Context Access to this tool was originally gated by looking at the `isStaff` value from the CourseMetadata data returned to us from the LMS. The user masquerading feature seems to have some interesting interactions with the Instructor Dashboard and we may be denying legitimate staff/admin/instructors access to the tool. Instead of using the value of `isStaff` we will now use `originalUserIsStaff` to determine if the user accessing the tool should be allowed access (and this follows how the Learning MFE gates content). --- src/components/bulk-email-tool/BulkEmailTool.jsx | 2 +- .../bulk-email-tool/test/BulkEmailTool.test.jsx | 9 +++++---- src/components/page-container/PageContainer.jsx | 6 +++--- .../data/__factories__/courseMetadata.factory.js | 2 +- src/components/page-container/data/api.js | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/components/bulk-email-tool/BulkEmailTool.jsx b/src/components/bulk-email-tool/BulkEmailTool.jsx index cf6eda3..ba49550 100644 --- a/src/components/bulk-email-tool/BulkEmailTool.jsx +++ b/src/components/bulk-email-tool/BulkEmailTool.jsx @@ -19,7 +19,7 @@ export default function BulkEmailTool() { return ( - {(courseMetadata) => (courseMetadata.isStaff ? ( + {(courseMetadata) => (courseMetadata.originalUserIsStaff ? (
diff --git a/src/components/bulk-email-tool/test/BulkEmailTool.test.jsx b/src/components/bulk-email-tool/test/BulkEmailTool.test.jsx index aaa6f40..9ea6c9b 100644 --- a/src/components/bulk-email-tool/test/BulkEmailTool.test.jsx +++ b/src/components/bulk-email-tool/test/BulkEmailTool.test.jsx @@ -3,6 +3,7 @@ */ import React from 'react'; import { Factory } from 'rosie'; +import { camelCaseObject } from '@edx/frontend-platform'; import { render, screen, cleanup, initializeMockApp, } from '../../../setupTest'; @@ -36,15 +37,15 @@ describe('BulkEmailTool', () => { */ function buildCourseMetadata(cohortData, courseData) { const { - org, number, title, tabs, is_staff: isStaff, - } = courseData; + org, number, title, tabs, originalUserIsStaff, + } = camelCaseObject(courseData); const { cohorts } = cohortData; return { org, number, title, - isStaff, + originalUserIsStaff, tabs: [...tabs], cohorts: cohorts.map(({ name }) => name), }; @@ -78,7 +79,7 @@ describe('BulkEmailTool', () => { test('BulkEmailTool renders error page on no staff user', async () => { const cohorts = { cohorts: [] }; - const courseInfo = Factory.build('courseMetadata', { is_staff: false }); + const courseInfo = Factory.build('courseMetadata', { original_user_is_staff: false }); const courseMetadata = buildCourseMetadata(cohorts, courseInfo); renderBulkEmailTool(courseMetadata); // verify error page is displayed for user without staff permissions diff --git a/src/components/page-container/PageContainer.jsx b/src/components/page-container/PageContainer.jsx index 220bccc..bb44656 100644 --- a/src/components/page-container/PageContainer.jsx +++ b/src/components/page-container/PageContainer.jsx @@ -31,7 +31,7 @@ export default function PageContainer(props) { org: '', number: '', title: '', - isStaff: false, + originalUserIsStaff: false, tabs: [], cohorts: [], }); @@ -39,7 +39,7 @@ export default function PageContainer(props) { } const { - org, number, title, tabs, is_staff: isStaff, + org, number, title, tabs, originalUserIsStaff, } = metadataResponse; const { cohorts } = cohortsResponse; @@ -47,7 +47,7 @@ export default function PageContainer(props) { org, number, title, - isStaff, + originalUserIsStaff, tabs: [...tabs], cohorts: cohorts.map(({ name }) => name), }); diff --git a/src/components/page-container/data/__factories__/courseMetadata.factory.js b/src/components/page-container/data/__factories__/courseMetadata.factory.js index f09aafb..1b1fc1c 100644 --- a/src/components/page-container/data/__factories__/courseMetadata.factory.js +++ b/src/components/page-container/data/__factories__/courseMetadata.factory.js @@ -6,7 +6,7 @@ export default Factory.define('courseMetadata') .option('host', 'http://localhost:18000') .attrs({ is_staff: true, - original_user_is_staff: false, + original_user_is_staff: true, number: 'DemoX', org: 'edX', title: 'Demonstration Course', diff --git a/src/components/page-container/data/api.js b/src/components/page-container/data/api.js index d767e3f..de70663 100644 --- a/src/components/page-container/data/api.js +++ b/src/components/page-container/data/api.js @@ -1,4 +1,4 @@ -import { getConfig } from '@edx/frontend-platform'; +import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; const courseHomeBaseUrl = `${getConfig().LMS_BASE_URL}/api/course_home/v1/course_metadata`; @@ -6,7 +6,7 @@ const courseHomeBaseUrl = `${getConfig().LMS_BASE_URL}/api/course_home/v1/course export async function getCourseHomeCourseMetadata(courseId) { const courseHomeMetadataUrl = `${courseHomeBaseUrl}/${courseId}`; const { data } = await getAuthenticatedHttpClient().get(courseHomeMetadataUrl); - return data; + return camelCaseObject(data); } export async function getCohorts(courseId) {