diff --git a/cypress/integration/portal-dashboard/portal-dashboard-feedback.spec.js b/cypress/integration/portal-dashboard/portal-dashboard-feedback.spec.js index fde0a2e5..f47cfca2 100644 --- 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 index 65e07516..8a6d9ba0 100644 --- 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 index 27b9859f..4c9abae1 100644 --- 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 = (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 ? : ; return ( diff --git a/js/components/portal-dashboard/feedback/rubric-summary-icon.tsx b/js/components/portal-dashboard/feedback/rubric-summary-icon.tsx index 68054775..de5e7715 100644 --- 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 = (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 = (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 = (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 index 229635d7..c8a1a2c8 100644 --- 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 { 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 (
diff --git a/js/reducers/feedback-reducer.ts b/js/reducers/feedback-reducer.ts index 73ec3664..1459be66 100644 --- 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); + const settingsMap = fromJS(action.response) as Map; + 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) + .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); 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 index 0253b796..efeb9f87 100644 --- 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; + } +};