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

chore: removed enable_learners_tab_in_discussions_mfe flag dependency #637

Merged
merged 1 commit into from
Jan 5, 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
4 changes: 1 addition & 3 deletions src/discussions/common/AuthorLabel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { Icon, OverlayTrigger, Tooltip } from '@edx/paragon';
import { Institution, School } from '@edx/paragon/icons';

import { Routes } from '../../data/constants';
import { useShowLearnersTab } from '../data/hooks';
import messages from '../messages';
import { DiscussionContext } from './context';
import timeLocale from './time-locale';
Expand Down Expand Up @@ -45,8 +44,7 @@ const AuthorLabel = ({
const showTextPrimary = !authorLabelMessage && !isRetiredUser && !alert;
const className = classNames('d-flex align-items-center', { 'mb-0.5': !postOrComment }, labelColor);

const showUserNameAsLink = useShowLearnersTab()
&& linkToProfile && author && author !== intl.formatMessage(messages.anonymous);
const showUserNameAsLink = linkToProfile && author && author !== intl.formatMessage(messages.anonymous);

const authorName = useMemo(() => (
<span
Expand Down
1 change: 0 additions & 1 deletion src/discussions/common/AuthorLabel.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ describe('Author label', () => {
store = initializeStore();
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
axiosMock.onGet(`${courseConfigApiUrl}${courseId}/`).reply(200, {
learners_tab_enabled: true,
has_moderation_privileges: true,
});
axiosMock.onGet(`${courseConfigApiUrl}${courseId}/settings`).reply(200, {});
Expand Down
3 changes: 0 additions & 3 deletions src/discussions/data/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
selectIsCourseAdmin,
selectIsCourseStaff,
selectIsPostingEnabled,
selectLearnersTabEnabled,
selectPostThreadCount,
selectUserHasModerationPrivileges,
selectUserIsGroupTa,
Expand Down Expand Up @@ -171,8 +170,6 @@ export const useAlertBannerVisible = (
);
};

export const useShowLearnersTab = () => useSelector(selectLearnersTabEnabled);

/**
* React hook that gets the current topic ID from the current topic or category.
* The topicId in the DiscussionContext only return the direct topicId from the URL.
Expand Down
2 changes: 0 additions & 2 deletions src/discussions/data/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ export const selectUserIsGroupTa = state => state.config.isGroupTa;

export const selectConfigLoadingStatus = state => state.config.status;

export const selectLearnersTabEnabled = state => state.config.learnersTabEnabled;

export const selectUserRoles = state => state.config.userRoles;

export const selectDivisionSettings = state => state.config.settings;
Expand Down
1 change: 0 additions & 1 deletion src/discussions/data/slices.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const configSlice = createSlice({
isCourseAdmin: false,
isCourseStaff: false,
isUserAdmin: false,
learnersTabEnabled: false,
isPostingEnabled: false,
settings: {
divisionScheme: 'none',
Expand Down
13 changes: 4 additions & 9 deletions src/discussions/discussions-home/DiscussionSidebar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ import { useWindowSize } from '@edx/paragon';
import Spinner from '../../components/Spinner';
import { RequestStatus, Routes as ROUTES } from '../../data/constants';
import { DiscussionContext } from '../common/context';
import {
useContainerSize, useIsOnDesktop, useIsOnXLDesktop, useShowLearnersTab,
} from '../data/hooks';
import { useContainerSize, useIsOnDesktop, useIsOnXLDesktop } from '../data/hooks';
import { selectConfigLoadingStatus, selectEnableInContext } from '../data/selectors';

const TopicPostsView = lazy(() => import('../in-context-topics/TopicPostsView'));
Expand All @@ -32,7 +30,6 @@ const DiscussionSidebar = ({ displaySidebar, postActionBarRef }) => {
const { enableInContextSidebar } = useContext(DiscussionContext);
const enableInContext = useSelector(selectEnableInContext);
const configStatus = useSelector(selectConfigLoadingStatus);
const redirectToLearnersTab = useShowLearnersTab();
const sidebarRef = useRef(null);
const postActionBarHeight = useContainerSize(postActionBarRef);
const { height: windowHeight } = useWindowSize();
Expand Down Expand Up @@ -103,14 +100,12 @@ const DiscussionSidebar = ({ displaySidebar, postActionBarRef }) => {
{ROUTES.TOPICS.PATH.map(path => (
<Route key={path} path={path} element={<LegacyTopicsView />} />
))}
{redirectToLearnersTab && (
{
[ROUTES.LEARNERS.POSTS, ROUTES.LEARNERS.POSTS_EDIT].map((route) => (
<Route key={route} path={route} element={<LearnerPostsView />} />
))
)}
{redirectToLearnersTab && (
<Route path={ROUTES.LEARNERS.PATH} element={<LearnersView />} />
)}
}
<Route path={ROUTES.LEARNERS.PATH} element={<LearnersView />} />
{configStatus === RequestStatus.SUCCESSFUL && (
<Route path={`${ROUTES.DISCUSSIONS.PATH}/*`} element={<Navigate to="posts" />} />
)}
Expand Down
5 changes: 2 additions & 3 deletions src/discussions/discussions-home/DiscussionsHome.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { selectCourseTabs } from '../../components/NavigationBar/data/selectors'
import { ALL_ROUTES, DiscussionProvider, Routes as ROUTES } from '../../data/constants';
import { DiscussionContext } from '../common/context';
import {
useCourseDiscussionData, useIsOnDesktop, useRedirectToThread, useShowLearnersTab, useSidebarVisible,
useCourseDiscussionData, useIsOnDesktop, useRedirectToThread, useSidebarVisible,
} from '../data/hooks';
import { selectDiscussionProvider, selectEnableInContext } from '../data/selectors';
import { EmptyLearners, EmptyPosts, EmptyTopics } from '../empty-posts';
Expand Down Expand Up @@ -44,7 +44,6 @@ const DiscussionsHome = () => {
const page = pageParams?.page || null;
const matchPattern = ALL_ROUTES.find((route) => matchPath({ path: route }, location.pathname));
const { params } = useMatch(matchPattern);
const isRedirectToLearners = useShowLearnersTab();
const isOnDesktop = useIsOnDesktop();
let displaySidebar = useSidebarVisible();
const enableInContextSidebar = Boolean(new URLSearchParams(location.search).get('inContextSidebar') !== null);
Expand Down Expand Up @@ -146,7 +145,7 @@ const DiscussionsHome = () => {
element={<EmptyPosts subTitleMessage={messages.emptyAllPosts} />}
/>
))}
{isRedirectToLearners && <Route path={ROUTES.LEARNERS.PATH} element={<EmptyLearners />} />}
<Route path={ROUTES.LEARNERS.PATH} element={<EmptyLearners />} />
</>
</Routes>
)}
Expand Down
4 changes: 1 addition & 3 deletions src/discussions/discussions-home/DiscussionsHome.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,7 @@ describe('DiscussionsHome', () => {
);

it('should display empty page message for empty learners list', async () => {
axiosMock.onGet(getDiscussionsConfigUrl(courseId)).reply(200, {
learners_tab_enabled: true,
});
axiosMock.onGet(getDiscussionsConfigUrl(courseId)).reply(200, {});
await executeThunk(fetchCourseConfig(courseId), store.dispatch, store.getState);
await renderComponent(`/${courseId}/learners`);

Expand Down
19 changes: 8 additions & 11 deletions src/discussions/learners/LearnersView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Button, Spinner } from '@edx/paragon';

import SearchInfo from '../../components/SearchInfo';
import { RequestStatus } from '../../data/constants';
import { selectConfigLoadingStatus, selectLearnersTabEnabled } from '../data/selectors';
import { selectConfigLoadingStatus } from '../data/selectors';
import NoResults from '../posts/NoResults';
import {
learnersLoadingStatus,
Expand All @@ -31,18 +31,15 @@ const LearnersView = () => {
const loadingStatus = useSelector(learnersLoadingStatus());
const usernameSearch = useSelector(selectUsernameSearch());
const courseConfigLoadingStatus = useSelector(selectConfigLoadingStatus);
const learnersTabEnabled = useSelector(selectLearnersTabEnabled);
const learners = useSelector(selectAllLearners);

useEffect(() => {
if (learnersTabEnabled) {
if (usernameSearch) {
dispatch(fetchLearners(courseId, { orderBy, usernameSearch }));
} else {
dispatch(fetchLearners(courseId, { orderBy }));
}
if (usernameSearch) {
dispatch(fetchLearners(courseId, { orderBy, usernameSearch }));
} else {
dispatch(fetchLearners(courseId, { orderBy }));
}
}, [courseId, orderBy, learnersTabEnabled, usernameSearch]);
}, [courseId, orderBy, usernameSearch]);

const loadPage = useCallback(async () => {
if (nextPage) {
Expand All @@ -60,12 +57,12 @@ const LearnersView = () => {

const renderLearnersList = useMemo(() => (
(
courseConfigLoadingStatus === RequestStatus.SUCCESSFUL && learnersTabEnabled && learners.map((learner) => (
courseConfigLoadingStatus === RequestStatus.SUCCESSFUL && learners.map((learner) => (
<LearnerCard learner={learner} key={learner.username} />
))
// eslint-disable-next-line react/jsx-no-useless-fragment
) || <></>
), [courseConfigLoadingStatus, learnersTabEnabled, learners]);
), [courseConfigLoadingStatus, learners]);

return (
<div className="d-flex flex-column border-right border-light-400">
Expand Down
9 changes: 0 additions & 9 deletions src/discussions/learners/LearnersView.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ describe('LearnersView', () => {
username: 'test_user',
administrator: true,
roles: [],
learnersTabEnabled: false,
},
});
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
Expand Down Expand Up @@ -106,21 +105,13 @@ describe('LearnersView', () => {

async function assignPrivilages(hasModerationPrivileges = false) {
axiosMock.onGet(getDiscussionsConfigUrl(courseId)).reply(200, {
learners_tab_enabled: true,
user_is_privileged: true,
hasModerationPrivileges,
});

await executeThunk(fetchCourseConfig(courseId), store.dispatch, store.getState);
}

test('Learners tab is disabled by default', async () => {
await setUpLearnerMockResponse();
await renderComponent();

expect(screen.queryByText('learner-1')).not.toBeInTheDocument();
});

test('Learners tab is enabled', async () => {
await setUpLearnerMockResponse();
await assignPrivilages();
Expand Down
16 changes: 5 additions & 11 deletions src/discussions/navigation/navigation-bar/NavigationBar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ import { Nav } from '@edx/paragon';

import { Routes } from '../../../data/constants';
import { DiscussionContext } from '../../common/context';
import { useShowLearnersTab } from '../../data/hooks';
import { discussionsPath } from '../../utils';
import messages from './messages';

const NavigationBar = () => {
const intl = useIntl();
const { courseId } = useContext(DiscussionContext);
const showLearnersTab = useShowLearnersTab();
const location = useLocation();
const isTopicsNavActive = Boolean(matchPath({ path: `${Routes.TOPICS.CATEGORY}/*` }, location.pathname));

Expand All @@ -31,16 +29,12 @@ const NavigationBar = () => {
route: Routes.TOPICS.ALL,
labelMessage: messages.allTopics,
},
]), []);
{
route: Routes.LEARNERS.PATH,
labelMessage: messages.learners,
},

useMemo(() => {
if (showLearnersTab) {
navLinks.push({
route: Routes.LEARNERS.PATH,
labelMessage: messages.learners,
});
}
}, [showLearnersTab]);
]), []);

return (
<Nav variant="pills" className="py-2 nav-button-group">
Expand Down
1 change: 0 additions & 1 deletion src/discussions/posts/post/PostLink.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const mockThread = async (id, abuseFlagged) => {
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
axiosMock.onGet(`${courseConfigApiUrl}${courseId}/settings`).reply(200, {});
axiosMock.onGet(`${courseConfigApiUrl}${courseId}/`).reply(200, {
learners_tab_enabled: true,
has_moderation_privileges: true,
});
axiosMock.onGet(`${threadsApiUrl}${id}/`).reply(200, Factory.build('thread', {
Expand Down
Loading