Achievement comments refactor to support avenger flow (#2252)

* major refactor to achievement feedback page

* fix formatting issues

* revert linting rules

* use typed selector

* fix boolean condition on crid

Co-authored-by: Martin Henz <henz@comp.nus.edu.sg>
Co-authored-by: En Rong <53928333+chownces@users.noreply.github.com>

Author: 3 people

src/commons/achievement/AchievementCommentCard.tsx

@@ -2,15 +2,18 @@ import { useMemo } from 'react';
 
 import { Assessment } from '../assessment/AssessmentTypes';
 import { history } from '../utils/HistoryHelper';
+import { useTypedSelector } from '../utils/Hooks';
 import { showWarningMessage } from '../utils/NotificationsHelper';
 import { assessmentTypeLink } from '../utils/ParamParseHelper';
 
-export type OwnProps = {
+const AchievementCommentCard = ({
+  assessment,
+  showToQuestion
+}: {
   assessment: Assessment;
-  courseId?: number;
-};
-
-const AchievementCommentCard = ({ assessment, courseId }: OwnProps) => {
+  showToQuestion: boolean;
+}) => {
+  const courseId = useTypedSelector(store => store.session.courseId);
   const toMission = useMemo(
     () => (questionId: number) => {
       if (!courseId) {
@@ -37,13 +40,15 @@ const AchievementCommentCard = ({ assessment, courseId }: OwnProps) => {
               </span>
 
               <div className="box-comment">
-                <p>{question.comments === null ? 'Not Graded' : question.comments}</p>
+                <p>{question.comments === null ? 'No Comments' : question.comments}</p>
                 <p className="xp">{'XP: ' + question.xp + '/' + question.maxXp}</p>
               </div>
 
-              <button className="to-assessment-button" onClick={() => toMission(index)}>
-                {'To Question'}
-              </button>
+              {showToQuestion && (
+                <button className="to-assessment-button" onClick={() => toMission(index)}>
+                  {'To Question'}
+                </button>
+              )}
             </div>
           ))}
       </div>

src/commons/achievement/AchievementOverview.tsx

@@ -1,12 +1,31 @@
+import { useEffect } from 'react';
+import { useDispatch } from 'react-redux';
+import { AchievementUser } from 'src/features/achievement/AchievementTypes';
+
+import { FETCH_TOTAL_XP, FETCH_TOTAL_XP_ADMIN } from '../application/types/SessionTypes';
 import { useTypedSelector } from '../utils/Hooks';
 import AchievementLevel from './overview/AchievementLevel';
 
 type AchievementOverviewProps = {
   name: string;
+  userState: [AchievementUser | undefined, any];
 };
 
 function AchievementOverview(props: AchievementOverviewProps) {
-  const { name } = props;
+  const { name, userState } = props;
+  const [selectedUser] = userState;
+  const crid = selectedUser?.courseRegId;
+  const userCrid = useTypedSelector(store => store.session.courseRegId);
+
+  const dispatch = useDispatch();
+  useEffect(() => {
+    // If user is student, fetch assessment details from assessment route instead, as seen below
+    if (crid && crid !== userCrid) {
+      dispatch({ type: FETCH_TOTAL_XP_ADMIN, payload: crid });
+    } else {
+      dispatch({ type: FETCH_TOTAL_XP });
+    }
+  }, [crid, userCrid, dispatch]);
 
   const studentXp = useTypedSelector(store => store.session.xp);
 

src/commons/achievement/AchievementView.tsx

@@ -8,9 +8,10 @@ import {
   getAbilityBackground,
   getAbilityGlow
 } from '../../features/achievement/AchievementConstants';
-import { AchievementStatus } from '../../features/achievement/AchievementTypes';
-import { FETCH_ASSESSMENT } from '../application/types/SessionTypes';
-import { Assessment, FETCH_ASSESSMENT_OVERVIEWS } from '../assessment/AssessmentTypes';
+import { AchievementStatus, AchievementUser } from '../../features/achievement/AchievementTypes';
+import { FETCH_ASSESSMENT, FETCH_ASSESSMENT_ADMIN } from '../application/types/SessionTypes';
+import { FETCH_ASSESSMENT_OVERVIEWS } from '../assessment/AssessmentTypes';
+import { Assessment } from '../assessment/AssessmentTypes';
 import { useTypedSelector } from '../utils/Hooks';
 import AchievementCommentCard from './AchievementCommentCard';
 import { prettifyDate } from './utils/DateHelper';
@@ -19,25 +20,41 @@ import AchievementViewGoal from './view/AchievementViewGoal';
 
 type AchievementViewProps = {
   focusUuid: string;
-  courseRegId?: number;
+  assessments?: Map<number, Assessment>;
+  userState?: [AchievementUser | undefined, any];
 };
 
-function AchievementView({ focusUuid, courseRegId }: AchievementViewProps) {
+function AchievementView({ focusUuid, userState }: AchievementViewProps) {
+  const assessmentId = !Number.isNaN(+focusUuid) && +focusUuid !== 0 ? +focusUuid : undefined;
+  let courseRegId: number | undefined;
+
+  if (userState) {
+    const [selectedUser] = userState!;
+    courseRegId = selectedUser?.courseRegId;
+  }
+  const userCrid = useTypedSelector(store => store.session.courseRegId);
+  const isAdminView: boolean = courseRegId !== undefined && courseRegId !== userCrid;
+
   const dispatch = useDispatch();
   useEffect(() => {
-    if (!Number.isNaN(+focusUuid) && +focusUuid !== 0) {
-      dispatch({ type: FETCH_ASSESSMENT, payload: +focusUuid });
-      dispatch({ type: FETCH_ASSESSMENT_OVERVIEWS });
+    dispatch({ type: FETCH_ASSESSMENT_OVERVIEWS });
+    if (!assessmentId) {
+      return;
     }
-  }, [focusUuid, courseRegId, dispatch]);
+    if (isAdminView) {
+      // Fetch selected user's assessment from admin route
+      dispatch({ type: FETCH_ASSESSMENT_ADMIN, payload: { assessmentId, courseRegId } });
+    } else {
+      // If user is student, fetch assessment details from assessment route instead, as seen below
+      dispatch({ type: FETCH_ASSESSMENT, payload: assessmentId });
+    }
+  }, [dispatch, assessmentId, courseRegId, isAdminView]);
 
   const inferencer = useContext(AchievementContext);
-  const courseId = useTypedSelector(store => store.session.courseId);
-
   const assessments = useTypedSelector(store => store.session.assessments);
+  const selectedAssessment: Assessment | undefined = assessments.get(assessmentId!);
   const allAssessmentConfigs = useTypedSelector(store => store.session.assessmentOverviews) ?? [];
-  const selectedAssessment: Assessment | undefined = assessments.get(+focusUuid);
-  const selectedAssessmentConfig = allAssessmentConfigs.find(config => config.id === +focusUuid);
+  const selectedAssessmentConfig = allAssessmentConfigs.find(config => config.id === assessmentId);
 
   if (focusUuid === '') {
     return (
@@ -47,7 +64,6 @@ function AchievementView({ focusUuid, courseRegId }: AchievementViewProps) {
       </div>
     );
   }
-
   const achievement = inferencer.getAchievement(focusUuid);
   const { deadline, title, view } = achievement;
   const { coverImage, completionText, description } = view;
@@ -82,7 +98,9 @@ function AchievementView({ focusUuid, courseRegId }: AchievementViewProps) {
         selectedAssessment &&
         selectedAssessmentConfig &&
         selectedAssessmentConfig.isManuallyGraded && (
-          <AchievementCommentCard courseId={courseId} assessment={selectedAssessment} />
+          // TODO: showToQuestion is currently used to disable the goto question button for admins,
+          // as it has not been integrated with the grading view yet
+          <AchievementCommentCard assessment={selectedAssessment} showToQuestion={!isAdminView} />
         )}
 
       {goals.length > 0 && (

src/commons/application/actions/SessionActions.ts

@@ -22,15 +22,17 @@ import {
   DELETE_USER_COURSE_REGISTRATION,
   FETCH_ADMIN_PANEL_COURSE_REGISTRATIONS,
   FETCH_ASSESSMENT,
+  FETCH_ASSESSMENT_ADMIN,
   FETCH_ASSESSMENT_CONFIGS,
   FETCH_ASSESSMENT_OVERVIEWS,
   FETCH_AUTH,
   FETCH_COURSE_CONFIG,
   FETCH_GRADING,
   FETCH_GRADING_OVERVIEWS,
   FETCH_NOTIFICATIONS,
+  FETCH_TOTAL_XP,
+  FETCH_TOTAL_XP_ADMIN,
   FETCH_USER_AND_COURSE,
-  GET_TOTAL_XP,
   LOGIN,
   LOGIN_GITHUB,
   LOGOUT_GITHUB,
@@ -77,11 +79,16 @@ export const fetchUserAndCourse = () => action(FETCH_USER_AND_COURSE);
 
 export const fetchCourseConfig = () => action(FETCH_COURSE_CONFIG);
 
-export const fetchAssessment = (id: number) => action(FETCH_ASSESSMENT, id);
+export const fetchAssessment = (assessmentId: number) => action(FETCH_ASSESSMENT, assessmentId);
+
+export const fetchAssessmentAdmin = (assessmentId: number, courseRegId: number) =>
+  action(FETCH_ASSESSMENT_ADMIN, { assessmentId, courseRegId });
 
 export const fetchAssessmentOverviews = () => action(FETCH_ASSESSMENT_OVERVIEWS);
 
-export const getTotalXp = () => action(GET_TOTAL_XP);
+export const fetchTotalXp = () => action(FETCH_TOTAL_XP);
+
+export const fetchTotalXpAdmin = (courseRegId: number) => action(FETCH_TOTAL_XP_ADMIN, courseRegId);
 
 export const fetchGrading = (submissionId: number) => action(FETCH_GRADING, submissionId);
 

src/commons/application/types/SessionTypes.ts

@@ -16,8 +16,10 @@ export const FETCH_AUTH = 'FETCH_AUTH';
 export const FETCH_USER_AND_COURSE = 'FETCH_USER_AND_COURSE';
 export const FETCH_COURSE_CONFIG = 'FETCH_COURSE_CONFIG';
 export const FETCH_ASSESSMENT = 'FETCH_ASSESSMENT';
+export const FETCH_ASSESSMENT_ADMIN = 'FETCH_ASSESSMENT_ADMIN';
 export const FETCH_ASSESSMENT_OVERVIEWS = 'FETCH_ASSESSMENT_OVERVIEWS';
-export const GET_TOTAL_XP = 'GET_TOTAL_XP';
+export const FETCH_TOTAL_XP = 'FETCH_TOTAL_XP';
+export const FETCH_TOTAL_XP_ADMIN = 'FETCH_TOTAL_XP_ADMIN';
 export const FETCH_GRADING = 'FETCH_GRADING';
 export const FETCH_GRADING_OVERVIEWS = 'FETCH_GRADING_OVERVIEWS';
 export const LOGIN = 'LOGIN';

src/commons/profile/ProfileContainer.ts

@@ -1,7 +1,7 @@
 import { connect, MapDispatchToProps, MapStateToProps } from 'react-redux';
 import { bindActionCreators, Dispatch } from 'redux';
 
-import { fetchAssessmentOverviews, getTotalXp } from '../application/actions/SessionActions';
+import { fetchAssessmentOverviews, fetchTotalXp } from '../application/actions/SessionActions';
 import { OverallState } from '../application/ApplicationTypes';
 import Profile, { DispatchProps, StateProps } from './Profile';
 
@@ -18,7 +18,7 @@ const mapDispatchToProps: MapDispatchToProps<DispatchProps, {}> = (dispatch: Dis
   bindActionCreators(
     {
       handleAssessmentOverviewFetch: fetchAssessmentOverviews,
-      handleTotalXpFetch: getTotalXp
+      handleTotalXpFetch: fetchTotalXp
     },
     dispatch
   );

src/commons/sagas/BackendSaga.ts

@@ -46,14 +46,16 @@ import {
   DELETE_USER_COURSE_REGISTRATION,
   FETCH_ADMIN_PANEL_COURSE_REGISTRATIONS,
   FETCH_ASSESSMENT,
+  FETCH_ASSESSMENT_ADMIN,
   FETCH_ASSESSMENT_CONFIGS,
   FETCH_AUTH,
   FETCH_COURSE_CONFIG,
   FETCH_GRADING,
   FETCH_GRADING_OVERVIEWS,
   FETCH_NOTIFICATIONS,
+  FETCH_TOTAL_XP,
+  FETCH_TOTAL_XP_ADMIN,
   FETCH_USER_AND_COURSE,
-  GET_TOTAL_XP,
   REAUTOGRADE_ANSWER,
   REAUTOGRADE_SUBMISSION,
   SUBMIT_ANSWER,
@@ -235,7 +237,7 @@ function* BackendSaga(): SagaIterator {
     }
   });
 
-  yield takeEvery(GET_TOTAL_XP, function* () {
+  yield takeEvery(FETCH_TOTAL_XP, function* () {
     const tokens: Tokens = yield selectTokens();
 
     const res: { totalXp: number } = yield call(getTotalXp, tokens);
@@ -244,17 +246,50 @@ function* BackendSaga(): SagaIterator {
     }
   });
 
+  yield takeEvery(
+    FETCH_TOTAL_XP_ADMIN,
+    function* (action: ReturnType<typeof actions.fetchTotalXpAdmin>) {
+      const tokens: Tokens = yield selectTokens();
+
+      const courseRegId = action.payload;
+
+      const res: { totalXp: number } = yield call(getTotalXp, tokens, courseRegId);
+      if (res) {
+        yield put(actions.updateTotalXp(res.totalXp));
+      }
+    }
+  );
+
   yield takeEvery(FETCH_ASSESSMENT, function* (action: ReturnType<typeof actions.fetchAssessment>) {
     const tokens: Tokens = yield selectTokens();
 
-    const id = action.payload;
+    const assessmentId = action.payload;
 
-    const assessment: Assessment | null = yield call(getAssessment, id, tokens);
+    const assessment: Assessment | null = yield call(getAssessment, assessmentId, tokens);
     if (assessment) {
       yield put(actions.updateAssessment(assessment));
     }
   });
 
+  yield takeEvery(
+    FETCH_ASSESSMENT_ADMIN,
+    function* (action: ReturnType<typeof actions.fetchAssessmentAdmin>) {
+      const tokens: Tokens = yield selectTokens();
+
+      const { assessmentId, courseRegId } = action.payload;
+
+      const assessment: Assessment | null = yield call(
+        getAssessment,
+        assessmentId,
+        tokens,
+        courseRegId
+      );
+      if (assessment) {
+        yield put(actions.updateAssessment(assessment));
+      }
+    }
+  );
+
   yield takeEvery(SUBMIT_ANSWER, function* (action: ReturnType<typeof actions.submitAnswer>): any {
     const tokens: Tokens = yield selectTokens();
     const questionId = action.payload.id;

src/commons/sagas/RequestsSaga.ts

@@ -468,11 +468,22 @@ export const getAssessmentOverviews = async (
 /**
  * GET /courses/{courseId}/user/total_xp
  */
-export const getTotalXp = async (tokens: Tokens): Promise<number | null> => {
-  const resp = await request(`${courseId()}/user/total_xp`, 'GET', {
-    ...tokens,
-    shouldRefresh: true
-  });
+export const getTotalXp = async (tokens: Tokens, courseRegId?: number): Promise<number | null> => {
+  let resp;
+  if (courseRegId !== undefined) {
+    // If courseRegId is provided, get the total XP of a specific student
+    resp = await request(`${courseId()}/admin/users/${courseRegId}/total_xp`, 'GET', {
+      ...tokens,
+      shouldRefresh: true
+    });
+  } else {
+    // Otherwise, get the total XP of the current user
+    resp = await request(`${courseId()}/user/total_xp`, 'GET', {
+      ...tokens,
+      shouldRefresh: true
+    });
+  }
+
   if (!resp || !resp.ok) {
     return null; // invalid accessToken _and_ refreshToken
   }
@@ -514,12 +525,30 @@ export const getUserAssessmentOverviews = async (
  * Note: if assessment is password-protected, a corresponding unlock request will be sent to
  * POST /courses/{courseId}/assessments/{assessmentId}/unlock
  */
-export const getAssessment = async (id: number, tokens: Tokens): Promise<Assessment | null> => {
-  let resp = await request(`${courseId()}/assessments/${id}`, 'GET', {
-    ...tokens,
-    shouldAutoLogout: false,
-    shouldRefresh: true
-  });
+export const getAssessment = async (
+  assessmentId: number,
+  tokens: Tokens,
+  courseRegId?: number
+): Promise<Assessment | null> => {
+  let resp;
+  if (courseRegId !== undefined) {
+    // If courseRegId is provided, we are getting the assessment for another user as an admin
+    resp = await request(
+      `${courseId()}/admin/users/${courseRegId}/assessments/${assessmentId}`,
+      'GET',
+      {
+        ...tokens,
+        shouldRefresh: true
+      }
+    );
+  } else {
+    // Otherwise, we are getting the assessment for the current user
+    resp = await request(`${courseId()}/assessments/${assessmentId}`, 'GET', {
+      ...tokens,
+      shouldAutoLogout: false,
+      shouldRefresh: true
+    });
+  }
 
   // Attempt to load password-protected assessment
   while (resp && resp.status === 403) {
@@ -530,7 +559,7 @@ export const getAssessment = async (id: number, tokens: Tokens): Promise<Assessm
       return null;
     }
 
-    resp = await request(`${courseId()}/assessments/${id}/unlock`, 'POST', {
+    resp = await request(`${courseId()}/assessments/${assessmentId}/unlock`, 'POST', {
       ...tokens,
       body: {
         password: input