fix: Previous activity info showing in the Activity Level Feedback se…

…ction [PT-187021319] Previous activity info should not be displayed in the Activity Level Feedback section. This section should only display information related to the selected activity. Also fixes feedback given badges in these scenarios: - When scoring with a rubric, feedback given badge should not appear on the student until all rows of the rubric have a score in them. At that point, the feedback given badge should appear and the awaiting feedback count should update. - When No Score or Autoscore is selected, user can enter text feedback OR complete rubric to mark a student as Feedback Given. At which point, the awaiting feedback count should update. If Rubric score is selected, user must complete the rubric to mark a student as Feedback Given and update the awaiting feedback count. In this case, feedback text field can be left blank and student will still be marked feedback given (as long as rubric is complete). - When Manual Score is selected, the user must enter a score in the score field to mark a student as Feedback Given and the awaiting feedback count should update. In this case, feedback text field can be left blank and student will still be marked feedback given (as long as score field is complete).
concord-consortium · Mar 19, 2024 · 5342ada · 5342ada
1 parent 3268d0e
commit 5342ada
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 42 deletions.
diff --git a/cypress/integration/portal-dashboard/portal-dashboard-feedback.spec.js b/cypress/integration/portal-dashboard/portal-dashboard-feedback.spec.js
@@ -116,8 +116,17 @@ context("Portal Dashboard Feedback Panel", () => {
       it('rubric feedback can be given', () => {
         cy.get('[data-cy=rating-radio-button]').eq(0).click();
         cy.get('[data-cy=rating-radio-button] div').eq(0)
+        .should('have.css', 'background-color')
+        .and('eq', 'rgb(78, 161, 90)');
+        // selecting only 1 of the 2 criteria does not trigger feedback complete
+        cy.get('[data-cy=feedback-badge]').eq(2).find('circle')
+          .should('have.attr', 'fill')
+          .and('not.include', '#4EA15A');
+        cy.get('[data-cy=rating-radio-button]').eq(2).click();
+        cy.get('[data-cy=rating-radio-button] div').eq(2)
           .should('have.css', 'background-color')
           .and('eq', 'rgb(78, 161, 90)');
+        // selecting both criteria does trigger feedback complete
         cy.get('[data-cy=feedback-badge]').eq(2).find('circle')
           .should('have.attr', 'fill')
           .and('include', '#4EA15A');
@@ -129,7 +138,7 @@ context("Portal Dashboard Feedback Panel", () => {
           .and('eq', 'rgb(255, 255, 255)');
         cy.get('[data-cy=feedback-badge]').eq(2).find('circle')
           .should('have.attr', 'fill')
-          .and('include', '#4EA15A'); //Still green because it has text feedback
+          .and('not.include', '#4EA15A');
       });
     });
     describe('verify question-level feedback textareas appear and accept input', () => {

diff --git a/cypress/integration/portal-dashboard/portal-dashboard-response-table.spec.js b/cypress/integration/portal-dashboard/portal-dashboard-response-table.spec.js
@@ -122,7 +122,7 @@ context("Portal Dashboard Response Table",()=>{
 
 context("Feedback badges in response table", () => {
   before(()=>{
-    cy.visit("/?portal-dashboard&enableFirestorePersistence=true&clearFirestorePersistence=true");
+    cy.visit("/?portal-dashboard&enableFirestorePersistence=true&clearFirestorePersistence=true&debug:disableRubric=true");
     cy.get("[data-cy=navigation-select]").click();
     cy.get("[data-cy=list-item-feedback-report]").click();
     cy.get('[data-cy=question-level-feedback-button]').should('be.visible').click();

diff --git a/js/components/portal-dashboard/feedback/activity-level-feedback-student-rows.tsx b/js/components/portal-dashboard/feedback/activity-level-feedback-student-rows.tsx
@@ -8,9 +8,8 @@ import AwaitingFeedbackActivityBadgeIcon from "../../../../img/svg-icons/awaitin
 import GivenFeedbackActivityBadgeIcon from "../../../../img/svg-icons/given-feedback-activity-badge-icon.svg";
 import { SORT_BY_FEEDBACK_PROGRESS } from "../../../actions/dashboard";
 import { TrackEventFunction } from "../../../actions";
-import { hasRubricFeedback } from "../../../util/activity-feedback-helper";
 import { Rubric } from "./rubric-utils";
-import { ScoringSettings } from "../../../util/scoring";
+import { ScoringSettings, hasFeedbackGivenScoreType } from "../../../util/scoring";
 import { ShowStudentAnswers } from "./show-student-answers";
 
 import css from "../../../../css/portal-dashboard/feedback/feedback-rows.less";
@@ -49,7 +48,14 @@ export const ActivityLevelFeedbackStudentRows: React.FC<IProps> = (props) => {
     const score = feedbackData.get("score");
     const hasRubric = rubric;
     const { rubricFeedback } = feedbackData.toJS();
-    const hasFeedbacks = feedback || (hasRubric && hasRubricFeedback(rubric, rubricFeedback)) || score !== undefined;
+    const hasFeedbacks = hasFeedbackGivenScoreType({
+      scoreType: scoringSettings.scoreType,
+      textFeedback: feedbackData.get("feedback"),
+      scoreFeedback: feedbackData.get("score"),
+      rubric,
+      rubricFeedback
+    });
+
     const feedbackBadge = hasFeedbacks ? <GivenFeedbackActivityBadgeIcon /> : <AwaitingFeedbackActivityBadgeIcon />;
 
     return (

diff --git a/js/components/portal-dashboard/feedback/rubric-summary-icon.tsx b/js/components/portal-dashboard/feedback/rubric-summary-icon.tsx
@@ -32,7 +32,7 @@ const maxIconHeight = Math.round(iconWidth / 1.66); // keep rectangular
 const defaultIconRowHeight = 18;
 
 export const RubricSummaryIcon: React.FC<IProps> = (props) => {
-  const { rubric, feedbacks: { rubricFeedbacks }, activityId, scoringSettings, trackEvent } = props;
+  const { rubric, feedbacks, activityId, scoringSettings, trackEvent } = props;
   const [modalOpen, setModalOpen] = useState(false);
 
   const handleToggleModal = () => setModalOpen(prev => {
@@ -52,6 +52,11 @@ export const RubricSummaryIcon: React.FC<IProps> = (props) => {
         return acc;
       }, 0);
     };
+    const rubricFeedbacks = feedbacks.feedbacks
+      .filter((f: any) => f.has("rubricFeedback"))
+      .map((f: any) => f.get("rubricFeedback"))
+      .toJS();
+
     const numCompletedRubrics = rubricFeedbacks.reduce((acc: number, cur: PartialRubricFeedback) => {
       const numNonZeroScores = getNumNonZeroScores(cur);
       if (numNonZeroScores >= rubric.criteria.length) {
@@ -89,7 +94,7 @@ export const RubricSummaryIcon: React.FC<IProps> = (props) => {
     }
 
     return { hasRubricFeedback, criteriaCounts };
-  }, [rubricFeedbacks, rubric]);
+  }, [feedbacks, rubric]);
 
   const renderIcon = () => {
     const rowHeight = Math.min(Math.round(maxIconHeight / criteriaCounts.length), defaultIconRowHeight);

diff --git a/js/components/portal-dashboard/student-answers.tsx b/js/components/portal-dashboard/student-answers.tsx
@@ -3,9 +3,8 @@ import { Map, List } from "immutable";
 import AnswerCompact from "../../containers/portal-dashboard/answer-compact";
 import { TrackEventFunction } from "../../actions";
 import ActivityFeedbackGivenIcon from "../../../img/svg-icons/feedback-activity-badge-icon.svg";
-import { hasRubricFeedback } from "../../util/activity-feedback-helper";
 import { Rubric } from "./feedback/rubric-utils";
-import { ScoreType, ScoringSettings, getRubricDisplayScore, getScoredQuestions } from "../../util/scoring";
+import { ScoreType, ScoringSettings, getRubricDisplayScore, getScoredQuestions, hasFeedbackGivenScoreType } from "../../util/scoring";
 import { AUTOMATIC_SCORE, MANUAL_SCORE, NO_SCORE, RUBRIC_SCORE } from "../../util/scoring-constants";
 
 import css from "../../../css/portal-dashboard/student-answers.less";
@@ -212,10 +211,13 @@ export class StudentAnswers extends React.PureComponent<IProps> {
     const progressClass = !hideFeedbackBadges && hasBeenReviewed ? css.reviewed
                                           : progress > 0 ? css.progress : "";
     const rubricFeedback = activityFeedbacks.size > 0 && activityStudentFeedback?.get("rubricFeedback")?.toJS();
-    const rubricFeedbackGiven = rubric && hasRubricFeedback(rubric, rubricFeedback);
-    const textFeedbackGiven = (activityStudentFeedback?.get("feedback") ?? "") !== "";
-    const scoreFeedbackGiven = activityStudentFeedback?.get("score") !== undefined;
-    const hasFeedbacks = activityFeedbacks.size > 0 && (textFeedbackGiven || rubricFeedbackGiven || scoreFeedbackGiven);
+    const hasFeedbacks = hasFeedbackGivenScoreType({
+      scoreType: this.props.scoringSettings.scoreType,
+      textFeedback: activityStudentFeedback?.get("feedback"),
+      scoreFeedback: activityStudentFeedback?.get("score"),
+      rubric,
+      rubricFeedback
+    });
 
     return (
       <div className={`${css.activityProgress} ${progressClass}`} key={activity.get("id")}>

diff --git a/js/reducers/feedback-reducer.ts b/js/reducers/feedback-reducer.ts
@@ -7,8 +7,7 @@ import {
 } from "../actions";
 import { RecordFactory } from "../util/record-factory";
 import { normalizeResourceJSON } from "../core/transform-json-response";
-import { getScoringSettings } from "../util/scoring";
-import { MANUAL_SCORE, RUBRIC_SCORE } from "../util/scoring-constants";
+import { getScoringSettings, hasFeedbackGivenScoreType } from "../util/scoring";
 import { Rubric } from "../components/portal-dashboard/feedback/rubric-utils";
 
 export interface IFeedbackState {
@@ -37,10 +36,25 @@ export class FeedbackState extends INITIAL_FEEDBACK_STATE implements IFeedbackSt
 
 export default function feedback(state = new FeedbackState({}), action: any) {
   let hasScoredQuestions: any;
+  let actFeedbacks: any;
 
   switch (action.type) {
     case RECEIVE_FEEDBACK_SETTINGS:
-      return state.set("settings", fromJS(action.response) as Map<any, any>);
+      const settingsMap = fromJS(action.response) as Map<any, any>;
+      hasScoredQuestions = state.get("hasScoredQuestions");
+      const activityFeedbacks = state.get("activityFeedbacks");
+      if (activityFeedbacks.size > 0 && hasScoredQuestions !== undefined) {
+        // score type may have changed so all feedback needs to be re-evaluated to see if it has been reviewed
+        actFeedbacks = updateHashBeenReviewed({
+          activityFeedbackValues: activityFeedbacks.toList().toJS(),
+          settings: action.response,
+          hasScoredQuestions: hasScoredQuestions.toJS()
+        });
+        return state
+          .set("activityFeedbacks", fromJS(actFeedbacks) as Map<any, any>)
+          .set("settings", settingsMap);
+      }
+      return state.set("settings", settingsMap);
     case RECEIVE_QUESTION_FEEDBACKS:
       const feedbacks = action.response.reduce((map: any, feedback: any) => {
         map[feedback.answerId] = feedback;
@@ -61,19 +75,30 @@ export default function feedback(state = new FeedbackState({}), action: any) {
     case RECEIVE_ACTIVITY_FEEDBACKS:
       const settings: any = state.get("settings")?.toJS();
       hasScoredQuestions = state.get("hasScoredQuestions")?.toJS();
-      const actFeedbacks = action.response.reduce((map: any, feedback: any) => {
-        if (feedback.activityId) {
-          feedback.hasBeenReviewed = computeHasBeenReviewed({feedback, settings, hasScoredQuestions});
-        }
-        map[`${feedback.activityId}-${feedback.platformStudentId}`] = feedback;
-        return map;
-      }, {});
+      actFeedbacks = updateHashBeenReviewed({
+        activityFeedbackValues: action.response,
+        settings,
+        hasScoredQuestions
+      });
       return state.set("activityFeedbacks", fromJS(actFeedbacks) as Map<any, any>);
     default:
       return state;
   }
 }
 
+export const updateHashBeenReviewed = (params: {activityFeedbackValues: any; settings: any; hasScoredQuestions: any}) => {
+  const {activityFeedbackValues, settings, hasScoredQuestions} = params;
+
+  const result = activityFeedbackValues.reduce((map: any, feedback: any) => {
+    if (feedback.activityId) {
+      feedback.hasBeenReviewed = computeHasBeenReviewed({feedback, settings, hasScoredQuestions});
+    }
+    map[`${feedback.activityId}-${feedback.platformStudentId}`] = feedback;
+    return map;
+  }, {});
+  return result;
+};
+
 export const computeHasBeenReviewed = (params: {feedback: any; settings: any; hasScoredQuestions: any}) => {
   const {feedback, settings, hasScoredQuestions} = params;
 
@@ -84,24 +109,13 @@ export const computeHasBeenReviewed = (params: {feedback: any; settings: any; ha
     hasScoredQuestions: hasScoredQuestions?.[feedback.activityId],
   }).scoreType;
 
-  const hasScore = !isNaN(parseInt(feedback.score, 10));
-  const hasText = (feedback.feedback ?? "").length > 0;
-
-  let hasFilledRubric = false;
-  const rubricFeedback = feedback.rubricFeedback;
-  if (rubric && rubricFeedback) {
-    const scoredValues = Object.values(rubricFeedback).filter((v: any) => v.score > 0);
-    hasFilledRubric = scoredValues.length === rubric.criteria.length;
-  }
-
-  switch (scoreType) {
-    case MANUAL_SCORE:
-      return hasScore;
-
-    case RUBRIC_SCORE:
-      return hasFilledRubric;
+  const result = hasFeedbackGivenScoreType({
+    scoreType,
+    textFeedback: feedback.feedback,
+    scoreFeedback: feedback.score,
+    rubric,
+    rubricFeedback: feedback.rubricFeedback
+  });
 
-    default:
-      return hasText || hasFilledRubric;
-  }
+  return result;
 };
diff --git a/js/util/scoring.ts b/js/util/scoring.ts
@@ -118,3 +118,26 @@ export const getRubricDisplayScore = (rubric: Rubric, rubricFeedback: any, maxSc
   return displayScore;
 };
 
+export const hasFeedbackGivenScoreType = (options: {scoreType: ScoreType; textFeedback?: string; scoreFeedback?: number; rubric?: Rubric; rubricFeedback: any }) => {
+  const {scoreType, textFeedback, scoreFeedback, rubric, rubricFeedback } = options;
+
+  const hasScore = scoreFeedback !== undefined; // !isNaN(parseInt(scoreFeedback, 10));
+  const hasText = (textFeedback ?? "").length > 0;
+
+  let hasFilledRubric = false;
+  if (rubric && rubricFeedback) {
+    const scoredValues = Object.values(rubricFeedback).filter((v: any) => v.score > 0);
+    hasFilledRubric = scoredValues.length === rubric.criteria.length;
+  }
+
+  switch (scoreType) {
+    case MANUAL_SCORE:
+      return hasScore;
+
+    case RUBRIC_SCORE:
+      return hasFilledRubric;
+
+    default:
+      return hasText || hasFilledRubric;
+  }
+};