Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User can report answer in peer review, teacher can view reports in manual review #1364

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { css, cx } from "@emotion/css"
import { useQuery } from "@tanstack/react-query"
import { ExclamationMessage } from "@vectopus/atlas-icons-react"
import React, { useContext, useMemo, useState } from "react"
import { useTranslation } from "react-i18next"

Expand All @@ -8,15 +9,18 @@ import ContentRenderer from "../../.."
import {
Block,
fetchPeerOrSelfReviewDataByExerciseId,
postFlagAnswerInPeerReview,
postPeerOrSelfReviewSubmission,
} from "../../../../../services/backend"
import ExerciseTaskIframe from "../ExerciseTaskIframe"

import PeerOrSelfReviewQuestion from "./PeerOrSelfReviewQuestion"
import MarkAsSpamDialog from "./PeerRevireMarkingSpam/MarkAsSpamDialog"

import { getPeerReviewBeginningScrollingId, PeerOrSelfReviewViewProps } from "."

import { CourseMaterialPeerOrSelfReviewQuestionAnswer } from "@/shared-module/common/bindings"
import Button from "@/shared-module/common/components/Button"
import ErrorBanner from "@/shared-module/common/components/ErrorBanner"
import PeerReviewProgress from "@/shared-module/common/components/PeerReview/PeerReviewProgress"
import Spinner from "@/shared-module/common/components/Spinner"
Expand All @@ -37,6 +41,7 @@ const PeerOrSelfReviewViewImpl: React.FC<React.PropsWithChildren<PeerOrSelfRevie
const [answers, setAnswers] = useState<Map<string, CourseMaterialPeerOrSelfReviewQuestionAnswer>>(
new Map(),
)
const [isReportDialogOpen, setIsReportDialogOpen] = useState(false)

const query = useQuery({
queryKey: [`exercise-${exerciseId}-peer-or-self-review`],
Expand Down Expand Up @@ -126,6 +131,27 @@ const PeerOrSelfReviewViewImpl: React.FC<React.PropsWithChildren<PeerOrSelfRevie
},
)

const reportMutation = useToastMutation(
async ({ reason, description }: { reason: string; description: string }) => {
if (!peerOrSelfReviewData || !peerOrSelfReviewData.answer_to_review) {
return
}
return await postFlagAnswerInPeerReview(exerciseId, {
submission_id: peerOrSelfReviewData.answer_to_review.exercise_slide_submission_id,
reason,
description,
flagged_user: null,
flagged_by: null,
})
},
{ notify: true, method: "POST" },
{},
)

const handleReportSubmit = (reason: string, description: string) => {
reportMutation.mutate({ reason, description })
}

if (query.isError) {
return <ErrorBanner variant={"readOnly"} error={query.error} />
}
Expand Down Expand Up @@ -306,6 +332,25 @@ const PeerOrSelfReviewViewImpl: React.FC<React.PropsWithChildren<PeerOrSelfRevie
>
{t("submit-button")}
</button>
<Button
className={css`
display: flex !important;
align-items: center;
gap: 6px;
`}
variant={"icon"}
transform="capitalize"
size={"small"}
onClick={() => setIsReportDialogOpen(true)}
>
<ExclamationMessage /> {t("button-text-report")}
</Button>

<MarkAsSpamDialog
isOpen={isReportDialogOpen}
onClose={() => setIsReportDialogOpen(false)}
onSubmit={handleReportSubmit}
/>
</div>
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { css } from "@emotion/css"
import styled from "@emotion/styled"
import React, { useState } from "react"
import { useTranslation } from "react-i18next"

import Button from "@/shared-module/common/components/Button"
import Dialog from "@/shared-module/common/components/Dialog"
import RadioButton from "@/shared-module/common/components/InputFields/RadioButton"

const FieldContainer = styled.div`
margin-bottom: 1rem;
`

