fix: only global staff can see 2 edx discussion providers in settings (#477)

* fix: only global staff can see 2 edx discussion providers in settings

* test: adds and updated test cases for app list

* refactor: memoized showoneedxprovider constant

---------

Co-authored-by: ayesha waris <73840786+ayeshoali@users.noreply.github.com>

src/pages-and-resources/discussions/DiscussionsSettings.test.jsx

@@ -5,17 +5,8 @@ import {
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 import { AppProvider, PageRoute } from '@edx/frontend-platform/react';
 import {
-  act,
-  findByRole,
-  getByRole,
-  queryByLabelText,
-  queryByRole,
-  queryByTestId,
-  queryByText,
-  render,
-  screen,
-  waitFor,
-  waitForElementToBeRemoved,
+  act, findByRole, getByRole, queryByLabelText, queryByRole, queryByTestId, queryByText, render,
+  screen, waitFor, waitForElementToBeRemoved,
 } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import MockAdapter from 'axios-mock-adapter';

src/pages-and-resources/discussions/app-list/AppList.jsx

@@ -1,8 +1,9 @@
-import React, { useCallback, useEffect } from 'react';
+import React, { useCallback, useEffect, useMemo } from 'react';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 import { CardGrid, Container, breakpoints } from '@edx/paragon';
 import { useDispatch, useSelector } from 'react-redux';
 import Responsive from 'react-responsive';
+import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
 import { useModels } from '../../../generic/model-store';
 import {
   selectApp, LOADED, LOADING,
@@ -16,12 +17,17 @@ import Loading from '../../../generic/Loading';
 
 const AppList = ({ intl }) => {
   const dispatch = useDispatch();
-
   const {
     appIds, featureIds, status, activeAppId, selectedAppId,
   } = useSelector(state => state.discussions);
   const apps = useModels('apps', appIds);
   const features = useModels('features', featureIds);
+  const isGlobalStaff = getAuthenticatedUser().administrator;
+  const ltiProvider = !['openedx', 'legacy'].includes(activeAppId);
+
+  const showOneEdxProvider = useMemo(() => apps.filter(app => (
+      activeAppId === 'openedx' ? app.id !== 'legacy' : app.id !== 'openedx'
+    )), [activeAppId]);
 
   // This could be a bit confusing.  activeAppId is the ID of the app that is currently configured
   // according to the server.  selectedAppId is the ID of the app that we _want_ to configure here
@@ -54,6 +60,16 @@ const AppList = ({ intl }) => {
     );
   }
 
+  const showAppCard = (filteredApps) => filteredApps.map(app => (
+    <AppCard
+      key={app.id}
+      app={app}
+      selected={app.id === selectedAppId}
+      onClick={handleSelectApp}
+      features={features}
+    />
+  ));
+
   return (
     <div className="my-sm-5 m-1" data-testid="appList">
       <h3 className="my-sm-5 my-4">
@@ -67,15 +83,7 @@ const AppList = ({ intl }) => {
           xl: 4,
         }}
       >
-        {apps.map(app => (
-          <AppCard
-            key={app.id}
-            app={app}
-            selected={app.id === selectedAppId}
-            onClick={handleSelectApp}
-            features={features}
-          />
-        ))}
+        {(isGlobalStaff || ltiProvider) ? showAppCard(apps) : showAppCard(showOneEdxProvider)}
       </CardGrid>
       <Responsive minWidth={breakpoints.small.minWidth}>
         <h3 className="my-sm-5 my-4">

src/pages-and-resources/discussions/app-list/AppList.test.jsx

@@ -1,14 +1,13 @@
 /* eslint-disable react/jsx-no-constructed-context-values */
 import React from 'react';
-
+import {
+ render, screen, within, queryAllByRole,
+} from '@testing-library/react';
 import { initializeMockApp } from '@edx/frontend-platform';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 import { IntlProvider } from '@edx/frontend-platform/i18n';
 import { AppProvider } from '@edx/frontend-platform/react';
 import { breakpoints } from '@edx/paragon';
-import {
-  queryByText, render, queryAllByRole, queryByRole, getByRole, queryByLabelText, getByLabelText, queryAllByText,
-} from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import MockAdapter from 'axios-mock-adapter';
 import { Context as ResponsiveContext } from 'react-responsive';
@@ -20,84 +19,109 @@ import { fetchDiscussionSettings, fetchProviders } from '../data/thunks';
 import {
   generateProvidersApiResponse,
   piazzaApiResponse,
+  legacyApiResponse,
 } from '../factories/mockApiResponses';
 import AppList from './AppList';
 import messages from './messages';
 
 const courseId = 'course-v1:edX+TestX+Test_Course';
+let axiosMock;
+let store;
+let container;
+
+const mockStore = async (mockResponse, provider) => {
+  axiosMock.onGet(getDiscussionsProvidersUrl(courseId))
+    .reply(200, generateProvidersApiResponse(false, provider));
+  axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, mockResponse);
+  await executeThunk(fetchProviders(courseId), store.dispatch);
+  await executeThunk(fetchDiscussionSettings(courseId), store.dispatch);
+};
+
+function renderComponent(screenWidth = breakpoints.extraLarge.minWidth) {
+  const wrapper = render(
+    <AppProvider store={store}>
+      <ResponsiveContext.Provider value={{ width: screenWidth }}>
+        <IntlProvider locale="en">
+          <AppList />
+        </IntlProvider>
+      </ResponsiveContext.Provider>
+    </AppProvider>,
+  );
+  container = wrapper.container;
+}
 
 describe('AppList', () => {
-  let axiosMock;
-  let store;
-  let container;
+  describe('AppList for Admin role', () => {
+    beforeEach(async () => {
+      initializeMockApp({
+        authenticatedUser: {
+          userId: 3,
+          username: 'abc123',
+          administrator: true,
+          roles: [],
+        },
+      });
 
-  function createComponent(screenWidth = breakpoints.extraLarge.minWidth) {
-    return (
-      <AppProvider store={store}>
-        <ResponsiveContext.Provider value={{ width: screenWidth }}>
-          <IntlProvider locale="en" messages={{}}>
-            <AppList />
-          </IntlProvider>
-        </ResponsiveContext.Provider>
-      </AppProvider>
-    );
-  }
-
-  beforeEach(async () => {
-    initializeMockApp({
-      authenticatedUser: {
-        userId: 3,
-        username: 'abc123',
-        administrator: true,
-        roles: [],
-      },
+      store = await initializeStore();
+      axiosMock = new MockAdapter(getAuthenticatedHttpClient());
+      await mockStore(piazzaApiResponse);
     });
 
-    store = await initializeStore();
-    axiosMock = new MockAdapter(getAuthenticatedHttpClient());
+    test('display a card for each available app', async () => {
+      renderComponent();
+      const appCount = store.getState().discussions.appIds.length;
+      expect(screen.queryAllByRole('radio')).toHaveLength(appCount);
+    });
+
+    test('displays the FeaturesTable at desktop sizes', async () => {
+      renderComponent();
+      expect(screen.queryByRole('table')).toBeInTheDocument();
+    });
+
+    test('hides the FeaturesTable at mobile sizes', async () => {
+      renderComponent(breakpoints.extraSmall.maxWidth);
+      expect(screen.queryByRole('table')).not.toBeInTheDocument();
+    });
+
+    test('hides the FeaturesList at desktop sizes', async () => {
+      renderComponent();
+      expect(screen.queryByText(messages['supportedFeatureList-mobile-show'].defaultMessage)).not.toBeInTheDocument();
+    });
+
+    test('displays the FeaturesList at mobile sizes', async () => {
+      renderComponent(breakpoints.extraSmall.maxWidth);
+      const appCount = store.getState().discussions.appIds.length;
+      expect(screen.queryAllByText(messages['supportedFeatureList-mobile-show'].defaultMessage)).toHaveLength(appCount);
+    });
+
+    test('selectApp is called when an app is clicked', async () => {
+      renderComponent();
+      userEvent.click(screen.getByLabelText('Select Piazza'));
+      const clickedCard = screen.getByRole('radio', { checked: true });
+      expect(within(clickedCard).queryByLabelText('Select Piazza')).toBeInTheDocument();
+    });
   });
 
-  const mockStore = async (mockResponse, screenWidth = breakpoints.extraLarge.minWidth) => {
-    axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse());
-    axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, mockResponse);
-    await executeThunk(fetchProviders(courseId), store.dispatch);
-    await executeThunk(fetchDiscussionSettings(courseId), store.dispatch);
-    const component = createComponent(screenWidth);
-    const wrapper = render(component);
-    container = wrapper.container;
-  };
+  describe('AppList for Non Admin role', () => {
+    beforeEach(async () => {
+      initializeMockApp({
+        authenticatedUser: {
+          userId: 3,
+          username: 'abc123',
+          administrator: false,
+          roles: [],
+        },
+      });
 
-  test('display a card for each available app', async () => {
-    await mockStore(piazzaApiResponse);
-    const appCount = store.getState().discussions.appIds.length;
-    expect(queryAllByRole(container, 'radio')).toHaveLength(appCount);
-  });
+      store = await initializeStore();
+      axiosMock = new MockAdapter(getAuthenticatedHttpClient());
+      await mockStore(legacyApiResponse, 'legacy');
+    });
 
-  test('displays the FeaturesTable at desktop sizes', async () => {
-    await mockStore(piazzaApiResponse);
-    expect(queryByRole(container, 'table')).toBeInTheDocument();
-  });
-
-  test('hides the FeaturesTable at mobile sizes', async () => {
-    await mockStore(piazzaApiResponse, breakpoints.extraSmall.maxWidth);
-    expect(queryByRole(container, 'table')).not.toBeInTheDocument();
-  });
-
-  test('hides the FeaturesList at desktop sizes', async () => {
-    await mockStore(piazzaApiResponse);
-    expect(queryByText(container, messages['supportedFeatureList-mobile-show'].defaultMessage)).not.toBeInTheDocument();
-  });
-
-  test('displays the FeaturesList at mobile sizes', async () => {
-    await mockStore(piazzaApiResponse, breakpoints.extraSmall.maxWidth);
-    const appCount = store.getState().discussions.appIds.length;
-    expect(queryAllByText(container, messages['supportedFeatureList-mobile-show'].defaultMessage)).toHaveLength(appCount);
-  });
-
-  test('selectApp is called when an app is clicked', async () => {
-    await mockStore(piazzaApiResponse);
-    userEvent.click(getByLabelText(container, 'Select Piazza'));
-    const clickedCard = getByRole(container, 'radio', { checked: true });
-    expect(queryByLabelText(clickedCard, 'Select Piazza')).toBeInTheDocument();
+    test('does not display two edx providers card for non admin role', async () => {
+      renderComponent();
+      const appCount = store.getState().discussions.appIds.length;
+      expect(queryAllByRole(container, 'radio')).toHaveLength(appCount - 1);
+    });
   });
 });

src/pages-and-resources/discussions/data/redux.test.js

@@ -161,7 +161,7 @@ describe('Data layer integration tests', () => {
       await executeThunk(fetchDiscussionSettings(courseId), store.dispatch);
 
       expect(store.getState().discussions).toEqual(expect.objectContaining({
-        appIds: ['legacy', 'piazza', 'discourse'],
+        appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
         featureIds,
         activeAppId: 'piazza',
         selectedAppId: null,
@@ -197,7 +197,7 @@ describe('Data layer integration tests', () => {
       await executeThunk(fetchDiscussionSettings(courseId), store.dispatch);
 
       expect(store.getState().discussions).toEqual(expect.objectContaining({
-        appIds: ['legacy', 'piazza', 'discourse'],
+        appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
         featureIds,
         activeAppId: 'piazza',
         selectedAppId: null,
@@ -225,7 +225,7 @@ describe('Data layer integration tests', () => {
       await executeThunk(fetchDiscussionSettings(courseId), store.dispatch);
 
       expect(store.getState().discussions).toEqual(expect.objectContaining({
-        appIds: ['legacy', 'piazza', 'discourse'],
+        appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
         featureIds,
         activeAppId: 'legacy',
         selectedAppId: null,
@@ -292,7 +292,7 @@ describe('Data layer integration tests', () => {
       expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`);
       expect(store.getState().discussions).toEqual(
         expect.objectContaining({
-          appIds: ['legacy', 'piazza', 'discourse'],
+          appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
           featureIds,
           activeAppId: 'piazza',
           selectedAppId: 'piazza',
@@ -319,7 +319,7 @@ describe('Data layer integration tests', () => {
       expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`);
       expect(store.getState().discussions).toEqual(
         expect.objectContaining({
-          appIds: ['legacy', 'piazza', 'discourse'],
+          appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
           featureIds,
           activeAppId: 'piazza',
           selectedAppId: 'piazza',
@@ -374,7 +374,7 @@ describe('Data layer integration tests', () => {
       expect(window.location.pathname).toEqual(pagesAndResourcesPath);
       expect(store.getState().discussions).toEqual(
         expect.objectContaining({
-          appIds: ['legacy', 'piazza', 'discourse'],
+          appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
           featureIds,
           activeAppId: 'piazza',
           selectedAppId: 'piazza',
@@ -468,7 +468,7 @@ describe('Data layer integration tests', () => {
       expect(window.location.pathname).toEqual(pagesAndResourcesPath);
       expect(store.getState().discussions).toEqual(
         expect.objectContaining({
-          appIds: ['legacy', 'piazza', 'discourse'],
+          appIds: ['legacy', 'openedx', 'piazza', 'discourse'],
           featureIds,
           activeAppId: 'legacy',
           selectedAppId: 'legacy',

src/pages-and-resources/discussions/factories/mockApiResponses.js

@@ -42,6 +42,24 @@ export const generateProvidersApiResponse = (piazzaAdminOnlyConfig = false, acti
       has_full_support: true,
       admin_only_config: false,
     },
+    openedx: {
+      features: [
+        'basic-configuration',
+        'discussion-page',
+        'embedded-course-sections',
+        'wcag-2.1',
+      ],
+      external_links: {
+        learn_more: '',
+        configuration: '',
+        general: '',
+        accessibility: '',
+        contact_email: '',
+      },
+      messages: [],
+      has_full_support: true,
+      admin_only_config: false,
+    },
     piazza: {
       features: [
         // We give piazza all features just so we can test our "full support" text.