From db98f66b3adc745ede3bf92a997404679f98ef01 Mon Sep 17 00:00:00 2001
From: AsadAzam <asadazam93@gmail.com>
Date: Sat, 24 Apr 2021 13:21:24 +0500
Subject: [PATCH] Fixed cohorts section step 2 (#84)

* Fixed cohorts section step 2

* Fixed test

* Fixed test

* Added in-context discussions

* Added margin top

* Refactored code

* refactored code
---
 src/generic/FormSwitchGroup.jsx                  |  2 +-
 .../apps/legacy/LegacyConfigForm.jsx             | 11 +++++------
 .../apps/legacy/LegacyConfigForm.test.jsx        |  4 ----
 .../apps/shared/AnonymousPostingFields.jsx       |  2 +-
 .../apps/shared/BlackoutDatesField.jsx           |  3 ++-
 .../apps/shared/DivisionByGroupFields.jsx        | 16 +++-------------
 .../apps/shared/InContextDiscussionFields.jsx    |  5 ++---
 .../app-config-form/apps/shared/messages.js      | 14 ++------------
 8 files changed, 16 insertions(+), 41 deletions(-)

diff --git a/src/generic/FormSwitchGroup.jsx b/src/generic/FormSwitchGroup.jsx
index ed7839819..03062d34f 100644
--- a/src/generic/FormSwitchGroup.jsx
+++ b/src/generic/FormSwitchGroup.jsx
@@ -17,7 +17,7 @@ export default function FormSwitchGroup({
     >
       <div className="d-flex justify-content-between">
         <div className="pr-3">
-          <Form.Label>
+          <Form.Label className="h4 mt-3">
             {label}
           </Form.Label>
           <Form.Text
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
index 0df2bf9c8..fc8761916 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
@@ -35,15 +35,15 @@ function LegacyConfigForm({
   return (
     <Card className="mb-5 pt-3 px-5 pb-5" data-testid="legacyConfigForm">
       <Form ref={formRef} onSubmit={handleSubmit}>
-        <h3>{title}</h3>
-        <AppConfigFormDivider />
-        <DivisionByGroupFields
+        <h3 className="mb-4 mt-2">{title}</h3>
+        <AppConfigFormDivider thick />
+        <AnonymousPostingFields
           onBlur={handleBlur}
           onChange={handleChange}
           values={values}
         />
-        <AppConfigFormDivider thick />
-        <AnonymousPostingFields
+        <AppConfigFormDivider />
+        <DivisionByGroupFields
           onBlur={handleBlur}
           onChange={handleChange}
           values={values}
@@ -63,7 +63,6 @@ function LegacyConfigForm({
 LegacyConfigForm.propTypes = {
   appConfig: PropTypes.shape({
     divideByCohorts: PropTypes.bool.isRequired,
-    allowDivisionByUnit: PropTypes.bool.isRequired,
     divideCourseWideTopics: PropTypes.bool.isRequired,
     divideGeneralTopic: PropTypes.bool.isRequired,
     divideQuestionsForTAsTopic: PropTypes.bool.isRequired,
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
index d1b6df9dc..7d08b0063 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
@@ -6,7 +6,6 @@ import LegacyConfigForm from './LegacyConfigForm';
 const defaultAppConfig = {
   id: 'legacy',
   divideByCohorts: false,
-  allowDivisionByUnit: false,
   divideCourseWideTopics: false,
   divideGeneralTopic: false,
   divideQuestionsForTAsTopic: false,
@@ -74,7 +73,6 @@ describe('LegacyConfigForm', () => {
     // DivisionByGroupFields
     expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
     expect(container.querySelector('#divideByCohorts')).not.toBeChecked();
-    expect(container.querySelector('#allowDivisionByUnit')).not.toBeInTheDocument();
     expect(container.querySelector('#divideCourseWideTopics')).not.toBeInTheDocument();
     expect(container.querySelector('#divideGeneralTopic')).not.toBeInTheDocument();
     expect(container.querySelector('#divideQuestionsForTAsTopic')).not.toBeInTheDocument();
@@ -108,8 +106,6 @@ describe('LegacyConfigForm', () => {
     // DivisionByGroupFields
     expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
     expect(container.querySelector('#divideByCohorts')).toBeChecked();
-    expect(container.querySelector('#allowDivisionByUnit')).toBeInTheDocument();
-    expect(container.querySelector('#allowDivisionByUnit')).not.toBeChecked();
     expect(container.querySelector('#divideCourseWideTopics')).toBeInTheDocument();
     expect(container.querySelector('#divideCourseWideTopics')).not.toBeChecked();
     expect(container.querySelector('#divideGeneralTopic')).toBeInTheDocument();
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx
index 4317fec06..939dbc1b4 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/AnonymousPostingFields.jsx
@@ -14,7 +14,7 @@ function AnonymousPostingFields({
 }) {
   return (
     <>
-      <h5>{intl.formatMessage(messages.anonymousPosting)}</h5>
+      <h5 className="mt-4">{intl.formatMessage(messages.anonymousPosting)}</h5>
       <FormSwitchGroup
         onChange={onChange}
         onBlur={onBlur}
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx
index 5f54744e5..d9e42737c 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/BlackoutDatesField.jsx
@@ -77,7 +77,7 @@ function BlackoutDatesField({
 }) {
   return (
     <>
-      <h5 className="mb-3">{intl.formatMessage(messages.blackoutDates)}</h5>
+      <h5 className="mb-5 mt-4 text-gray-500">{intl.formatMessage(messages.blackoutDates)}</h5>
       <Form.Group
         controlId="blackoutDates"
       >
@@ -85,6 +85,7 @@ function BlackoutDatesField({
           value={values.blackoutDates}
           onChange={onChange}
           onBlur={onBlur}
+          className="mb-3"
           floatingLabel={intl.formatMessage(messages.blackoutDatesLabel)}
         />
         {errors.blackoutDates && (
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx
index 9bcde162f..8c50b9395 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx
@@ -14,10 +14,11 @@ function DivisionByGroupFields({
 }) {
   return (
     <>
-      <h5>{intl.formatMessage(messages.divisionByGroup)}</h5>
+      <h5 className="text-gray-500">{intl.formatMessage(messages.divisionByGroup)}</h5>
 
       <FormSwitchGroup
         onChange={onChange}
+        className="mt-3"
         onBlur={onBlur}
         id="divideByCohorts"
         checked={values.divideByCohorts}
@@ -31,17 +32,7 @@ function DivisionByGroupFields({
             <FormSwitchGroup
               onChange={onChange}
               onBlur={onBlur}
-              className="ml-4"
-              id="allowDivisionByUnit"
-              checked={values.allowDivisionByUnit}
-              label={intl.formatMessage(messages.allowDivisionByUnitLabel)}
-              helpText={intl.formatMessage(messages.allowDivisionByUnitHelp)}
-            />
-            <AppConfigFormDivider />
-            <FormSwitchGroup
-              onChange={onChange}
-              onBlur={onBlur}
-              className="ml-4"
+              className="ml-4 mt-3"
               id="divideCourseWideTopics"
               checked={values.divideCourseWideTopics}
               label={intl.formatMessage(messages.divideCourseWideTopicsLabel)}
@@ -76,7 +67,6 @@ DivisionByGroupFields.propTypes = {
   intl: intlShape.isRequired,
   values: PropTypes.shape({
     divideByCohorts: PropTypes.bool,
-    allowDivisionByUnit: PropTypes.bool,
     divideCourseWideTopics: PropTypes.bool,
     divideGeneralTopic: PropTypes.bool,
     divideQuestionsForTAsTopic: PropTypes.bool,
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx
index d74e3d1b7..ac1cd38b1 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx
@@ -14,7 +14,7 @@ function InContextDiscussionFields({
 }) {
   return (
     <>
-      <h5>{intl.formatMessage(messages.visibilityInContext)}</h5>
+      <h5 className="text-gray-500 mt-4">{intl.formatMessage(messages.visibilityInContext)}</h5>
       <FormSwitchGroup
         onChange={onChange}
         onBlur={onBlur}
@@ -23,10 +23,10 @@ function InContextDiscussionFields({
         label={intl.formatMessage(messages.inContextDiscussionLabel)}
         helpText={intl.formatMessage(messages.inContextDiscussionHelp)}
       />
-      <AppConfigFormDivider />
       <TransitionReplace>
         {values.inContextDiscussion ? (
           <React.Fragment key="open">
+            <AppConfigFormDivider />
             <FormSwitchGroup
               onChange={onChange}
               onBlur={onBlur}
@@ -56,7 +56,6 @@ function InContextDiscussionFields({
               label={intl.formatMessage(messages.allowUnitLevelVisibilityLabel)}
               helpText={intl.formatMessage(messages.allowUnitLevelVisibilityHelp)}
             />
-            <AppConfigFormDivider />
           </React.Fragment>
         ) : <React.Fragment key="closed" />}
 
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js b/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js
index 02e355031..8b87472b8 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js
@@ -4,7 +4,7 @@ const messages = defineMessages({
   // Division by cohort fields
   divisionByGroup: {
     id: 'authoring.discussions.builtIn.divisionByGroup',
-    defaultMessage: 'Division by group',
+    defaultMessage: 'Cohorts',
   },
   divideByCohortsLabel: {
     id: 'authoring.discussions.builtIn.divideByCohorts.label',
@@ -16,16 +16,6 @@ const messages = defineMessages({
     defaultMessage: 'Learners will only be able to view and respond to discussions posted by members of their cohort.',
     description: 'Help text for a switch that enables dividing discussions by cohorts.',
   },
-  allowDivisionByUnitLabel: {
-    id: 'authoring.discussions.builtIn.allowDivisionByUnit.label',
-    defaultMessage: 'Allow cohort division for each course unit',
-    description: 'Label for a switch that enables dividing discussions in course units by cohort.',
-  },
-  allowDivisionByUnitHelp: {
-    id: 'authoring.discussions.builtIn.allowDivisionByUnit.help',
-    defaultMessage: 'With this advanced setting enabled, you will be able to override the global visibility, and turn the division of cohorts on or off for each unit from the course outline view.',
-    description: 'Help text for a switch that enables dividing discussions in course units by cohort.',
-  },
   divideCourseWideTopicsLabel: {
     id: 'authoring.discussions.builtIn.divideCourseWideTopics.label',
     defaultMessage: 'Divide course wide discussion topics',
@@ -58,7 +48,7 @@ const messages = defineMessages({
   },
   inContextDiscussionHelp: {
     id: 'authoring.discussions.builtIn.inContextDiscussion.help',
-    defaultMessage: 'Learners will eb able to view or hide a discussion side panel to engage with discussion on te course unit page.',
+    defaultMessage: 'Learners will be able to view or hide a discussion side panel to engage with discussion on the course unit page.',
   },
   gradedUnitPagesLabel: {
     id: 'authoring.discussions.builtIn.gradedUnitPages.label',