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

Add Publish Grading Feature #965

Merged
merged 87 commits into from
Apr 13, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
87 commits
Select commit Hold shift + click to select a range
8022419
Add migrations to create is_grading_published column for submissions
emptygx Jul 3, 2023
8125ac1
Merge branch 'master' into add-publish-grading
emptygx Jul 3, 2023
e286902
Add and update endpoints for new publish grading feature
emptygx Jul 7, 2023
d174fbd
Fix missing return tuple values in assessments.ex
emptygx Jul 9, 2023
bc36b60
Fix notification and formatting
emptygx Jul 12, 2023
5dcc425
Merge branch 'master' into add-publish-grading
emptygx Jul 16, 2023
2153280
Fix indentation and minor error
emptygx Jul 17, 2023
1891d72
Fix formatting
emptygx Jul 17, 2023
55c5a36
fix formatting
YaleChen299 Jul 23, 2023
af683fd
fix credo long line
YaleChen299 Jul 24, 2023
cf98cff
fix credo nesting alias
YaleChen299 Jul 24, 2023
568cc6f
Fix formatting and code quality
emptygx Jul 25, 2023
4700a49
Fix tests
emptygx Jul 28, 2023
0ec5a50
Merge branch 'master' into add-publish-grading
RichDom2185 Sep 3, 2023
a026ec8
Merge branch 'master' into add-publish-grading
GabrielCWT Feb 26, 2024
750ba95
feat: Implement format helper function
GabrielCWT Mar 1, 2024
f82e43f
feat: Add formatting function for getting all assessments
GabrielCWT Mar 1, 2024
e7090f8
fix: Fix bug of using virtual variable instead
GabrielCWT Mar 1, 2024
5c1b155
refactor: Remove default value for virtual variable is_grading_published
GabrielCWT Mar 1, 2024
788e31e
feat: Implement tests for helper functions
GabrielCWT Mar 2, 2024
70ebac5
feat: Add isGradingPublished to response for fetching all assessments
GabrielCWT Mar 2, 2024
54cbcfc
refactor: Format
GabrielCWT Mar 2, 2024
cf62185
chore: Remove unused match
GabrielCWT Mar 4, 2024
6b5f0e8
feat: Implement tests for unpublish route
GabrielCWT Mar 4, 2024
2a05597
feat: Implement tests for publish route
GabrielCWT Mar 4, 2024
9934af4
refactor: Move repeated code into setup for publish test
GabrielCWT Mar 4, 2024
1097e23
refactor: Move repeated code into setup for unpublish test
GabrielCWT Mar 4, 2024
4ce2d6e
refactor: Format code
GabrielCWT Mar 4, 2024
9de59c6
Merge branch 'master' into add-publish-grading
GabrielCWT Mar 4, 2024
98587e6
Merge branch 'master' into add-publish-grading
RichDom2185 Mar 4, 2024
d046b3b
feat: Add a guard to prevent unsubmit when grade is still published
GabrielCWT Mar 14, 2024
07f2548
feat: Implement filter by notPublished
GabrielCWT Mar 14, 2024
9b33670
refactor: Format
GabrielCWT Mar 14, 2024
e658166
fix: Fix incorrect guard check for is_grading_published
GabrielCWT Mar 15, 2024
054d89a
refactor: Edit tests to accommodate new guard
GabrielCWT Mar 15, 2024
2caecd7
feat: Implement test for new guard (check is_grading_published)
GabrielCWT Mar 15, 2024
dd7742e
Merge branch 'master' into add-publish-grading
GabrielCWT Mar 21, 2024
f65bc56
Merge branch 'master' into add-publish-grading
GabrielCWT Mar 21, 2024
1f41b11
feat: Update seed to include is_grading_published
GabrielCWT Mar 21, 2024
8c1387f
refactor: Update old tests to accommodate for is_grading_published
GabrielCWT Mar 21, 2024
7cc1448
refactor: Improve filter tests to be more flexible
GabrielCWT Mar 21, 2024
44cbab8
feat: Implement test for filter by not published
GabrielCWT Mar 21, 2024
89411ad
chore: Format
GabrielCWT Mar 21, 2024
888a4f7
refactor: Make is_grading_published default to false in factory function
GabrielCWT Mar 21, 2024
4298970
feat: Add is_grading_auto_published column to assessment config
GabrielCWT Mar 22, 2024
147cf4d
feat: Add guard clause to ensure submission is fully graded before pu…
GabrielCWT Mar 22, 2024
a41a894
feat: Implement publish_grading and is_fully_autograded?
GabrielCWT Mar 22, 2024
d2b173b
chore: Clean up code
GabrielCWT Mar 22, 2024
626b61c
feat: Add is_grading_auto_published and is_manually_graded to seed
GabrielCWT Mar 22, 2024
07a63e6
feat: Implement tests for is_fully_autograded?/1
GabrielCWT Mar 22, 2024
38b3303
feat: Implement publish_all_grades route
GabrielCWT Mar 24, 2024
ae9b42f
feat: Add publish_all_grades in controller
GabrielCWT Mar 24, 2024
b606905
feat: Implement publish_all_graded function
GabrielCWT Mar 24, 2024
582a394
feat: Implement tests for publish_all_graded/2
GabrielCWT Mar 24, 2024
ade0e24
refactor: Rename param names and allow filter by true or false
GabrielCWT Mar 24, 2024
f3a0eeb
refactor: Add tests for change in param and refactor code
GabrielCWT Mar 24, 2024
1abf60b
refactor: Change response for publish all grades
GabrielCWT Mar 24, 2024
1ae8d87
refactor: Use update_all instead of recursively updating individual s…
GabrielCWT Mar 24, 2024
5077d70
feat: Implement unpublish all grades route
GabrielCWT Mar 24, 2024
50a85a9
feat: Implement unpublish_all
GabrielCWT Mar 24, 2024
ddd522d
feat: Implement unpublish_all tests
GabrielCWT Mar 24, 2024
5360e3a
chore: Format
GabrielCWT Mar 24, 2024
a33ff69
feat: Implement auto publish for mcq/voting questions
GabrielCWT Mar 24, 2024
858c740
feat: Implement auto publish for auto graded programming questions
GabrielCWT Mar 24, 2024
f376f1e
chore: Fix consistency issue
GabrielCWT Mar 24, 2024
a2a23f7
Merge branch 'master' into add-publish-grading
GabrielCWT Mar 27, 2024
5b91c12
feat: Implement published and unpublished notifications and remove de…
GabrielCWT Mar 27, 2024
64b5305
feat: Include isGradingAutoPublished in response for assessment configs
GabrielCWT Mar 27, 2024
21c65f6
fix: Include sending of notifications when publishing/unpublishing all
GabrielCWT Apr 1, 2024
596d3c3
docs: Add docs for functions implemented
GabrielCWT Apr 2, 2024
340de3a
chore: Update wording for tests
GabrielCWT Apr 2, 2024
b2ab4b0
chore: Remove unused variables
GabrielCWT Apr 2, 2024
a25f9f7
feat: Implement test for unpublish and publish all routes
GabrielCWT Apr 6, 2024
9d9069c
Merge branch 'master' into add-publish-grading
GabrielCWT Apr 6, 2024
fd1508b
feat: Implement guard for publish/unpublish grades
GabrielCWT Apr 7, 2024
4ac87af
refactor: Change notification types in swagger_schema
GabrielCWT Apr 7, 2024
93d6bd2
chore: Add comment in seed
GabrielCWT Apr 7, 2024
bd9abf9
refactor: remove redundant lines
GabrielCWT Apr 8, 2024
c34d265
Merge branch 'master' into add-publish-grading
GabrielCWT Apr 8, 2024
365cfb4
Merge branch 'master' into add-publish-grading
GabrielCWT Apr 11, 2024
ce6ec53
Redate migrations
RichDom2185 Apr 11, 2024
710213c
Revert unnecessary changes
RichDom2185 Apr 11, 2024
593708e
Revert more unnecessary changes
RichDom2185 Apr 11, 2024
d00b686
refactor: Move duplicate code into helper function
GabrielCWT Apr 12, 2024
e355d54
chore: Fix typo
GabrielCWT Apr 12, 2024
541fac9
refactor: Change control flow structure
GabrielCWT Apr 12, 2024
b366bc9
Merge branch 'master' into add-publish-grading
RichDom2185 Apr 13, 2024
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
4 changes: 3 additions & 1 deletion lib/cadet/accounts/notification_type.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ defenum(Cadet.Accounts.NotificationType, :notification_type, [
# Notifications for unsubmitted submissions
:unsubmitted,
# Notifications for submitted assessments
:submitted
:submitted,
# Notifications for unpublished grades
:unpublished_grading
])
26 changes: 25 additions & 1 deletion lib/cadet/accounts/notifications.ex
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ defmodule Cadet.Accounts.Notifications do
Notification
|> where(course_reg_id: ^student.id)
|> where(assessment_id: ^assessment_id)
|> where([n], n.type in ^[:autograded, :graded])
|> where([n], n.type in ^[:autograded, :graded, :unpublished_grading])
|> Repo.delete_all()

