Skip to content

Commit

Permalink
fix: removing references to outdated variable (#1337)
Browse files Browse the repository at this point in the history
* fix: removing references to outdated variable

* fix: PR requests
  • Loading branch information
kiram15 authored Oct 17, 2024
1 parent b69e59e commit 38f28c9
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
9 changes: 5 additions & 4 deletions src/components/ContentHighlights/ContentHighlights.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ import ContentHighlightsContextProvider from './ContentHighlightsContext';
import ContentHighlightToast from './ContentHighlightToast';
import { EnterpriseAppContext } from '../EnterpriseApp/EnterpriseAppContextProvider';
import { withLocation } from '../../hoc';
import { GROUP_TYPE_BUDGET } from '../PeopleManagement/constants';

const ContentHighlights = ({ location, enterpriseGroupsV1 }) => {
const navigate = useNavigate();
const { state: locationState } = location;
const [toasts, setToasts] = useState([]);
const isEdxStaff = getAuthenticatedUser().administrator;
const [isSubGroup, setIsSubGroup] = useState(false);
const [hasBudgetGroup, setHasBudgetGroup] = useState(false);
const { enterpriseCuration: { enterpriseCuration } } = useContext(EnterpriseAppContext);
const intl = useIntl();

Expand All @@ -31,8 +32,8 @@ const ContentHighlights = ({ location, enterpriseGroupsV1 }) => {
try {
const response = await LmsApiService.fetchEnterpriseGroups();
response.data.results.forEach((group) => {
if (group.applies_to_all_contexts === false) {
setIsSubGroup(true);
if (group.group_type === GROUP_TYPE_BUDGET) {
setHasBudgetGroup(true);
}
});
} catch (error) {
Expand Down Expand Up @@ -102,7 +103,7 @@ const ContentHighlights = ({ location, enterpriseGroupsV1 }) => {
description: 'Hero title for the highlights page.',
})}
/>
{isSubGroup && (
{hasBudgetGroup && (
<Alert variant="warning" className="mt-4 mb-0" icon={WarningFilled}>
<Alert.Heading>
<FormattedMessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { IntlProvider } from '@edx/frontend-platform/i18n';
import ContentHighlights from '../ContentHighlights';
import { EnterpriseAppContext } from '../../EnterpriseApp/EnterpriseAppContextProvider';
import LmsApiService from '../../../data/services/LmsApiService';
import { GROUP_TYPE_BUDGET } from '../../PeopleManagement/constants';

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

Expand Down Expand Up @@ -86,7 +87,7 @@ describe('<ContentHighlights>', () => {
});
it('Displays the alert if custom groups is enabled and user is staff', () => {
LmsApiService.fetchEnterpriseGroups.mockImplementation(() => Promise.resolve({
data: { results: [{ applies_to_all_contexts: true }] },
data: { results: [{ group_type: GROUP_TYPE_BUDGET }] },
}));
renderWithRouter(<ContentHighlightsWrapper location={{ state: {} }} />);
});
Expand Down
6 changes: 4 additions & 2 deletions src/components/PeopleManagement/constants.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
const MAX_LENGTH_GROUP_NAME = 60;
export default MAX_LENGTH_GROUP_NAME;
export const MAX_LENGTH_GROUP_NAME = 60;

export const GROUP_TYPE_BUDGET = 'budget';
export const GROUP_TYPE_FLEX = 'flex';
9 changes: 5 additions & 4 deletions src/components/Sidebar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { useOnMount } from '../../hooks';
import { EnterpriseSubsidiesContext } from '../EnterpriseSubsidiesContext';
import { EnterpriseAppContext } from '../EnterpriseApp/EnterpriseAppContextProvider';
import LmsApiService from '../../data/services/LmsApiService';
import { GROUP_TYPE_BUDGET } from '../PeopleManagement/constants';

const Sidebar = ({
baseUrl,
Expand All @@ -48,8 +49,8 @@ const Sidebar = ({
const { canManageLearnerCredit } = useContext(EnterpriseSubsidiesContext);
const { FEATURE_CONTENT_HIGHLIGHTS } = getConfig();
const isEdxStaff = getAuthenticatedUser().administrator;
const [isSubGroup, setIsSubGroup] = useState(false);
const hideHighlightsForGroups = enterpriseGroupsV1 && isSubGroup && !isEdxStaff;
const [hasBudgetGroup, setHasBudgetGroup] = useState(false);
const hideHighlightsForGroups = enterpriseGroupsV1 && hasBudgetGroup && !isEdxStaff;
const intl = useIntl();

const getSidebarWidth = useCallback(() => {
Expand Down Expand Up @@ -89,8 +90,8 @@ const Sidebar = ({
// 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);
if (group.group_type === GROUP_TYPE_BUDGET) {
setHasBudgetGroup(true);
}
});
} catch (error) {
Expand Down
10 changes: 5 additions & 5 deletions src/containers/Sidebar/Sidebar.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,9 @@ describe('<Sidebar />', () => {
});

it.each([
{ appliesToAllContexts: true },
{ appliesToAllContexts: false },
])('hides highlights when we have groups with a subset of all learners (%s)', async ({ appliesToAllContexts }) => {
{ groupType: 'budget' },
{ groupType: 'flex' },
])('hides highlights when we have budget groups (%s)', async ({ groupType }) => {
getAuthenticatedUser.mockReturnValue({
administrator: false,
});
Expand All @@ -460,12 +460,12 @@ describe('<Sidebar />', () => {
},
});
LmsApiService.fetchEnterpriseGroups.mockImplementation(() => Promise.resolve({
data: { results: [{ applies_to_all_contexts: appliesToAllContexts }] },
data: { results: [{ group_type: groupType }] },
}));
render(<SidebarWrapper store={store} />);
const highlightsLink = screen.queryByRole('link', { name: 'Highlights' });
await waitFor(() => {
if (appliesToAllContexts) {
if (groupType === 'flex') {
expect(highlightsLink).toBeInTheDocument();
} else {
expect(highlightsLink).not.toBeInTheDocument();
Expand Down

0 comments on commit 38f28c9

Please sign in to comment.