const MarkAsSpamDialog: React.FC<{
isOpen: boolean
onClose: () => void
onSubmit: (reason: string, description: string) => void
}> = ({ isOpen, onClose, onSubmit }) => {
const { t } = useTranslation()
const [selectedReason, setSelectedReason] = useState<string>("")
const [description, setDescription] = useState<string>("")
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add form validation and improve type safety for reasons.

Consider using an enum for report reasons to ensure type safety and validation.

+export enum ReportReason {
+  SPAM = 'flagging-reason-spam',
+  HARMFUL_CONTENT = 'flagging-reason-harmful-content',
+  AI_GENERATED = 'flagging-reason-ai-generated',
+}

-const [selectedReason, setSelectedReason] = useState<string>("")
+const [selectedReason, setSelectedReason] = useState<ReportReason | "">("")

Committable suggestion skipped: line range outside the PR's diff.


const handleSubmit = () => {
if (selectedReason) {
onSubmit(selectedReason, description)
setSelectedReason("")
setDescription("")
onClose()
}
}

return (
<Dialog open={isOpen} onClose={onClose}>
<h1> {t("title-report-dialog")}</h1>

<div
className={css`
margin-bottom: 1rem;
`}
>
{t("select-reason")}
<FieldContainer>
<RadioButton
label={t("flagging-reason-spam")}
value={t("flagging-reason-spam")}
// eslint-disable-next-line i18next/no-literal-string
name={"reason"}
onChange={(e) => setSelectedReason(e.target.value)}
></RadioButton>
</FieldContainer>
<FieldContainer>
<RadioButton
label={t("flagging-reason-harmful-content")}
value={t("flagging-reason-harmful-content")}
// eslint-disable-next-line i18next/no-literal-string
name={"reason"}
onChange={(e) => setSelectedReason(e.target.value)}
></RadioButton>
</FieldContainer>
<FieldContainer>
<RadioButton
label={t("flagging-reason-ai-generated")}
value={t("flagging-reason-ai-generated")} // eslint-disable-next-line i18next/no-literal-string
name={"reason"}
onChange={(e) => setSelectedReason(e.target.value)}
></RadioButton>
</FieldContainer>
</div>
<textarea
placeholder={t("optional-description")}
value={description}
onChange={(e) => setDescription(e.target.value)}
className={css`
width: 100%;
height: 5rem;
margin-bottom: 1rem;
`}
/>
<Button variant="primary" onClick={handleSubmit} disabled={!selectedReason} size={"small"}>
{t("submit-button")}
</Button>
</Dialog>
)
}

export default MarkAsSpamDialog
Comment on lines +1 to +86
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix typo in folder name and improve form validation.

  1. The folder name contains a typo: "PeerRevireMarkingSpam" should be "PeerReviewMarkingSpam"
  2. Add form validation for the description field
  3. Improve accessibility with ARIA labels
 const MarkAsSpamDialog: React.FC<{
   isOpen: boolean
   onClose: () => void
   onSubmit: (reason: string, description: string) => void
 }> = ({ isOpen, onClose, onSubmit }) => {
   const { t } = useTranslation()
   const [selectedReason, setSelectedReason] = useState<string>("")
   const [description, setDescription] = useState<string>("")
+  const [error, setError] = useState<string>("")
+  const MAX_DESCRIPTION_LENGTH = 1000
 
   const handleSubmit = () => {
     if (selectedReason) {
+      if (description && description.length > MAX_DESCRIPTION_LENGTH) {
+        setError(t("error-description-too-long"))
+        return
+      }
       onSubmit(selectedReason, description)
       setSelectedReason("")
       setDescription("")
+      setError("")
       onClose()
     }
   }
 
   return (
-    <Dialog open={isOpen} onClose={onClose}>
+    <Dialog 
+      open={isOpen} 
+      onClose={onClose}
+      aria-labelledby="report-dialog-title"
+      role="dialog"
+    >
-      <h1> {t("title-report-dialog")}</h1>
+      <h1 id="report-dialog-title">{t("title-report-dialog")}</h1>
 
       <div
         className={css`
           margin-bottom: 1rem;
         `}
+        role="radiogroup"
+        aria-label={t("select-reason")}
       >
         {t("select-reason")}
         <FieldContainer>
           <RadioButton
             label={t("flagging-reason-spam")}
             value={t("flagging-reason-spam")}
-            // eslint-disable-next-line i18next/no-literal-string
-            name={"reason"}
+            name="report-reason"
+            aria-describedby="report-reason-description"
             onChange={(e) => setSelectedReason(e.target.value)}
           />
         </FieldContainer>
         {/* Similar changes for other RadioButton components */}
       </div>
       <textarea
         placeholder={t("optional-description")}
         value={description}
         onChange={(e) => setDescription(e.target.value)}
+        maxLength={MAX_DESCRIPTION_LENGTH}
+        aria-label={t("report-description")}
         className={css`
           width: 100%;
           height: 5rem;
           margin-bottom: 1rem;
         `}
       />
+      {error && (
+        <div role="alert" className={css`color: ${baseTheme.colors.error};`}>
+          {error}
+        </div>
+      )}
       <Button
         variant="primary"
         onClick={handleSubmit}
         disabled={!selectedReason}
         size={"small"}
+        aria-label={t("submit-report")}
       >
         {t("submit-button")}
       </Button>
     </Dialog>
   )
 }

Committable suggestion skipped: line range outside the PR's diff.

11 changes: 11 additions & 0 deletions services/course-material/src/services/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
IsChapterFrontPage,
MaterialReference,
NewFeedback,
NewFlaggedAnswer,
NewMaterialReference,
NewProposedPageEdits,
NewResearchFormQuestionAnswer,
Expand Down Expand Up @@ -412,6 +413,16 @@ export const postPeerOrSelfReviewSubmission = async (
)
}

export const postFlagAnswerInPeerReview = async (
exerciseId: string,
newFlaggedAnswer: NewFlaggedAnswer,
): Promise<void> => {
await courseMaterialClient.post(
`/exercises/${exerciseId}/flag-peer-review-answer`,
newFlaggedAnswer,
)
}

