Merge pull request #247 from edx/aj/fix-certificate-checklist

fix(coursechecklist): fix certificate check if cert url is empty
openedx · Sep 14, 2018 · 253ad98 · 253ad98
2 parents 128f3cb + 3272e72
commit 253ad98
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 24 deletions.
diff --git a/src/components/CourseChecklist/CourseChecklist.test.jsx b/src/components/CourseChecklist/CourseChecklist.test.jsx
@@ -61,6 +61,12 @@ const testData = {
   grades: {
     sum_of_weights: 1,
   },
+  sections: {
+    highlights_enabled: true,
+  },
+  certificates: {
+    is_enabled: true,
+  },
 };
 
 const defaultProps = {

diff --git a/src/components/CourseChecklist/index.jsx b/src/components/CourseChecklist/index.jsx
@@ -329,8 +329,11 @@ class CourseChecklist extends React.Component {
 
   updateChecklistState(props) {
     if (Object.keys(props.data).length > 0) {
-      const checks = getFilteredChecklist(
-        props.dataList, props.data.is_self_paced);
+      const isSelfPaced = props.data.is_self_paced;
+      const hasCertificatesEnabled = props.data.certificates && props.data.certificates.is_enabled;
+      const hasHighlightsEnabled = props.data.sections && props.data.sections.highlights_enabled;
+      const checks = getFilteredChecklist(props.dataList,
+        isSelfPaced, hasCertificatesEnabled, hasHighlightsEnabled);
 
       const values = {};
       let totalCompletedChecks = 0;
@@ -397,6 +400,7 @@ CourseChecklist.propTypes = {
         has_update: PropTypes.bool,
       }),
       certificates: PropTypes.shape({
+        is_enabled: PropTypes.bool,
         is_activated: PropTypes.bool,
         has_certificate: PropTypes.bool,
       }),
@@ -422,6 +426,7 @@ CourseChecklist.propTypes = {
         has_update: PropTypes.bool,
       }),
       certificates: PropTypes.shape({
+        is_enabled: PropTypes.bool,
         is_activated: PropTypes.bool,
         has_certificate: PropTypes.bool,
       }),

diff --git a/src/components/CourseChecklistPage/index.jsx b/src/components/CourseChecklistPage/index.jsx
@@ -93,6 +93,7 @@ CourseChecklistPage.propTypes = {
       total_visible: PropTypes.number,
       total_number: PropTypes.number,
       highlights_enabled: PropTypes.bool,
+      highlights_active_for_course: PropTypes.bool,
     }),
     subsections: PropTypes.object,
     units: PropTypes.object,
@@ -115,6 +116,7 @@ CourseChecklistPage.propTypes = {
       has_update: PropTypes.bool,
     }),
     certificates: PropTypes.shape({
+      is_enabled: PropTypes.bool,
       is_activated: PropTypes.bool,
       has_certificate: PropTypes.bool,
     }),