write(%{
Expand All @@ -162,6 +162,30 @@ defmodule Cadet.Accounts.Notifications do
})
end

@doc """
Function that handles notifications when a submission grade is unpublished.
"""
@spec handle_unpublish_grades_notifications(integer(), CourseRegistration.t()) ::
{:ok, Ecto.Schema.t()} | {:error, Ecto.Changeset.t()}
def handle_unpublish_grades_notifications(assessment_id, student = %CourseRegistration{})
when is_ecto_id(assessment_id) do
# Fetch and delete all notifications of :graded
# Add new notification :unpublished

Notification
|> where(course_reg_id: ^student.id)
|> where(assessment_id: ^assessment_id)
|> where([n], n.type in ^[:autograded, :graded, :unsubmitted])
|> Repo.delete_all()

write(%{
type: :unpublished_grading,
role: student.role,
course_reg_id: student.id,
assessment_id: assessment_id
})
end

@doc """
Writes a notification that a student's submission has been
graded successfully. (for the student)
Expand Down
1 change: 1 addition & 0 deletions lib/cadet/assessments/assessment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ defmodule Cadet.Assessments.Assessment do
field(:grading_status, :string, virtual: true)
field(:question_count, :integer, virtual: true)
field(:graded_count, :integer, virtual: true)
field(:is_grading_published, :boolean, virtual: true)
field(:title, :string)
field(:is_published, :boolean, default: false)
field(:summary_short, :string)
Expand Down
174 changes: 163 additions & 11 deletions lib/cadet/assessments/assessments.ex
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
submission_xp =
Submission
|> where(student_id: ^cr_id)
|> where(is_grading_published: true)
|> join(:inner, [s], a in Answer, on: s.id == a.submission_id)
|> group_by([s], s.id)
|> select([s, a], %{
Expand Down Expand Up @@ -281,7 +282,18 @@
end)
|> load_contest_voting_entries(course_reg, assessment)

assessment = assessment |> Map.put(:questions, questions)
is_grading_published =
Submission
|> where(assessment_id: ^id)
|> where(student_id: ^course_reg.id)
|> select([s], s.is_grading_published)
|> Repo.one()

assessment =
assessment
|> Map.put(:questions, questions)
|> Map.put(:is_grading_published, is_grading_published)

{:ok, assessment}
else
{:error, {:forbidden, "Assessment not open"}}
Expand Down Expand Up @@ -312,7 +324,7 @@
submission_status =
Submission
|> where([s], s.student_id == ^cr.id)
|> select([s], [:assessment_id, :status])
|> select([s], [:assessment_id, :status, :is_grading_published])

assessments =
cr.course_id
Expand All @@ -329,7 +341,8 @@
a
| xp: sa.xp,
graded_count: sa.graded_count,
user_status: s.status
user_status: s.status,
is_grading_published: s.is_grading_published
})
|> filter_published_assessments(cr)
|> order_by(:open_at)
Expand All @@ -339,6 +352,52 @@
{:ok, assessments}
end

@doc """
A helper function which removes grading information from all assessments
if it's grading is not published.
"""
def format_all_assessments(assessments) do
Enum.map(assessments, fn a ->
if a.is_grading_published do
a
else
a
|> Map.put(:xp, 0)
|> Map.put(:graded_count, 0)
end
end)
end

@doc """
A helper function which removes grading information from the assessment
if it's grading is not published.
"""
def format_assessment_with_questions_and_answers(assessment) do
if assessment.is_grading_published do
assessment
else
%{
assessment
| questions:
Enum.map(assessment.questions, fn q ->
%{
q
| answer: %{
q.answer
| xp: 0,
xp_adjustment: 0,
autograding_status: :none,
autograding_results: [],
grader: nil,
grader_id: nil,
comments: nil
}
}
end)
}
end
end

def filter_published_assessments(assessments, cr) do
role = cr.role

Expand Down Expand Up @@ -851,7 +910,9 @@
{:allowed_to_unsubmit?, true} <-
{:allowed_to_unsubmit?,
role == :admin or bypass or
Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)} do
Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)},
{:is_grading_published?, false} <-
{:is_grading_published?, submission.is_grading_published} do
Multi.new()
|> Multi.run(
:rollback_submission,
Expand Down Expand Up @@ -893,7 +954,7 @@
end)
|> Repo.transaction()

Cadet.Accounts.Notifications.handle_unsubmit_notifications(
Notifications.handle_unsubmit_notifications(
submission.assessment.id,
Repo.get(CourseRegistration, submission.student_id)
)
Expand All @@ -915,6 +976,98 @@
{:allowed_to_unsubmit?, false} ->
{:error, {:forbidden, "Only Avenger of student or Admin is permitted to unsubmit"}}

{:is_grading_published?, true} ->
{:error, {:forbidden, "Grading has not been unpublished"}}

_ ->
{:error, {:internal_server_error, "Please try again later."}}
end
end

# This function changes the is_grading_published field of the submission to false
# and sends a notification to the student
def unpublish_grading(submission_id, cr = %CourseRegistration{id: course_reg_id, role: role})
when is_ecto_id(submission_id) do
submission =
Submission
|> join(:inner, [s], a in assoc(s, :assessment))
|> preload([_, a], assessment: a)
|> Repo.get(submission_id)

# allows staff to unpublish own assessment
bypass = role in @bypass_closed_roles and submission.student_id == course_reg_id

with {:submission_found?, true} <- {:submission_found?, is_map(submission)},
{:allowed_to_unpublish?, true} <-
{:allowed_to_unpublish?,
role == :admin or bypass or
Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)} do
submission
|> Submission.changeset(%{is_grading_published: false})
|> Repo.update()

Notifications.handle_unpublish_grades_notifications(
submission.assessment.id,
Repo.get(CourseRegistration, submission.student_id)
)

{:ok, nil}
else
{:submission_found?, false} ->
{:error, {:not_found, "Submission not found"}}

{:allowed_to_unpublish?, false} ->
{:error,
{:forbidden, "Only Avenger of student or Admin is permitted to unpublish grading"}}

_ ->
{:error, {:internal_server_error, "Please try again later."}}
end
end

# This function changes the is_grading_published field of the submission to true
# only if all the answers are graded
# and sends a notification to the student
def publish_grading(submission_id, cr = %CourseRegistration{id: course_reg_id, role: role})
when is_ecto_id(submission_id) do
submission =
Submission
|> join(:inner, [s], a in assoc(s, :assessment))
|> preload([_, a], assessment: a)
|> Repo.get(submission_id)

# allows staff to publish own assessment
bypass = role in @bypass_closed_roles and submission.student_id == course_reg_id

with {:submission_found?, true} <- {:submission_found?, is_map(submission)},
{:status, :submitted} <- {:status, submission.status},
{:allowed_to_publish?, true} <-
{:allowed_to_publish?,
role == :admin or bypass or
Cadet.Accounts.Query.avenger_of?(cr, submission.student_id)} do
submission
|> Submission.changeset(%{is_grading_published: true})
|> Repo.update()

Notifications.write_notification_when_graded(
submission.id,
:graded
)

{:ok, nil}
else
{:submission_found?, false} ->
{:error, {:not_found, "Submission not found"}}

{:status, :attempting} ->
{:error, {:bad_request, "Some questions have not been attempted"}}

{:status, :attempted} ->
{:error, {:bad_request, "Assessment has not been submitted"}}

{:allowed_to_publish?, false} ->
{:error, {:forbidden, "Only Avenger of student or Admin is permitted to publish grading"}}

_ ->
{:error, {:internal_server_error, "Please try again later."}}
end
Expand Down Expand Up @@ -1401,6 +1554,7 @@
unsubmitted_by_id: s.unsubmitted_by_id,
student_id: s.student_id,
assessment_id: s.assessment_id,
is_grading_published: s.is_grading_published,
xp: ans.xp,
xp_adjustment: ans.xp_adjustment,
graded_count: ans.graded_count,
Expand Down Expand Up @@ -1456,6 +1610,9 @@
{"notFullyGraded", "true"}, dynamic ->
dynamic([ans: ans, asst: asst], ^dynamic and asst.question_count > ans.graded_count)

{"notPublished", "true"}, dynamic ->
dynamic([submission], ^dynamic and not submission.is_grading_published)

{_, _}, dynamic ->
dynamic
end)
Expand Down Expand Up @@ -1625,7 +1782,7 @@
end
end

defp is_fully_graded?(%Answer{submission_id: submission_id}) do

Check warning on line 1785 in lib/cadet/assessments/assessments.ex

View workflow job for this annotation

GitHub Actions / Run CI

function is_fully_graded?/1 is unused
submission =
Submission
|> Repo.get_by(id: submission_id)
Expand Down Expand Up @@ -1681,12 +1838,7 @@
{:valid, changeset = %Ecto.Changeset{valid?: true}} <-
{:valid, Answer.grading_changeset(answer, attrs)},
{:ok, _} <- Repo.update(changeset) do
if is_fully_graded?(answer) and not is_own_submission do
# Every answer in this submission has been graded manually
Notifications.write_notification_when_graded(submission_id, :graded)
else
{:ok, nil}
end
{:ok, nil}
else
{:answer_found?, false} ->
{:error, {:bad_request, "Answer not found or user not permitted to grade."}}
Expand Down
3 changes: 2 additions & 1 deletion lib/cadet/assessments/submission.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ defmodule Cadet.Assessments.Submission do
field(:graded_count, :integer, virtual: true, default: 0)
field(:grading_status, :string, virtual: true)
field(:unsubmitted_at, :utc_datetime_usec)
field(:is_grading_published, :boolean, default: false)

belongs_to(:assessment, Assessment)
belongs_to(:student, CourseRegistration)
Expand All @@ -26,7 +27,7 @@ defmodule Cadet.Assessments.Submission do
timestamps()
end

@required_fields ~w(student_id assessment_id status)a
@required_fields ~w(student_id assessment_id status is_grading_published)a
@optional_fields ~w(xp_bonus unsubmitted_by_id unsubmitted_at)a

def changeset(submission, params) do
Expand Down
2 changes: 1 addition & 1 deletion lib/cadet/devices/devices.ex
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ defmodule Cadet.Devices do
},
300,
[],
''
~c""
YaleChen299 marked this conversation as resolved.
Show resolved Hide resolved
)

# ExAws includes the session token in the signed payload and doesn't allow
Expand Down
2 changes: 1 addition & 1 deletion lib/cadet_web/admin_controllers/admin_assets_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ defmodule CadetWeb.AdminAssetsController do

case Assets.delete_object(Courses.assets_prefix(course_reg.course), foldername, filename) do
{:error, {status, message}} -> conn |> put_status(status) |> text(message)
_ -> conn |> put_status(204) |> text('')
_ -> conn |> put_status(204) |> text(~c"")
YaleChen299 marked this conversation as resolved.
Show resolved Hide resolved
end
end

Expand Down
6 changes: 5 additions & 1 deletion lib/cadet_web/admin_controllers/admin_courses_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ defmodule CadetWeb.AdminCoursesController do
title("AdminSublanguage")

properties do
chapter(:integer, "Chapter number from 1 to 4", required: true, minimum: 1, maximum: 4)
chapter(:integer, "Chapter number from 1 to 4",
required: true,
minimum: 1,
maximum: 4
)

variant(Schema.ref(:SourceVariant), "Variant name", required: true)
end
Expand Down
30 changes: 30 additions & 0 deletions lib/cadet_web/admin_controllers/admin_grading_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,34 @@ defmodule CadetWeb.AdminGradingController do
end
end

def unpublish_grades(conn, %{"submissionid" => submission_id}) when is_ecto_id(submission_id) do
GabrielCWT marked this conversation as resolved.
Show resolved Hide resolved
course_reg = conn.assigns[:course_reg]

case Assessments.unpublish_grading(submission_id, course_reg) do
{:ok, nil} ->
text(conn, "OK")

{:error, {status, message}} ->
conn
|> put_status(status)
|> text(message)
end
end

def publish_grades(conn, %{"submissionid" => submission_id}) when is_ecto_id(submission_id) do
course_reg = conn.assigns[:course_reg]

case Assessments.publish_grading(submission_id, course_reg) do
{:ok, nil} ->
text(conn, "OK")

{:error, {status, message}} ->
conn
|> put_status(status)
|> text(message)
end
end

def autograde_submission(conn, %{"submissionid" => submission_id}) do
course_reg = conn.assigns[:course_reg]

Expand Down Expand Up @@ -276,6 +304,8 @@ defmodule CadetWeb.AdminGradingController do

unsubmittedBy(Schema.ref(:GraderInfo))
unsubmittedAt(:string, "Last unsubmitted at", format: "date-time", required: false)

isGradingPublished(:boolean, "Whether the grading is published", required: true)
end
end,
AssessmentInfo:
Expand Down
Loading
Loading