Address review comments

chownces · chownces · commit 19bf45ca427d · 2021-08-08T01:06:32.000+08:00
diff --git a/src/commons/application/Application.tsx b/src/commons/application/Application.tsx
@@ -203,14 +203,6 @@ const Application: React.FC<ApplicationProps> = props => {
             />
             <Route path="/playground" render={ensureUserAndRoleAndRouteTo(props, <Playground />)} />
 
-            {/* <Route
-              path="/sourcecast/:sourcecastId?"
-              render={ensureUserAndRoleAndRouteTo(props, <Sourcecast />)}
-            />
-            <Route
-              path="/achievements"
-              render={ensureUserAndRoleAndRouteTo(props, <Achievement />)}
-            /> */}
             <Redirect
               from="/"
               exact={true}
diff --git a/src/commons/assessment/__tests__/Assessment.tsx b/src/commons/assessment/__tests__/Assessment.tsx
@@ -8,6 +8,7 @@ import { mockRouterProps } from '../../mocks/ComponentMocks';
 import Assessment, { AssessmentProps } from '../Assessment';
 
 const defaultProps: AssessmentProps = {
+  courseId: 1,
   assessmentConfiguration: {
     assessmentConfigId: 1,
     type: 'Missions',
diff --git a/src/commons/assessment/__tests__/__snapshots__/Assessment.tsx.snap b/src/commons/assessment/__tests__/__snapshots__/Assessment.tsx.snap
diff --git a/src/commons/navigationBar/subcomponents/AcademyNavigationBar.tsx b/src/commons/navigationBar/subcomponents/AcademyNavigationBar.tsx
@@ -24,7 +24,7 @@ const AcademyNavigationBar: React.FunctionComponent<OwnProps> = props => (
           to={`/courses/${props.courseId}/${assessmentTypeLink(assessmentType)}`}
           activeClassName={Classes.ACTIVE}
           className={classNames('NavigationBar__link', Classes.BUTTON, Classes.MINIMAL)}
-          key={assessmentType}
+          key={idx}
         >
           <Icon icon={icons[idx]} />
           <div className="navbar-button-text hidden-xs hidden-sm">{assessmentType}</div>
diff --git a/src/commons/navigationBar/subcomponents/NavigationBarMobileSideMenu.tsx b/src/commons/navigationBar/subcomponents/NavigationBarMobileSideMenu.tsx
@@ -50,7 +50,7 @@ const NavigationBarMobileSideMenu: React.FC<NavigationBarMobileSideMenuProps> =
             Classes.LARGE
           )}
           onClick={props.onClose}
-          key={assessmentType}
+          key={idx}
         >
           <Icon icon={icons[idx]} />
           <div>{assessmentType}</div>
diff --git a/src/commons/navigationBar/subcomponents/__tests__/NavigationBarMobileSideMenu.tsx b/src/commons/navigationBar/subcomponents/__tests__/NavigationBarMobileSideMenu.tsx
@@ -9,7 +9,8 @@ test('NavigationBarMobileSideMenu renders "Not logged in" correctly', () => {
     onClose: () => {},
     handleGitHubLogIn: () => {},
     handleGitHubLogOut: () => {},
-    assessmentTypes: []
+    assessmentTypes: [],
+    courseId: 1
   };
   const tree = shallow(<NavigationBarMobileSideMenu {...props} />);
   expect(tree.debug()).toMatchSnapshot();
@@ -22,7 +23,8 @@ test('NavigationBarMobileSideMenu renders correctly when logged in (no course se
     name: 'Avenger',
     handleGitHubLogIn: () => {},
     handleGitHubLogOut: () => {},
-    assessmentTypes: []
+    assessmentTypes: [],
+    courseId: 1
   };
   const tree = shallow(<NavigationBarMobileSideMenu {...props} />);
   expect(tree.debug()).toMatchSnapshot();
@@ -36,7 +38,8 @@ test('NavigationBarMobileSideMenu renders correctly when logged in (with course
     role: Role.Student,
     handleGitHubLogIn: () => {},
     handleGitHubLogOut: () => {},
-    assessmentTypes: ['Missions', 'Quests', 'Paths', 'Contests', 'Others']
+    assessmentTypes: ['Missions', 'Quests', 'Paths', 'Contests', 'Others'],
+    courseId: 1
   };
   const tree = shallow(<NavigationBarMobileSideMenu {...props} />);
   expect(tree.debug()).toMatchSnapshot();
diff --git a/src/commons/navigationBar/subcomponents/__tests__/__snapshots__/NavigationBarMobileSideMenu.tsx.snap b/src/commons/navigationBar/subcomponents/__tests__/__snapshots__/NavigationBarMobileSideMenu.tsx.snap
@@ -30,35 +30,35 @@ exports[`NavigationBarMobileSideMenu renders correctly when logged in (no course
 
 exports[`NavigationBarMobileSideMenu renders correctly when logged in (with course selected) 1`] = `
 "<Blueprint3.Drawer isOpen={true} position=\\"left\\" onClose={[Function: onClose]} title=\\"\\" className=\\"bp3-dark\\" canOutsideClickClose={true} shouldReturnFocusOnClose={true} style={{...}} vertical={false}>
-  <NavLink to=\\"/courses/undefined/missions\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
+  <NavLink to=\\"/courses/1/missions\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
     <Blueprint3.Icon icon=\\"flame\\" />
     <div>
       Missions
     </div>
     <Connect(NotificationBadge) notificationFilter={[Function (anonymous)]} disableHover={true} />
   </NavLink>
-  <NavLink to=\\"/courses/undefined/quests\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
+  <NavLink to=\\"/courses/1/quests\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
     <Blueprint3.Icon icon=\\"lightbulb\\" />
     <div>
       Quests
     </div>
     <Connect(NotificationBadge) notificationFilter={[Function (anonymous)]} disableHover={true} />
   </NavLink>
-  <NavLink to=\\"/courses/undefined/paths\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
+  <NavLink to=\\"/courses/1/paths\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
     <Blueprint3.Icon icon=\\"predictive-analysis\\" />
     <div>
       Paths
     </div>
     <Connect(NotificationBadge) notificationFilter={[Function (anonymous)]} disableHover={true} />
   </NavLink>
-  <NavLink to=\\"/courses/undefined/contests\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
+  <NavLink to=\\"/courses/1/contests\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
     <Blueprint3.Icon icon=\\"comparison\\" />
     <div>
       Contests
     </div>
     <Connect(NotificationBadge) notificationFilter={[Function (anonymous)]} disableHover={true} />
   </NavLink>
-  <NavLink to=\\"/courses/undefined/others\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
+  <NavLink to=\\"/courses/1/others\\" activeClassName=\\"bp3-active\\" className=\\"NavigationBar__link__mobile bp3-button bp3-minimal bp3-large\\" onClick={[Function: onClose]}>
     <Blueprint3.Icon icon=\\"manual\\" />
     <div>
       Others
diff --git a/src/commons/profile/__tests__/Profile.tsx b/src/commons/profile/__tests__/Profile.tsx
@@ -28,6 +28,7 @@ test('Profile renders correctly when there are no closed assessments', () => {
   const props = {
     name: 'yeet',
     role: Role.Student,
+    courseId: 1,
     assessmentOverviews: mockNoClosedAssessmentOverviews,
     assessmentTypes: ['Missions', 'Quests', 'Paths', 'Contests', 'Others'],
     isOpen: true,
@@ -56,6 +57,7 @@ test('Profile renders correctly when there are closed and graded, or closed and
   const props = {
     name: 'yeeet',
     role: Role.Staff,
+    courseId: 1,
     assessmentOverviews: mockAssessmentOverviews,
     assessmentTypes: ['Missions', 'Quests', 'Paths', 'Contests', 'Others'],
     isOpen: true,
diff --git a/src/commons/profile/__tests__/__snapshots__/Profile.tsx.snap b/src/commons/profile/__tests__/__snapshots__/Profile.tsx.snap
@@ -3,7 +3,7 @@
 exports[`Profile renders correctly when there are closed and graded, or closed and not manually graded assessments 1`] = `
 "<MemoryRouter initialEntries={{...}}>
   <Router history={{...}}>
-    <Profile name=\\"yeeet\\" role=\\"staff\\" assessmentOverviews={{...}} assessmentTypes={{...}} isOpen={true} handleAssessmentOverviewFetch={[Function: handleAssessmentOverviewFetch]} onClose={[Function: onClose]}>
+    <Profile name=\\"yeeet\\" role=\\"staff\\" courseId={1} assessmentOverviews={{...}} assessmentTypes={{...}} isOpen={true} handleAssessmentOverviewFetch={[Function: handleAssessmentOverviewFetch]} onClose={[Function: onClose]}>
       <Blueprint3.Drawer className=\\"profile\\" icon=\\"user\\" isCloseButtonShown={true} isOpen={true} onClose={[Function: onClose]} title=\\"User Profile\\" position=\\"left\\" size=\\"360px\\" canOutsideClickClose={true} shouldReturnFocusOnClose={true} style={{...}} vertical={false}>
         <Blueprint3.Overlay className=\\"bp3-overlay-container\\" icon=\\"user\\" isCloseButtonShown={true} isOpen={true} onClose={[Function: onClose]} title=\\"User Profile\\" position=\\"left\\" size=\\"360px\\" canOutsideClickClose={true} shouldReturnFocusOnClose={true} style={{...}} vertical={false} onOpening={[Function (anonymous)]} onClosed={[Function (anonymous)]} autoFocus={true} backdropProps={{...}} canEscapeKeyClose={true} enforceFocus={true} hasBackdrop={true} lazy={true} transitionDuration={300} transitionName=\\"bp3-overlay\\" usePortal={true}>
           <Blueprint3.Portal className={[undefined]} container={{...}}>
@@ -90,11 +90,11 @@ exports[`Profile renders correctly when there are closed and graded, or closed a
                             </div>
                           </div>
                           <div className=\\"profile-callouts\\">
-                            <ProfileCard item={{...}} getFrac={[Function: getFrac]} parseColour={[Function: parseColour]} renderIcon={[Function: renderIcon]} courseId={[undefined]}>
-                              <NavLink className=\\"profile-summary-navlink\\" target=\\"_blank\\" to=\\"/courses/undefined/quests/5/0\\" activeClassName=\\"profile-summary-navlink\\">
+                            <ProfileCard item={{...}} getFrac={[Function: getFrac]} parseColour={[Function: parseColour]} renderIcon={[Function: renderIcon]} courseId={1}>
+                              <NavLink className=\\"profile-summary-navlink\\" target=\\"_blank\\" to=\\"/courses/1/quests/5/0\\" activeClassName=\\"profile-summary-navlink\\">
                                 <Link aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} to={{...}} target=\\"_blank\\">
-                                  <LinkAnchor aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/undefined/quests/5/0\\" navigate={[Function: navigate]}>
-                                    <a aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/undefined/quests/5/0\\" onClick={[Function: onClick]}>
+                                  <LinkAnchor aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/1/quests/5/0\\" navigate={[Function: navigate]}>
+                                    <a aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/1/quests/5/0\\" onClick={[Function: onClick]}>
                                       <Blueprint3.Callout className=\\"profile-summary-callout\\" icon=\\"lightbulb\\" title=\\"Closed (fully graded) Sidequest\\">
                                         <div className=\\"bp3-callout bp3-callout-icon profile-summary-callout\\">
                                           <Blueprint3.Icon icon=\\"lightbulb\\" size={20}>
@@ -134,11 +134,11 @@ exports[`Profile renders correctly when there are closed and graded, or closed a
                                 </Link>
                               </NavLink>
                             </ProfileCard>
-                            <ProfileCard item={{...}} getFrac={[Function: getFrac]} parseColour={[Function: parseColour]} renderIcon={[Function: renderIcon]} courseId={[undefined]}>
-                              <NavLink className=\\"profile-summary-navlink\\" target=\\"_blank\\" to=\\"/courses/undefined/quests/5/0\\" activeClassName=\\"profile-summary-navlink\\">
+                            <ProfileCard item={{...}} getFrac={[Function: getFrac]} parseColour={[Function: parseColour]} renderIcon={[Function: renderIcon]} courseId={1}>
+                              <NavLink className=\\"profile-summary-navlink\\" target=\\"_blank\\" to=\\"/courses/1/quests/5/0\\" activeClassName=\\"profile-summary-navlink\\">
                                 <Link aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} to={{...}} target=\\"_blank\\">
-                                  <LinkAnchor aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/undefined/quests/5/0\\" navigate={[Function: navigate]}>
-                                    <a aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/undefined/quests/5/0\\" onClick={[Function: onClick]}>
+                                  <LinkAnchor aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/1/quests/5/0\\" navigate={[Function: navigate]}>
+                                    <a aria-current={{...}} className=\\"profile-summary-navlink\\" style={[undefined]} target=\\"_blank\\" href=\\"/courses/1/quests/5/0\\" onClick={[Function: onClick]}>
                                       <Blueprint3.Callout className=\\"profile-summary-callout\\" icon=\\"lightbulb\\" title=\\"Ungraded assessment\\">
                                         <div className=\\"bp3-callout bp3-callout-icon profile-summary-callout\\">
                                           <Blueprint3.Icon icon=\\"lightbulb\\" size={20}>
@@ -197,7 +197,7 @@ exports[`Profile renders correctly when there are closed and graded, or closed a
 exports[`Profile renders correctly when there are no closed assessments 1`] = `
 "<MemoryRouter initialEntries={{...}}>
   <Router history={{...}}>
-    <Profile name=\\"yeet\\" role=\\"student\\" assessmentOverviews={{...}} assessmentTypes={{...}} isOpen={true} handleAssessmentOverviewFetch={[Function: handleAssessmentOverviewFetch]} onClose={[Function: onClose]}>
+    <Profile name=\\"yeet\\" role=\\"student\\" courseId={1} assessmentOverviews={{...}} assessmentTypes={{...}} isOpen={true} handleAssessmentOverviewFetch={[Function: handleAssessmentOverviewFetch]} onClose={[Function: onClose]}>
       <Blueprint3.Drawer className=\\"profile\\" icon=\\"user\\" isCloseButtonShown={true} isOpen={true} onClose={[Function: onClose]} title=\\"User Profile\\" position=\\"left\\" size=\\"360px\\" canOutsideClickClose={true} shouldReturnFocusOnClose={true} style={{...}} vertical={false}>
         <Blueprint3.Overlay className=\\"bp3-overlay-container\\" icon=\\"user\\" isCloseButtonShown={true} isOpen={true} onClose={[Function: onClose]} title=\\"User Profile\\" position=\\"left\\" size=\\"360px\\" canOutsideClickClose={true} shouldReturnFocusOnClose={true} style={{...}} vertical={false} onOpening={[Function (anonymous)]} onClosed={[Function (anonymous)]} autoFocus={true} backdropProps={{...}} canEscapeKeyClose={true} enforceFocus={true} hasBackdrop={true} lazy={true} transitionDuration={300} transitionName=\\"bp3-overlay\\" usePortal={true}>
           <Blueprint3.Portal className={[undefined]} container={{...}}>
diff --git a/src/commons/sagas/BackendSaga.ts b/src/commons/sagas/BackendSaga.ts
@@ -170,8 +170,10 @@ function* BackendSaga(): SagaIterator {
       yield put(actions.setCourseRegistration(courseRegistration));
       yield put(actions.setCourseConfiguration(courseConfiguration));
       yield put(actions.setAssessmentConfigurations(assessmentConfigurations));
-      yield history.push(`/courses/${courseRegistration.courseId}`);
+      return yield history.push(`/courses/${courseRegistration.courseId}`);
     }
+
+    return yield history.push(`/welcome`);
   });
 
   yield takeEvery(
@@ -724,6 +726,16 @@ function* BackendSaga(): SagaIterator {
       return yield showWarningMessage('An error occurred. Please try again.');
     }
 
+    /**
+     * setUser updates the Dropdown course selection menu with the updated courses
+     *
+     * Setting the role here handles an edge case where a user creates his first ever course.
+     * Without it, the history.push below would not work as the /courses routes will not be rendered
+     * (see Application.tsx)
+     */
+    yield put(actions.setUser(user));
+    yield put(actions.setCourseRegistration({ role: Role.Student }));
+
     const placeholderAssessmentConfig = [
       {
         type: 'Missions',
diff --git a/src/commons/sagas/__tests__/BackendSaga.ts b/src/commons/sagas/__tests__/BackendSaga.ts
@@ -981,6 +981,8 @@ describe('Test CREATE_COURSE action', () => {
       .withState(mockStates)
       .call(postCreateCourse, mockTokens, courseConfig)
       .call(getUser, mockTokens)
+      .put(setUser(mockUser))
+      .put(setCourseRegistration({ role: Role.Student }))
       .call(
         putAssessmentConfigs,
         mockTokens,
@@ -1014,9 +1016,7 @@ describe('Test CREATE_COURSE action', () => {
       .not.call.fn(getUser)
       .not.put.actionType(SET_USER)
       .not.put.actionType(SET_COURSE_REGISTRATION)
-      .not.put.actionType(SET_COURSE_CONFIGURATION)
       .not.call.fn(putAssessmentConfigs)
-      .not.put.actionType(SET_ASSESSMENT_CONFIGURATIONS)
       .not.call.fn(showSuccessMessage)
       .not.call.fn(history.push)
       .provide([[call(postCreateCourse, mockTokens, courseConfig), errorResp]])

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ const AcademyNavigationBar: React.FunctionComponent<OwnProps> = props => (`
`24`	`24`	to={`/courses/${props.courseId}/${assessmentTypeLink(assessmentType)}`}
`25`	`25`	`activeClassName={Classes.ACTIVE}`
`26`	`26`	`className={classNames('NavigationBar__link', Classes.BUTTON, Classes.MINIMAL)}`
`27`		`- key={assessmentType}`
	`27`	`+ key={idx}`
`28`	`28`	`>`
`29`	`29`	`<Icon icon={icons[idx]} />`
`30`	`30`	`<div className="navbar-button-text hidden-xs hidden-sm">{assessmentType}</div>`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ const NavigationBarMobileSideMenu: React.FC<NavigationBarMobileSideMenuProps> =`
`50`	`50`	`Classes.LARGE`
`51`	`51`	`)}`
`52`	`52`	`onClick={props.onClose}`
`53`		`- key={assessmentType}`
	`53`	`+ key={idx}`
`54`	`54`	`>`
`55`	`55`	`<Icon icon={icons[idx]} />`
`56`	`56`	`<div>{assessmentType}</div>`