Andal.LND/frontend-app-authoring
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: divide discussions by cohort toggle should not divide course-wid… (#233)

Browse Source 
* fix: divide discussions by cohort toggle should not divide course-wide discussions

* test: update failed test cases
This commit is contained in:
Awais Ansari
2022-01-18 17:16:00 +05:00
committed by GitHub
parent a44f11a731
commit 7e9261e30f
4 changed files with 43 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
View File

@@ -29,23 +29,21 @@ import { selectApp } from '../../../data/slice';
import { DivisionSchemes } from '../../../../../data/constants';
const courseId = 'course-v1:edX+TestX+Test_Course';
const defaultAppConfig = {
const defaultAppConfig = (divideDiscussionIds = []) => ({
id: 'legacy',
divideByCohorts: false,
divideCourseTopicsByCohorts: false,
alwaysDivideInlineDiscussions: false,
discussionTopics: [
{ name: 'Edx', id: '13f106c6-6735-4e84-b097-0456cff55960' },
{ name: 'General', id: 'course' },
],
divideDiscussionIds: [
'13f106c6-6735-4e84-b097-0456cff55960',
'course',
],
divideDiscussionIds,
allowAnonymousPosts: false,
allowAnonymousPostsPeers: false,
allowDivisionByUnit: false,
blackoutDates: [],
};
});
describe('LegacyConfigForm', () => {
let axiosMock;
let store;
@@ -111,8 +109,8 @@ describe('LegacyConfigForm', () => {
// any of the form inputs, this exact object shape is returned back to us, so we're reusing
// it here. It's not supposed to be 'the same object', it just happens to be.
{
...defaultAppConfig,
divideByCohorts: true,
...defaultAppConfig(),
divideByCohorts: false,
divisionScheme: DivisionSchemes.COHORT,
},
);
@@ -121,12 +119,14 @@ describe('LegacyConfigForm', () => {
test('default field states are correct, including removal of folded sub-fields', async () => {
await mockStore({ ...legacyApiResponse, plugin_configuration: { divided_course_wide_discussions: [] } });
createComponent();
const { divideDiscussionIds } = defaultAppConfig(['13f106c6-6735-4e84-b097-0456cff55960', 'course']);
// DivisionByGroupFields
expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
expect(container.querySelector('#divideByCohorts')).not.toBeChecked();
expect(container.querySelector('#divideCourseTopicsByCohorts')).not.toBeInTheDocument();
defaultAppConfig.divideDiscussionIds.forEach(id => expect(
divideDiscussionIds.forEach(id => expect(
container.querySelector(`#checkbox-${id}`),
).not.toBeInTheDocument());
@@ -144,9 +144,15 @@ describe('LegacyConfigForm', () => {
test('folded sub-fields are in the DOM when parents are enabled', async () => {
await mockStore({
...legacyApiResponse,
plugin_configuration: { ...legacyApiResponse.plugin_configuration, allow_anonymous: true },
plugin_configuration: {
...legacyApiResponse.plugin_configuration,
allow_anonymous: true,
always_divide_inline_discussions: true,
divided_course_wide_discussions: [],
},
});
createComponent();
const { divideDiscussionIds } = defaultAppConfig(['13f106c6-6735-4e84-b097-0456cff55960', 'course']);
// DivisionByGroupFields
expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
@@ -158,7 +164,7 @@ describe('LegacyConfigForm', () => {
container.querySelector('#divideCourseTopicsByCohorts'),
).not.toBeChecked();
defaultAppConfig.divideDiscussionIds.forEach(id => expect(
divideDiscussionIds.forEach(id => expect(
container.querySelector(`#checkbox-${id}`),
).not.toBeInTheDocument());
@@ -179,11 +185,13 @@ describe('LegacyConfigForm', () => {
...legacyApiResponse,
plugin_configuration: {
...legacyApiResponse.plugin_configuration,
divided_course_wide_discussions: ['13f106c6-6735-4e84-b097-0456cff55960',
'course', 'test-topic'],
allow_anonymous: true,
always_divide_inline_discussions: true,
divided_course_wide_discussions: ['13f106c6-6735-4e84-b097-0456cff55960', 'course'],
},
});
createComponent();
const { divideDiscussionIds } = defaultAppConfig(['13f106c6-6735-4e84-b097-0456cff55960', 'course']);
// DivisionByGroupFields
expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
@@ -191,7 +199,7 @@ describe('LegacyConfigForm', () => {
expect(container.querySelector('#divideCourseTopicsByCohorts')).toBeInTheDocument();
expect(container.querySelector('#divideCourseTopicsByCohorts')).toBeChecked();
defaultAppConfig.divideDiscussionIds.forEach(id => {
divideDiscussionIds.forEach(id => {
expect(container.querySelector(`#checkbox-${id}`)).toBeInTheDocument();
expect(container.querySelector(`#checkbox-${id}`)).toBeChecked();
});

15
src/pages-and-resources/discussions/data/api.js
View File

@@ -57,17 +57,16 @@ function normalizePluginConfig(data) {
if (!data || Object.keys(data).length < 1) {
return {};
}
const discussionDividedTopicsCount = _.size(data.divided_course_wide_discussions);
const discussionTopicsCount = _.size(data.discussion_topics);
const enableDivideCourseTopicsByCohorts = Boolean(discussionDividedTopicsCount
&& (discussionDividedTopicsCount !== discussionTopicsCount));
const enableDivideByCohorts = data.always_divide_inline_discussions && data.division_scheme === 'cohort';
const enableDivideCourseTopicsByCohorts = enableDivideByCohorts && data.divided_course_wide_discussions.length > 0;
return {
allowAnonymousPosts: data.allow_anonymous,
allowAnonymousPostsPeers: data.allow_anonymous_to_peers,
divisionScheme: data.division_scheme,
alwaysDivideInlineDiscussions: data.always_divide_inline_discussions,
blackoutDates: normalizeBlackoutDates(data.discussion_blackouts),
allowDivisionByUnit: false,
divideByCohorts: discussionDividedTopicsCount > 0,
divideByCohorts: enableDivideByCohorts,
divideCourseTopicsByCohorts: enableDivideCourseTopicsByCohorts,
};
}
@@ -180,6 +179,7 @@ function denormalizeData(courseId, appId, data) {
}
if ('divideByCohorts' in data) {
pluginConfiguration.division_scheme = data.divideByCohorts ? DivisionSchemes.COHORT : DivisionSchemes.NONE;
pluginConfiguration.always_divide_inline_discussions = data.divideByCohorts;
}
if (data.blackoutDates?.length) {
pluginConfiguration.discussion_blackouts = data.blackoutDates.map((blackoutDates) => (
@@ -195,8 +195,9 @@ function denormalizeData(courseId, appId, data) {
return newTopics;
}, {});
}
if (data.divideDiscussionIds) {
pluginConfiguration.divided_course_wide_discussions = data.divideDiscussionIds;
if ('divideCourseTopicsByCohorts' in data) {
pluginConfiguration.divided_course_wide_discussions = data.divideCourseTopicsByCohorts
? data.divideDiscussionIds : [];
}
const ltiConfiguration = {};

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

@@ -227,7 +227,7 @@ describe('Data layer integration tests', () => {
saveStatus: SAVED,
hasValidationError: false,
discussionTopicIds,
divideDiscussionIds,
divideDiscussionIds: [],
});
expect(store.getState().models.apps.legacy).toEqual(legacyApp);
expect(store.getState().models.apps.piazza).toEqual(piazzaApp);
@@ -240,7 +240,8 @@ describe('Data layer integration tests', () => {
// TODO: Note! As of this writing, all the data below this line is NOT returned in the API
// but we add it in during normalization.
divisionScheme: DivisionSchemes.COHORT,
divideByCohorts: true,
divideByCohorts: false,
alwaysDivideInlineDiscussions: false,
allowDivisionByUnit: false,
divideCourseTopicsByCohorts: false,
});
@@ -388,8 +389,9 @@ describe('Data layer integration tests', () => {
plugin_configuration: {
allow_anonymous: true,
allow_anonymous_to_peers: true,
always_divide_inline_discussions: true,
discussion_blackouts: [],
division_scheme: DivisionSchemes.NONE,
division_scheme: DivisionSchemes.COHORT,
discussion_topics: {
Edx: { id: '13f106c6-6735-4e84-b097-0456cff55960' },
General: { id: 'course' },
@@ -406,6 +408,7 @@ describe('Data layer integration tests', () => {
plugin_configuration: {
allow_anonymous: true,
allow_anonymous_to_peers: true,
always_divide_inline_discussions: true,
discussion_blackouts: [],
division_scheme: DivisionSchemes.COHORT,
discussion_topics: {
@@ -431,9 +434,10 @@ describe('Data layer integration tests', () => {
blackoutDates: [],
// TODO: Note! As of this writing, all the data below this line is NOT returned in the API
// but we technically send it to the thunk, so here it is.
divideByCohorts: false,
allowDivisionByUnit: true,
divideCourseTopicsByCohorts: false,
divideByCohorts: true,
allowDivisionsByUnit: true,
alwaysDivideInlineDiscussions: true,
divideCourseTopicsByCohorts: true,
divideDiscussionIds,
discussionTopics: [
{ name: 'Edx', id: '13f106c6-6735-4e84-b097-0456cff55960' },
@@ -462,13 +466,14 @@ describe('Data layer integration tests', () => {
// These three fields should be updated.
allowAnonymousPosts: true,
allowAnonymousPostsPeers: true,
alwaysDivideInlineDiscussions: true,
blackoutDates: [],
// TODO: Note! The values we tried to save were ignored, this test reflects what currently
// happens, but NOT what we want to have happen!
divideByCohorts: true,
divisionScheme: DivisionSchemes.COHORT,
allowDivisionByUnit: false,
divideCourseTopicsByCohorts: false,
divideCourseTopicsByCohorts: true,
});
});
});

6
src/pages-and-resources/discussions/factories/mockApiResponses.js
View File

@@ -99,11 +99,7 @@ export const generateLegacyApiResponse = () => ({
Edx: { id: '13f106c6-6735-4e84-b097-0456cff55960' },
General: { id: 'course' },
},
divided_course_wide_discussions: [
'13f106c6-6735-4e84-b097-0456cff55960',
'course',
],
divided_inline_discussions: [],
divided_course_wide_discussions: [],
division_scheme: DivisionSchemes.COHORT,
// Note, this gets stringified when normalized into the app, but the API returns it as an
// actual array. Argh.