export const postStartPeerOrSelfReview = async (exerciseId: string): Promise<void> => {
await courseMaterialClient.post(`/exercises/${exerciseId}/peer-or-self-reviews/start`)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
DROP TABLE IF EXISTS flagged_answers;

ALTER TABLE exercise_slide_submissions DROP COLUMN flag_count;

ALTER TABLE courses DROP COLUMN flagged_answers_threshold;
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
CREATE TABLE flagged_answers (
id UUID DEFAULT uuid_generate_v4() PRIMARY KEY,
submission_id UUID NOT NULL REFERENCES exercise_slide_submissions(id),
flagged_user UUID NOT NULL REFERENCES users(id),
flagged_by UUID NOT NULL REFERENCES users(id),
reason TEXT NOT NULL,
description TEXT,
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(),
updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(),
deleted_at TIMESTAMP WITH TIME ZONE,
UNIQUE NULLS NOT DISTINCT (submission_id, flagged_by, deleted_at)
);
Comment on lines +1 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add index on submission_id for better query performance.

Since submission_id will be frequently queried to count flags and retrieve flagged answers, an index would improve query performance.

Add this index after the table creation:

+CREATE INDEX idx_flagged_answers_submission_id ON flagged_answers(submission_id);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CREATE TABLE flagged_answers (
id UUID DEFAULT uuid_generate_v4() PRIMARY KEY,
submission_id UUID NOT NULL REFERENCES exercise_slide_submissions(id),
flagged_user UUID NOT NULL REFERENCES users(id),
flagged_by UUID NOT NULL REFERENCES users(id),
reason TEXT NOT NULL,
description TEXT,
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(),
updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(),
deleted_at TIMESTAMP WITH TIME ZONE,
UNIQUE NULLS NOT DISTINCT (submission_id, flagged_by, deleted_at)
);
CREATE TABLE flagged_answers (
id UUID DEFAULT uuid_generate_v4() PRIMARY KEY,
submission_id UUID NOT NULL REFERENCES exercise_slide_submissions(id),
flagged_user UUID NOT NULL REFERENCES users(id),
flagged_by UUID NOT NULL REFERENCES users(id),
reason TEXT NOT NULL,
description TEXT,
created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(),
updated_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW(),
deleted_at TIMESTAMP WITH TIME ZONE,
UNIQUE NULLS NOT DISTINCT (submission_id, flagged_by, deleted_at)
);
CREATE INDEX idx_flagged_answers_submission_id ON flagged_answers(submission_id);


CREATE TRIGGER set_timestamp BEFORE
UPDATE ON flagged_answers FOR EACH ROW EXECUTE PROCEDURE trigger_set_timestamp();

COMMENT ON TABLE flagged_answers IS 'Used to keep track of answers that has been flagged in peer review';
COMMENT ON COLUMN flagged_answers.submission_id IS 'The id of the exercise task submission being flagged.';
COMMENT ON COLUMN flagged_answers.flagged_user IS 'The id of the user whose answer was flagged.';
COMMENT ON COLUMN flagged_answers.flagged_by IS 'The id of the user who flagged the answer.';
COMMENT ON COLUMN flagged_answers.reason IS 'The reason for flagging the answer.';
COMMENT ON COLUMN flagged_answers.description IS 'Optional additional explanation provided by the user.';
COMMENT ON COLUMN flagged_answers.created_at IS 'Timestamp when the flag was created.';
COMMENT ON COLUMN flagged_answers.updated_at IS 'Timestamp when the flag was last updated. The field is updated automatically by the set_timestamp trigger.';
COMMENT ON COLUMN flagged_answers.deleted_at IS 'Timestamp when the flag was deleted. If null, the record is not deleted.';

ALTER TABLE exercise_slide_submissions
ADD COLUMN flag_count INTEGER NOT NULL DEFAULT 0;

COMMENT ON COLUMN exercise_slide_submissions.flag_count IS 'The number of times the submission has been flagged.';

ALTER TABLE courses
ADD COLUMN flagged_answers_threshold INTEGER NOT NULL DEFAULT 5;

COMMENT ON COLUMN courses.flagged_answers_threshold IS 'The amount of flags required to trigger a teacher review for an answer.';
Comment on lines +32 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding a CHECK constraint for the threshold.

While the default value of 5 is reasonable, the column should prevent negative values or zero, which wouldn't make sense for a threshold.

 ALTER TABLE courses
-ADD COLUMN flagged_answers_threshold INTEGER NOT NULL DEFAULT 5;
+ADD COLUMN flagged_answers_threshold INTEGER NOT NULL DEFAULT 5
+  CHECK (flagged_answers_threshold > 0);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ALTER TABLE courses
ADD COLUMN flagged_answers_threshold INTEGER NOT NULL DEFAULT 5;
COMMENT ON COLUMN courses.flagged_answers_threshold IS 'The amount of flags required to trigger a teacher review for an answer.';
ALTER TABLE courses
ADD COLUMN flagged_answers_threshold INTEGER NOT NULL DEFAULT 5
CHECK (flagged_answers_threshold > 0);
COMMENT ON COLUMN courses.flagged_answers_threshold IS 'The amount of flags required to trigger a teacher review for an answer.';

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading