Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hiding highlights for custom courses #1191

Merged
merged 3 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/components/Sidebar/index.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {
useRef, useContext, useEffect, useCallback,
useCallback, useContext, useEffect, useRef, useState,
} from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
Expand All @@ -8,6 +8,7 @@ import {
BookOpen, CreditCard, Description, InsertChartOutlined, MoneyOutline, Settings, Support, Tag, TrendingUp,
} from '@edx/paragon/icons';

import { logError } from '@edx/frontend-platform/logging';
import { getConfig } from '@edx/frontend-platform/config';
import IconLink from './IconLink';

Expand All @@ -18,6 +19,7 @@ import { TOUR_TARGETS } from '../ProductTours/constants';
import { useOnMount } from '../../hooks';
import { EnterpriseSubsidiesContext } from '../EnterpriseSubsidiesContext';
import { EnterpriseAppContext } from '../EnterpriseApp/EnterpriseAppContextProvider';
import LmsApiService from '../../data/services/LmsApiService';

const Sidebar = ({
baseUrl,
Expand All @@ -31,13 +33,16 @@ const Sidebar = ({
enableAnalyticsScreen,
onWidthChange,
isMobile,
enterpriseGroupsV1,
}) => {
const navRef = useRef();
const widthRef = useRef();
const { enterpriseCuration: { enterpriseCuration, isNewArchivedContent } } = useContext(EnterpriseAppContext);
const { subsidyRequestsCounts } = useContext(SubsidyRequestsContext);
const { canManageLearnerCredit } = useContext(EnterpriseSubsidiesContext);
const { FEATURE_CONTENT_HIGHLIGHTS } = getConfig();
const [isSubGroup, setIsSubGroup] = useState(false);
const hideHighlightsForGroups = enterpriseGroupsV1 && isSubGroup;

const getSidebarWidth = useCallback(() => {
if (navRef && navRef.current) {
Expand All @@ -54,6 +59,23 @@ const Sidebar = ({
widthRef.current = sideBarWidth;
onWidthChange(sideBarWidth);
}
async function fetchGroupsData() {
try {
const response = await LmsApiService.fetchEnterpriseGroup();
// we only want to hide the feature if a customer has a group this does not
// apply to all contexts/include all users
response.data.results.forEach((group) => {
if (group.applies_to_all_contexts === false) {
setIsSubGroup(true);
}
});
} catch (error) {
logError(error);
}
}
if (enterpriseGroupsV1) {
fetchGroupsData();
}
});

useEffect(() => {
Expand Down Expand Up @@ -104,7 +126,7 @@ const Sidebar = ({
id: TOUR_TARGETS.CONTENT_HIGHLIGHTS,
to: `${baseUrl}/admin/${ROUTE_NAMES.contentHighlights}`,
icon: <Icon src={BookOpen} />,
hidden: !FEATURE_CONTENT_HIGHLIGHTS || !enterpriseCuration?.isHighlightFeatureActive,
hidden: !FEATURE_CONTENT_HIGHLIGHTS || !enterpriseCuration?.isHighlightFeatureActive || hideHighlightsForGroups,
notification: isNewArchivedContent,
},
{
Expand Down Expand Up @@ -187,6 +209,7 @@ Sidebar.defaultProps = {
enableAnalyticsScreen: false,
onWidthChange: () => {},
isMobile: false,
enterpriseGroupsV1: false,
};

Sidebar.propTypes = {
Expand All @@ -201,6 +224,7 @@ Sidebar.propTypes = {
enableAnalyticsScreen: PropTypes.bool,
onWidthChange: PropTypes.func,
isMobile: PropTypes.bool,
enterpriseGroupsV1: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would help readability to have a name that's a bit more explicit that it is a feature flag, IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it follows the same naming pattern as the other features, and you can see thats its being brought in from enterpriseFeatures

};

export default Sidebar;
41 changes: 39 additions & 2 deletions src/containers/Sidebar/Sidebar.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable react/prop-types */
import React from 'react';
import { getConfig } from '@edx/frontend-platform/config';
import PropTypes from 'prop-types';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
Expand All @@ -12,12 +11,13 @@ import {
render, screen,
} from '@testing-library/react';
import '@testing-library/jest-dom';
import { getConfig } from '@edx/frontend-platform/config';

import Sidebar from './index';
import { SubsidyRequestsContext } from '../../components/subsidy-requests';
import { EnterpriseSubsidiesContext } from '../../components/EnterpriseSubsidiesContext';
import { EnterpriseAppContext } from '../../components/EnterpriseApp/EnterpriseAppContextProvider';

import LmsApiService from '../../data/services/LmsApiService';
import { features } from '../../config';

import {
Expand All @@ -34,6 +34,8 @@ jest.mock('@edx/frontend-platform/config', () => ({
})),
}));

jest.mock('../../data/services/LmsApiService');

const mockStore = configureMockStore([thunk]);
const initialState = {
sidebar: {
Expand All @@ -46,6 +48,9 @@ const initialState = {
enableSubscriptionManagementScreen: true,
enableAnalyticsScreen: true,
enableReportingConfigScreenLink: true,
enterpriseFeatures: {
enterpriseGroupsV1: false,
},
},
};

Expand Down Expand Up @@ -127,6 +132,9 @@ describe('<Sidebar />', () => {
},
portalConfiguration: {
enableCodeManagementScreen: false,
enterpriseFeatures: {
enterpriseGroupsV1: false,
},
},
});

Expand Down Expand Up @@ -421,6 +429,35 @@ describe('<Sidebar />', () => {
}
});

it('hides highlights when we have groups with a subset of all learners', async () => {
getConfig.mockReturnValue({ FEATURE_CONTENT_HIGHLIGHTS: true });
const store = mockStore({
...initialState,
portalConfiguration: {
enterpriseFeatures: {
enterpriseGroupsV1: true,
},
},
});

LmsApiService.fetchEnterpriseGroup.mockImplementation(() => Promise.resolve({
data: { results: [{ applies_to_all_contexts: false }] },
}));
render(<SidebarWrapper store={store} />);
const highlightsLink = expect(screen.queryByRole('link', { name: 'Highlights' }));
// we have to wait for the async call to set the state
setTimeout(() => {
expect(highlightsLink).not.toBeInTheDocument();
}, 1000);

LmsApiService.fetchEnterpriseGroup.mockImplementation(() => Promise.resolve({
data: { results: [{ applies_to_all_contexts: true }] },
}));
render(<SidebarWrapper store={store} />);
setTimeout(() => {
expect(highlightsLink).toBeInTheDocument();
}, 1000);
});
describe('notifications', () => {
it('displays notification bubble when there are outstanding license requests', () => {
const contextValue = { subsidyRequestsCounts: { subscriptionLicenses: 2 } };
Expand Down
1 change: 1 addition & 0 deletions src/containers/Sidebar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const mapStateToProps = state => ({
enableLearnerPortal: state.portalConfiguration.enableLearnerPortal,
enableLmsConfigurationsScreen: state.portalConfiguration.enableLmsConfigurationsScreen,
enableAnalyticsScreen: state.portalConfiguration.enableAnalyticsScreen,
enterpriseGroupsV1: state.portalConfiguration.enterpriseFeatures?.enterpriseGroupsV1,
});

const mapDispatchToProps = dispatch => ({
Expand Down
5 changes: 5 additions & 0 deletions src/data/services/LmsApiService.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ class LmsApiService {
return response;
};

static fetchEnterpriseGroup = async () => {
const url = `${LmsApiService.enterpriseGroupUrl}`;
return LmsApiService.apiClient().get(url);
};

static inviteEnterpriseLearnersToGroup = async (groupUuid, formData) => {
const assignLearnerEndpoint = `${LmsApiService.enterpriseGroupUrl}${groupUuid}/assign_learners/`;
return LmsApiService.apiClient().post(assignLearnerEndpoint, formData);
Expand Down
Loading