diff --git a/src/components/CourseOutlineStatus/CourseOutlineStatus.test.jsx b/src/components/CourseOutlineStatus/CourseOutlineStatus.test.jsx
@@ -26,6 +26,10 @@ const testChecklistData = ['a', 'b', 'c', 'd'].reduce(((accumulator, currentValu
 const testChecklist = {
   heading: 'test',
   data: testChecklistData,
+  sections: { highlights_enabled: true },
+};
+const testCourseLaunchData = {
+  certificates: { is_enabled: true },
 };
 
 /**
@@ -338,7 +342,7 @@ describe('CourseOutlineStatus', () => {
 
         wrapper.setProps({
           courseBestPracticesData: testChecklist,
-          courseLaunchData: testChecklist,
+          courseLaunchData: testCourseLaunchData,
         });
 
         const checklistsLink = wrapper.find(CourseOutlineStatusValue).at(2).find(Hyperlink);
@@ -370,7 +374,7 @@ describe('CourseOutlineStatus', () => {
 
         wrapper.setProps({
           courseBestPracticesData: testChecklist,
-          courseLaunchData: testChecklist,
+          courseLaunchData: testCourseLaunchData,
         });
 
         const checklistsLink = wrapper.find(CourseOutlineStatusValue).at(2).find(Hyperlink);

diff --git a/src/components/CourseOutlineStatus/index.jsx b/src/components/CourseOutlineStatus/index.jsx
@@ -31,11 +31,14 @@ export default class CourseOutlineStatus extends React.Component {
   }
 
   componentWillMount() {
+    const isSelfPaceCourse = this.props.studioDetails.course.is_course_self_paced;
+    const hasCertificatesEnabled = false;
+    const hasHighlightsEnabled = false;
     this.setState({
       totalCourseBestPracticesChecks: getFilteredChecklist(bestPracticesChecklist.data,
-        this.props.studioDetails.course.is_course_self_paced).length,
+        isSelfPaceCourse, hasCertificatesEnabled, hasHighlightsEnabled).length,
       totalCourseLaunchChecks: getFilteredChecklist(launchChecklist.data,
-        this.props.studioDetails.course.is_course_self_paced).length,
+        isSelfPaceCourse, hasCertificatesEnabled, hasHighlightsEnabled).length,
     });
   }
 
@@ -45,17 +48,24 @@ export default class CourseOutlineStatus extends React.Component {
   }
 
   componentWillReceiveProps(nextProps) {
+    const courseData = {
+      isSelfPaced: false,
+      hasCertificatesEnabled: false,
+      hasHighlightsEnabled: false,
+    };
     if (Object.keys(nextProps.courseLaunchData).length > 0) {
-      const checks = getFilteredChecklist(
-        launchChecklist.data, nextProps.courseLaunchData.is_self_paced);
+      courseData.isSelfPaced = nextProps.courseLaunchData.is_self_paced;
+      courseData.hasCertificatesEnabled = nextProps.courseLaunchData.certificates.is_enabled;
+      const filteredCourseLaunchChecks = getFilteredChecklist(launchChecklist.data,
+        courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
 
       let completedCourseLaunchChecks = 0;
 
       const props = {
         data: nextProps.courseLaunchData,
       };
 
-      checks.forEach((check) => {
+      filteredCourseLaunchChecks.forEach((check) => {
         const value = getValidatedValue(props, check.id);
 
         if (value) {
@@ -65,21 +75,24 @@ export default class CourseOutlineStatus extends React.Component {
 
       this.setState({
         completedCourseLaunchChecks,
+        totalCourseLaunchChecks: filteredCourseLaunchChecks.length,
       });
     }
 
     if (Object.keys(nextProps.courseBestPracticesData).length > 0
       && nextProps.studioDetails.enable_quality) {
-      const checks = getFilteredChecklist(
-        bestPracticesChecklist.data, nextProps.courseBestPracticesData.is_self_paced);
+      courseData.hasHighlightsEnabled =
+        nextProps.courseBestPracticesData.sections.highlights_enabled;
+      const filteredCourseBestPracticesChecks = getFilteredChecklist(bestPracticesChecklist.data,
+        courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
 
       let completedCourseBestPracticesChecks = 0;
 
       const props = {
         data: nextProps.courseBestPracticesData,
       };
 
-      checks.forEach((check) => {
+      filteredCourseBestPracticesChecks.forEach((check) => {
         const value = getValidatedValue(props, check.id);
 
         if (value) {
@@ -89,6 +102,7 @@ export default class CourseOutlineStatus extends React.Component {
 
       this.setState({
         completedCourseBestPracticesChecks,
+        totalCourseBestPracticesChecks: filteredCourseBestPracticesChecks.length,
       });
     }
   }
@@ -251,6 +265,7 @@ CourseOutlineStatus.propTypes = {
       total_visible: PropTypes.number,
       total_number: PropTypes.number,
       highlights_enabled: PropTypes.bool,
+      highlights_active_for_course: PropTypes.bool,
     }),
     subsections: PropTypes.object,
     units: PropTypes.object,
@@ -273,6 +288,7 @@ CourseOutlineStatus.propTypes = {
       has_update: PropTypes.bool,
     }),
     certificates: PropTypes.shape({
+      is_enabled: PropTypes.bool,
       is_activated: PropTypes.bool,
       has_certificate: PropTypes.bool,
     }),

diff --git a/src/utils/CourseChecklist/courseChecklistValidators.js b/src/utils/CourseChecklist/courseChecklistValidators.js
@@ -64,7 +64,7 @@ export const hasDiverseSequences = (subsections) => {
 };
 
 export const hasWeeklyHighlights = sections => (
-  sections.highlights_enabled
+  sections.highlights_active_for_course && sections.highlights_enabled
 );
 
 export const hasShortUnitDepth = units => (

diff --git a/src/utils/CourseChecklist/courseChecklistValidators.test.js b/src/utils/CourseChecklist/courseChecklistValidators.test.js
@@ -243,11 +243,21 @@ describe('courseCheckValidators utility functions', () => {
 
   describe('hasWeeklyHighlights', () => {
     it('returns true when course run has highlights enabled', () => {
-      expect(validators.hasWeeklyHighlights({ highlights_enabled: true })).toEqual(true);
+      const data = { highlights_active_for_course: true, highlights_enabled: true };
+      expect(validators.hasWeeklyHighlights(data)).toEqual(true);
     });
 
     it('returns false when course run has highlights enabled', () => {
-      expect(validators.hasWeeklyHighlights({ highlights_enabled: false })).toEqual(false);
+      const data = { highlights_active_for_course: false, highlights_enabled: false };
+      expect(validators.hasWeeklyHighlights(data)).toEqual(false);
+
+      data.highlights_enabled = true;
+      data.highlights_active_for_course = false;
+      expect(validators.hasWeeklyHighlights(data)).toEqual(false);
+
+      data.highlights_enabled = false;
+      data.highlights_active_for_course = true;
+      expect(validators.hasWeeklyHighlights(data)).toEqual(false);
     });
   });
 

diff --git a/src/utils/CourseChecklist/getFilteredChecklist.js b/src/utils/CourseChecklist/getFilteredChecklist.js
@@ -1,19 +1,26 @@
 import { filters } from './courseChecklistData';
 
-const getFilteredChecklist = (checklist, isSelfPaced) => {
-  let healthChecks;
+const getFilteredChecklist = (
+  checklist, isSelfPaced, hasCertificatesEnabled, hasHighlightsEnabled) => {
+  let filteredCheckList;
 
   if (isSelfPaced) {
-    healthChecks =
+    filteredCheckList =
       checklist.filter(data => data.pacingTypeFilter === filters.ALL ||
         data.pacingTypeFilter === filters.SELF_PACED);
   } else {
-    healthChecks =
+    filteredCheckList =
       checklist.filter(data => data.pacingTypeFilter === filters.ALL ||
         data.pacingTypeFilter === filters.INSTRUCTOR_PACED);
   }
 
-  return healthChecks;
+  filteredCheckList = filteredCheckList.filter(data => data.id !== 'certificate' ||
+    hasCertificatesEnabled);
+
+  filteredCheckList = filteredCheckList.filter(data => data.id !== 'weeklyHighlights' ||
+    hasHighlightsEnabled);
+
+  return filteredCheckList;
 };
 
 export default getFilteredChecklist;
diff --git a/src/utils/CourseChecklist/getFilteredChecklist.test.js b/src/utils/CourseChecklist/getFilteredChecklist.test.js
@@ -3,26 +3,43 @@ import getFilteredChecklist from './getFilteredChecklist';
 
 const checklist = [
   {
+    id: 'welcomeMessage',
     pacingTypeFilter: filters.ALL,
   },
   {
-    pacingTypeFilter: filters.SELF_PACED,
+    id: 'gradingPolicy',
+    pacingTypeFilter: filters.ALL,
   },
   {
-    pacingTypeFilter: filters.INSTRUCTOR_PACED,
+    id: 'certificate',
+    pacingTypeFilter: filters.ALL,
   },
   {
+    id: 'courseDates',
     pacingTypeFilter: filters.ALL,
   },
   {
+    id: 'assignmentDeadlines',
+    pacingTypeFilter: filters.INSTRUCTOR_PACED,
+  },
+  {
+    id: 'weeklyHighlights',
     pacingTypeFilter: filters.SELF_PACED,
   },
 ];
 
 
+let courseData;
 describe('getFilteredChecklist utility function', () => {
+  beforeEach(() => {
+    courseData = {
+      isSelfPaced: true,
+      hasCertificatesEnabled: true,
+      hasHighlightsEnabled: true };
+  });
   it('returns only checklist items with filters ALL and SELF_PACED when isSelfPaced is true', () => {
-    const filteredChecklist = getFilteredChecklist(checklist, true);
+    const filteredChecklist = getFilteredChecklist(checklist,
+      courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
 
     filteredChecklist.forEach(((
       item => expect(item.pacingTypeFilter === filters.ALL ||
@@ -36,7 +53,9 @@ describe('getFilteredChecklist utility function', () => {
   });
 
   it('returns only checklist items with filters ALL and INSTRUCTOR_PACED when isSelfPaced is false', () => {
-    const filteredChecklist = getFilteredChecklist(checklist, false);
+    courseData.isSelfPaced = false;
+    const filteredChecklist = getFilteredChecklist(checklist,
+      courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
 
     filteredChecklist.forEach(((
       item => expect(item.pacingTypeFilter === filters.ALL ||
@@ -49,4 +68,26 @@ describe('getFilteredChecklist utility function', () => {
       .filter(item => item.pacingTypeFilter === filters.INSTRUCTOR_PACED).length)
       .toEqual(checklist.filter(item => item.pacingTypeFilter === filters.INSTRUCTOR_PACED).length);
   });
+
+  it('excludes certificates when they are disabled', () => {
+    let filteredChecklist = getFilteredChecklist(checklist,
+      courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
+    expect(checklist.filter(item => item.id === 'certificate').length).toEqual(1);
+
+    courseData.hasCertificatesEnabled = false;
+    filteredChecklist = getFilteredChecklist(checklist,
+      courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
+    expect(filteredChecklist.filter(item => item.id === 'certificate').length).toEqual(0);
+  });
+
+  it('excludes weekly highlights when they are disabled', () => {
+    let filteredChecklist = getFilteredChecklist(checklist,
+      courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
+    expect(filteredChecklist.filter(item => item.id === 'weeklyHighlights').length).toEqual(1);
+
+    courseData.hasHighlightsEnabled = false;
+    filteredChecklist = getFilteredChecklist(checklist,
+      courseData.isSelfPaced, courseData.hasCertificatesEnabled, courseData.hasHighlightsEnabled);
+    expect(filteredChecklist.filter(item => item.id === 'weeklyHighlights').length).toEqual(0);
+  });
 });