Skip to content

Migrate filtering to the Backend #1065

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

Merged
merged 33 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
29246b0
feat: implement paginated submissions query function
GabrielCWT Jan 28, 2024
a60e39f
feat: implement /grading route accepting pagination query parameters
GabrielCWT Jan 28, 2024
29057e7
fix: fix paginated submissions with group filter
GabrielCWT Jan 30, 2024
c662fbc
refactor: change page to offset for pagination
GabrielCWT Jan 31, 2024
6adbf8c
feat: Update admin grading controller to take variable amount of params
GabrielCWT Feb 9, 2024
3b621af
refactor: Refactor code from using raw sql to using ecto query
GabrielCWT Feb 11, 2024
fc55c55
feat: Implement filtering by mission name
GabrielCWT Feb 11, 2024
610ffbf
feat: Implement filtering by progress for submission
GabrielCWT Feb 11, 2024
7de9882
refactor: show all and show only grader's groups now use ecto query
GabrielCWT Feb 11, 2024
c9ba93e
feat: Implement filter by group id
GabrielCWT Feb 11, 2024
451cea8
fix: Fix limit and offset default for missing query param
GabrielCWT Feb 11, 2024
940e282
refactor: Rename group filter to course registration filter
GabrielCWT Feb 11, 2024
2f5c910
feat: Implement filter by name
GabrielCWT Feb 11, 2024
ca768aa
feat: Implement filter by assessment type
GabrielCWT Feb 11, 2024
9ff4108
Implement filter by is_manually_graded
GabrielCWT Feb 12, 2024
8afcc22
feat: Include total count of submissions in response
GabrielCWT Feb 13, 2024
c7dc2ca
refactor: Use one join query for assessment config queries
GabrielCWT Feb 14, 2024
2e9b1ea
refactor: Use one join query for user queries
GabrielCWT Feb 14, 2024
70b7f59
refactor: Move queries for assessment and assessment config to builder
GabrielCWT Feb 15, 2024
f5f651d
fix: Update admin_grading_controller test to account for change in re…
GabrielCWT Feb 15, 2024
37c7a4e
refactor: Rename function
GabrielCWT Feb 15, 2024
b7ea15b
docs: Include a description of the function
GabrielCWT Feb 15, 2024
0fd3e65
refactor: Format code
GabrielCWT Feb 15, 2024
bac36b4
Merge branch 'master' into gabriel/pagination
GabrielCWT Feb 15, 2024
00e48f2
fix: Update filter by groupID to groupName
GabrielCWT Feb 17, 2024
80e2f5e
feat: Assessment title filter using LIKE
GabrielCWT Feb 18, 2024
b1a249c
fix: Change Notification/Email tests to use the new response
GabrielCWT Feb 18, 2024
82b2a10
refactor: Remove old code for submissions query
GabrielCWT Feb 18, 2024
2e3be04
feat: Implement notFullyGraded feature
GabrielCWT Feb 21, 2024
2ea2f55
refactor: Remove TODO comment for what's now known as notFullyGraded
GabrielCWT Feb 21, 2024
4d2f3b2
refactor: Change param name for notification worker to filter notFull…
GabrielCWT Feb 22, 2024
ed907e5
Merge branch 'master' into gabriel/pagination
RichDom2185 Feb 23, 2024
fa2cb80
Remove commented code
RichDom2185 Feb 23, 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
297 changes: 217 additions & 80 deletions lib/cadet/assessments/assessments.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1230,92 +1230,229 @@ defmodule Cadet.Assessments do

@doc """
Function returning submissions under a grader. This function returns only the
fields that are exposed in the /grading endpoint. The reason we select only
those fields is to reduce the memory usage especially when the number of
submissions is large i.e. > 25000 submissions.
fields that are exposed in the /grading endpoint.

The input parameters are the user and group_only. group_only is used to check
whether only the groups under the grader should be returned. The parameter is
a boolean which is false by default.
The input parameters are the user and query parameters. Query parameters are
used to filter the submissions.

The return value is {:ok, submissions} if no errors, else it is {:error,
{:forbidden, "Forbidden."}}
The group parameter is used to check whether only the groups under the grader
should be returned. If pageSize and offset are not provided, the default
values are 10 and 0 respectively.

The return value is
{:ok, %{"count": count, "data": submissions}} if no errors,
else it is {:error, {:forbidden, "Forbidden."}}
"""
@spec all_submissions_by_grader_for_index(CourseRegistration.t()) ::
{:ok, %{:assessments => [any()], :submissions => [any()], :users => [any()]}}
def all_submissions_by_grader_for_index(

# We bypass Ecto here and use a raw query to generate JSON directly from
# PostgreSQL, because doing it in Elixir/Erlang is too inefficient.
@spec submissions_by_grader_for_index(CourseRegistration.t()) ::
{:ok,
%{
:count => integer,
:data => %{:assessments => [any()], :submissions => [any()], :users => [any()]}
}}
def submissions_by_grader_for_index(
grader = %CourseRegistration{course_id: course_id},
group_only \\ false,
_ungraded_only \\ false
params \\ %{
"group" => "false",
"notFullyGraded" => "true",
"pageSize" => "10",
"offset" => "0"
}
) do
show_all = not group_only

group_filter =
if show_all,
do: "",
else:
"AND s.student_id IN (SELECT cr.id FROM course_registrations AS cr INNER JOIN groups AS g ON cr.group_id = g.id WHERE g.leader_id = #{grader.id}) OR s.student_id = #{grader.id}"

# TODO: Restore ungraded filtering
# ... or more likely, decouple email logic from this function
# ungraded_where =
# if ungraded_only,
# do: "where s.\"gradedCount\" < assts.\"questionCount\"",
# else: ""

# We bypass Ecto here and use a raw query to generate JSON directly from
# PostgreSQL, because doing it in Elixir/Erlang is too inefficient.

submissions =
case Repo.query("""
SELECT
s.id,
s.status,
s.unsubmitted_at,
s.unsubmitted_by_id,
s_ans.xp,
s_ans.xp_adjustment,
s.xp_bonus,
s_ans.graded_count,
s.student_id,
s.assessment_id
FROM
submissions AS s
LEFT JOIN (
SELECT
ans.submission_id,
SUM(ans.xp) AS xp,
SUM(ans.xp_adjustment) AS xp_adjustment,
COUNT(ans.id) FILTER (
WHERE
ans.grader_id IS NOT NULL
) AS graded_count
FROM
answers AS ans
GROUP BY
ans.submission_id
) AS s_ans ON s_ans.submission_id = s.id
WHERE
s.assessment_id IN (
SELECT
id
FROM
assessments
WHERE
assessments.course_id = #{course_id}
) #{group_filter};
""") do
{:ok, %{columns: columns, rows: result}} ->
result
|> Enum.map(
&(columns
|> Enum.map(fn c -> String.to_atom(c) end)
|> Enum.zip(&1)
|> Enum.into(%{}))
)
end
submission_answers_query =
from(ans in Answer,
group_by: ans.submission_id,
select: %{
submission_id: ans.submission_id,
xp: sum(ans.xp),
xp_adjustment: sum(ans.xp_adjustment),
graded_count: filter(count(ans.id), not is_nil(ans.grader_id))
}
)

question_answers_query =
from(q in Question,
group_by: q.assessment_id,
join: a in Assessment,
on: q.assessment_id == a.id,
select: %{
assessment_id: q.assessment_id,
question_count: count(q.id)
}
)

query =
from(s in Submission,
left_join: ans in subquery(submission_answers_query),
on: ans.submission_id == s.id,
as: :ans,
left_join: asst in subquery(question_answers_query),
on: asst.assessment_id == s.assessment_id,
as: :asst,
where: ^build_user_filter(params),
where: s.assessment_id in subquery(build_assessment_filter(params, course_id)),
where: s.assessment_id in subquery(build_assessment_config_filter(params)),
where: ^build_submission_filter(params),
where: ^build_course_registration_filter(params, grader),
order_by: [desc: s.inserted_at],
limit: ^elem(Integer.parse(Map.get(params, "pageSize", "10")), 0),
offset: ^elem(Integer.parse(Map.get(params, "offset", "0")), 0),
select: %{
id: s.id,
status: s.status,
xp_bonus: s.xp_bonus,
unsubmitted_at: s.unsubmitted_at,
unsubmitted_by_id: s.unsubmitted_by_id,
student_id: s.student_id,
assessment_id: s.assessment_id,
xp: ans.xp,
xp_adjustment: ans.xp_adjustment,
graded_count: ans.graded_count,
question_count: asst.question_count
}
)

submissions = Repo.all(query)

count_query =
from(s in Submission,
left_join: ans in subquery(submission_answers_query),
on: ans.submission_id == s.id,
as: :ans,
left_join: asst in subquery(question_answers_query),
on: asst.assessment_id == s.assessment_id,
as: :asst,
where: s.assessment_id in subquery(build_assessment_filter(params, course_id)),
where: s.assessment_id in subquery(build_assessment_config_filter(params)),
where: ^build_user_filter(params),
where: ^build_submission_filter(params),
where: ^build_course_registration_filter(params, grader),
select: count(s.id)
)

count = Repo.one(count_query)

{:ok, %{count: count, data: generate_grading_summary_view_model(submissions, course_id)}}
end

{:ok, generate_grading_summary_view_model(submissions, course_id)}
defp build_assessment_filter(params, course_id) do
assessments_filters =
Enum.reduce(params, dynamic(true), fn
{"title", value}, dynamic ->
dynamic([assessment], ^dynamic and ilike(assessment.title, ^"%#{value}%"))

{_, _}, dynamic ->
dynamic
end)

from(a in Assessment,
where: a.course_id == ^course_id,
where: ^assessments_filters,
select: a.id
)
end

defp build_submission_filter(params) do
Enum.reduce(params, dynamic(true), fn
{"status", value}, dynamic ->
dynamic([submission], ^dynamic and submission.status == ^value)

{"notFullyGraded", "true"}, dynamic ->
dynamic([ans: ans, asst: asst], ^dynamic and asst.question_count > ans.graded_count)

{_, _}, dynamic ->
dynamic
end)
end

defp build_course_registration_filter(params, grader) do
Enum.reduce(params, dynamic(true), fn
{"group", "true"}, dynamic ->
dynamic(
[submission],
(^dynamic and
submission.student_id in subquery(
from(cr in CourseRegistration,
join: g in Group,
on: cr.group_id == g.id,
where: g.leader_id == ^grader.id,
select: cr.id
)
)) or submission.student_id == ^grader.id
)

{"groupName", value}, dynamic ->
dynamic(
[submission],
^dynamic and
submission.student_id in subquery(
from(cr in CourseRegistration,
join: g in Group,
on: cr.group_id == g.id,
where: g.name == ^value,
select: cr.id
)
)
)

{_, _}, dynamic ->
dynamic
end)
end

defp build_user_filter(params) do
Enum.reduce(params, dynamic(true), fn
{"name", value}, dynamic ->
dynamic(
[submission],
^dynamic and
submission.student_id in subquery(
from(user in User,
where: ilike(user.name, ^"%#{value}%"),
select: user.id
)
)
)

{"username", value}, dynamic ->
dynamic(
[submission],
^dynamic and
submission.student_id in subquery(
from(user in User,
where: ilike(user.username, ^"%#{value}%"),
select: user.id
)
)
)

{_, _}, dynamic ->
dynamic
end)
end

defp build_assessment_config_filter(params) do
assessment_config_filters =
Enum.reduce(params, dynamic(true), fn
{"type", value}, dynamic ->
dynamic([assessment_config: config], ^dynamic and config.type == ^value)

{"isManuallyGraded", value}, dynamic ->
dynamic([assessment_config: config], ^dynamic and config.is_manually_graded == ^value)

{_, _}, dynamic ->
dynamic
end)

from(a in Assessment,
inner_join: config in AssessmentConfig,
on: a.config_id == config.id,
as: :assessment_config,
where: ^assessment_config_filters,
select: a.id
)
end

defp generate_grading_summary_view_model(submissions, course_id) do
Expand Down
7 changes: 5 additions & 2 deletions lib/cadet/workers/NotificationWorker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,11 @@ defmodule Cadet.Workers.NotificationWorker do
for avenger_cr <- avengers_crs do
avenger = Cadet.Accounts.get_user(avenger_cr.user_id)

{:ok, %{submissions: ungraded_submissions}} =
Cadet.Assessments.all_submissions_by_grader_for_index(avenger_cr, true, true)
{:ok, %{data: %{submissions: ungraded_submissions}}} =
Cadet.Assessments.submissions_by_grader_for_index(avenger_cr, %{
"group" => "true",
"notFullyGraded" => "true"
})

if Enum.count(ungraded_submissions) < ungraded_threshold do
IO.puts("[AVENGER_BACKLOG] below threshold!")
Expand Down
7 changes: 3 additions & 4 deletions lib/cadet_web/admin_controllers/admin_grading_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ defmodule CadetWeb.AdminGradingController do

alias Cadet.Assessments

def index(conn, %{"group" => group}) when group in ["true", "false"] do
def index(conn, %{"group" => group} = params)
when group in ["true", "false"] do
course_reg = conn.assigns[:course_reg]

group = String.to_atom(group)

case Assessments.all_submissions_by_grader_for_index(course_reg, group) do
case Assessments.submissions_by_grader_for_index(course_reg, params) do
{:ok, view_model} ->
conn
|> put_status(:ok)
Expand Down
49 changes: 28 additions & 21 deletions lib/cadet_web/admin_views/admin_grading_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,36 @@ defmodule CadetWeb.AdminGradingView do
end

def render("gradingsummaries.json", %{
users: users,
assessments: assessments,
submissions: submissions
count: count,
data: %{
users: users,
assessments: assessments,
submissions: submissions
}
}) do
for submission <- submissions do
user = users |> Enum.find(&(&1.id == submission.student_id))
assessment = assessments |> Enum.find(&(&1.id == submission.assessment_id))
%{
count: count,
data:
for submission <- submissions do
user = users |> Enum.find(&(&1.id == submission.student_id))
assessment = assessments |> Enum.find(&(&1.id == submission.assessment_id))

render(
CadetWeb.AdminGradingView,
"gradingsummary.json",
%{
user: user,
assessment: assessment,
submission: submission,
unsubmitter:
case submission.unsubmitted_by_id do
nil -> nil
_ -> users |> Enum.find(&(&1.id == submission.unsubmitted_by_id))
end
}
)
end
render(
CadetWeb.AdminGradingView,
"gradingsummary.json",
%{
user: user,
assessment: assessment,
submission: submission,
unsubmitter:
case submission.unsubmitted_by_id do
nil -> nil
_ -> users |> Enum.find(&(&1.id == submission.unsubmitted_by_id))
end
}
)
end
}
end

def render("gradingsummary.json", %{
Expand Down
Loading