E2612 - course based reports#333
Open
kamatmihir2002 wants to merge 9 commits into
Open
Conversation
Generated by 🚫 Danger |
kamatmihir2002
changed the title
Add seed data
Add report controller assignment_records_controller
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)app/controllers/**/*.rb⚙️ CodeRabbit configuration file
Files:
🪛 RuboCop (1.86.1)app/controllers/course_reports_controller.rb[convention] 3-158: Class has too many lines. [109/100] (Metrics/ClassLength) [convention] 22-41: Assignment Branch Condition size for (Metrics/AbcSize) [convention] 29-29: Align (Layout/MultilineMethodCallIndentation) [convention] 30-30: Align (Layout/MultilineMethodCallIndentation) [convention] 33-33: Align (Layout/MultilineMethodCallIndentation) [convention] 34-34: Align (Layout/MultilineMethodCallIndentation) [convention] 58-58: Avoid the use of double negation ( (Style/DoubleNegation) [convention] 85-85: Trailing whitespace detected. (Layout/TrailingWhitespace) [convention] 86-86: Missing space after (Layout/LeadingCommentSpace) [convention] 129-129: Line is too long. [121/120] (Layout/LineLength) [convention] 130-130: Line is too long. [127/120] (Layout/LineLength) 🔇 Additional comments (2)
WalkthroughAdds a Course Reports API endpoint and route; implements controller logic to authorize access, order assignments by final review review-deadline, build a student-by-assignment JSON matrix (participant ids, peer/instructor grades, teammate and author-feedback averages, optional topic names), extends seeds to generate report data, and adds request specs. ChangesCourse Reports feature
🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 9 | ❌ 5❌ Failed checks (5 warnings)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
spec/requests/api/v1/course_reports_controller_spec.rb (2)
325-328: Remove debug output from the success-path spec.Line 327 prints full response payload on every run, which adds log noise and slows CI output parsing.
Suggested cleanup
run_test! do |response| data = JSON.parse(response.body) - puts "\nCourse report stats:\n#{JSON.pretty_generate(data)}" expect(data['course_id']).to eq(course.id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/requests/api/v1/course_reports_controller_spec.rb` around lines 325 - 328, The spec's success-path prints the entire response payload; remove the debug output by deleting the puts "\nCourse report stats:\n#{JSON.pretty_generate(data)}" inside the run_test! block (the code that parses the response with JSON.parse(response.body) and assigns data can remain if used), or replace it with a conditional debug log gated by an ENV flag if you want optional output; update the run_test! block accordingly so it no longer always emits the payload to STDOUT.
6-311: Split this spec into setup helpers/shared contexts.The current single-block fixture graph and assertion chain is hard to debug when one expectation fails; extracting builders and shared examples will make failures more localized and maintenance easier.
Also applies to: 313-500
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/requests/api/v1/course_reports_controller_spec.rb` around lines 6 - 311, Split the large fixture block into reusable setup helpers/shared_contexts: extract role and user creation (create_roles_hierarchy, instructor, student, teammate, reviewer_one, etc.) into one shared helper, assignment and topic setup (assignment, assignment2, topic, signed_up_team) into another, team/participant builders (team, team2, participant..participant15, team.add_member calls) into a team/participant helper, and response/answer builders (review_map1..7, review_response1..7, review_answer1..7, feedback_map1..4, feedback_response1..4, feedback_answer1..4, teammate_review_map1..4, teammate_response1..4, teammate_answer1..4) into a response helper; then replace the giant before/let! block in spec/requests/api/v1/course_reports_controller_spec.rb with includes of those shared_contexts or calls to the new helper methods, using FactoryBot or helper methods returning created objects where appropriate (referencing symbols: create_roles_hierarchy, instructor_token, Authorization, assignment_questionnaire1/2, AssignmentDueDate objects) so each context/test only sets up the minimal fixtures it needs and failures are localized.app/controllers/course_reports_controller.rb (1)
38-40: Prefer a 4xx status for invalid assignment deadline state.Returning
500here signals a server fault, but this is a domain/data validation condition (final deadline type mismatch). A422response would better represent the failure mode.Also applies to: 69-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/course_reports_controller.rb` around lines 38 - 40, Change the HTTP status for the domain validation error rescues from a server error to an unprocessable entity: replace render json: { error: e.message }, status: :internal_server_error with status: :unprocessable_entity in the rescue blocks that catch FinalDueDateNotReviewDeadlineError (and the other identical rescue around lines 69-71) so the controller returns 422 for the invalid assignment deadline state rather than 500.db/seeds.rb (1)
61-63: Make course-report seed generation deterministic.Using
.samplehere makes report fixtures vary between runs, which hurts reproducibility when debugging report behavior.Deterministic option
- topic_assignment_indexes = (0...num_assignments).to_a.sample(num_assignments / 2) + seeded_rng = Random.new(2612) + topic_assignment_indexes = (0...num_assignments).to_a.sample(num_assignments / 2, random: seeded_rng) ... - project_topic_id: assignment_topics[assignment_id].sample.id, + project_topic_id: assignment_topics[assignment_id].sample(random: seeded_rng).id, ... - SignedUpTeam.find_or_create_by!(team_id: team.id, project_topic_id: existing_signup&.project_topic_id || topics.sample.id) do |signup| + SignedUpTeam.find_or_create_by!( + team_id: team.id, + project_topic_id: existing_signup&.project_topic_id || topics.sample(random: seeded_rng).id + ) do |signup|Also applies to: 115-116, 342-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/seeds.rb` around lines 61 - 63, The use of Array#sample makes topic assignment nondeterministic; replace those calls (e.g., where topic_assignment_indexes is created and the similar occurrences around the other mentioned spots) with a deterministic sampling using a seeded Random instance (e.g., create rng = Random.new(SEED) and pass it to sample via the random: rng option or call rng.sample on the range/array) so the same seed yields identical fixtures across runs; update the three locations (topic_assignment_indexes creation and the two other .sample uses at the referenced spots) to use the same fixed seed or a configurable seed variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/course_reports_controller.rb`:
- Around line 100-131: build_assignment_cell performs per-row DB calls
(participant.team, teammate_review_maps_for, author_feedback_maps_for,
topic_name_for) causing N+1s; preload all needed data once and replace those
lookups with in-memory lookups. Specifically, before iterating
assignments/participants load participants' teams and teams_participants,
SignedUpTeam records, TeammateReviewResponseMap rows (filter by assignment.id
and participant.ids), ReviewResponseMap rows (filter by assignment.id and
reviewer_ids), and FeedbackResponseMap rows (filter by review_map ids), then
update build_assignment_cell to read from these preloaded hashes/relations
instead of calling participant.team, TeamsParticipant.find_by,
TeammateReviewResponseMap.where, ReviewResponseMap.where and
FeedbackResponseMap.where so that topic_name_for, teammate_review_maps_for and
author_feedback_maps_for use the in-memory collections.
In `@config/routes.rb`:
- Around line 157-166: Fix the layout issues in the participants routes block by
normalizing indentation and removing trailing whitespace: align the nested
collection do ... end block consistently with the resources :participants line
and ensure each route line (e.g., get '/user/:user_id' ->
'participants#list_user_participants', get '/assignment/:assignment_id' ->
'participants#list_assignment_participants', get '/:id' -> 'participants#show',
post '/:authorization' -> 'participants#add', patch '/:id/:authorization' ->
'participants#update_authorization', delete '/:id' -> 'participants#destroy')
has no trailing spaces and uses two-space indentation (or the project's
standard) so RuboCop layout cops no longer fail.
In `@db/seeds.rb`:
- Line 334: The block passed to teams_with_members.each_with_index introduces an
unused block argument team_index; rename it to a discard variable (e.g.,
_team_index) or remove it to satisfy the linter and signal intent. Edit the
block signature for teams_with_members.each_with_index do |(team,
_team_participants), _team_index| (or simply drop the index if not needed) so
the unused index is clearly ignored.
---
Nitpick comments:
In `@app/controllers/course_reports_controller.rb`:
- Around line 38-40: Change the HTTP status for the domain validation error
rescues from a server error to an unprocessable entity: replace render json: {
error: e.message }, status: :internal_server_error with status:
:unprocessable_entity in the rescue blocks that catch
FinalDueDateNotReviewDeadlineError (and the other identical rescue around lines
69-71) so the controller returns 422 for the invalid assignment deadline state
rather than 500.
In `@db/seeds.rb`:
- Around line 61-63: The use of Array#sample makes topic assignment
nondeterministic; replace those calls (e.g., where topic_assignment_indexes is
created and the similar occurrences around the other mentioned spots) with a
deterministic sampling using a seeded Random instance (e.g., create rng =
Random.new(SEED) and pass it to sample via the random: rng option or call
rng.sample on the range/array) so the same seed yields identical fixtures across
runs; update the three locations (topic_assignment_indexes creation and the two
other .sample uses at the referenced spots) to use the same fixed seed or a
configurable seed variable.
In `@spec/requests/api/v1/course_reports_controller_spec.rb`:
- Around line 325-328: The spec's success-path prints the entire response
payload; remove the debug output by deleting the puts "\nCourse report
stats:\n#{JSON.pretty_generate(data)}" inside the run_test! block (the code that
parses the response with JSON.parse(response.body) and assigns data can remain
if used), or replace it with a conditional debug log gated by an ENV flag if you
want optional output; update the run_test! block accordingly so it no longer
always emits the payload to STDOUT.
- Around line 6-311: Split the large fixture block into reusable setup
helpers/shared_contexts: extract role and user creation (create_roles_hierarchy,
instructor, student, teammate, reviewer_one, etc.) into one shared helper,
assignment and topic setup (assignment, assignment2, topic, signed_up_team) into
another, team/participant builders (team, team2, participant..participant15,
team.add_member calls) into a team/participant helper, and response/answer
builders (review_map1..7, review_response1..7, review_answer1..7,
feedback_map1..4, feedback_response1..4, feedback_answer1..4,
teammate_review_map1..4, teammate_response1..4, teammate_answer1..4) into a
response helper; then replace the giant before/let! block in
spec/requests/api/v1/course_reports_controller_spec.rb with includes of those
shared_contexts or calls to the new helper methods, using FactoryBot or helper
methods returning created objects where appropriate (referencing symbols:
create_roles_hierarchy, instructor_token, Authorization,
assignment_questionnaire1/2, AssignmentDueDate objects) so each context/test
only sets up the minimal fixtures it needs and failures are localized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7699424-963b-4765-b70b-6656a61e461e
📒 Files selected for processing (4)
app/controllers/course_reports_controller.rbconfig/routes.rbdb/seeds.rbspec/requests/api/v1/course_reports_controller_spec.rb
Comment on lines
+100
to
+131
| def build_assignment_cell(assignment, participant) | ||
| team = participant.team | ||
|
|
||
| { | ||
| participant_id: participant.id, | ||
| peer_grade: team&.aggregate_review_grade, | ||
| instructor_grade: team&.grade_for_submission, | ||
| avg_teammate_score: participant.aggregate_teammate_review_grade(teammate_review_maps_for(assignment, participant)), | ||
| avg_author_feedback_score: participant.aggregate_teammate_review_grade(author_feedback_maps_for(assignment, participant)) | ||
| }.tap do |cell| | ||
| cell[:topic] = topic_name_for(assignment, participant) if assignment.has_topics | ||
| end | ||
| end | ||
|
|
||
| def topic_name_for(assignment, participant) | ||
| return unless assignment.has_topics | ||
|
|
||
| team_id = TeamsParticipant.find_by(participant_id: participant.id)&.team_id | ||
| return unless team_id | ||
|
|
||
| SignedUpTeam.find_by(team_id: team_id)&.project_topic&.topic_name | ||
| end | ||
|
|
||
| def teammate_review_maps_for(assignment, participant) | ||
| TeammateReviewResponseMap.where(reviewed_object_id: assignment.id, reviewee_id: participant.id) | ||
| end | ||
|
|
||
| def author_feedback_maps_for(assignment, participant) | ||
| review_maps = ReviewResponseMap.where(reviewed_object_id: assignment.id, reviewer_id: participant.id) | ||
|
|
||
| FeedbackResponseMap.where(reviewed_object_id: review_maps.select(:id), reviewee_id: participant.id) | ||
| end |
There was a problem hiding this comment.
This report builder currently has an N+1 query cascade.
At Line 100-Line 131, each assignment cell triggers additional DB calls (participant.team, teammate maps, author feedback maps, topic lookup). On real courses, this will degrade badly.
Refactor direction (preload once, then lookup in-memory)
def index
course = Course.find_by(id: params[:course_id])
return render json: { error: 'Course not found' }, status: :not_found unless course
assignments = assignments_ordered_by_final_review_due_date(course)
assignment_ids = assignments.map(&:id)
- participants = AssignmentParticipant
- .includes(:user, :assignment)
+ participants = AssignmentParticipant
+ .includes(:user, :assignment)
.where(parent_id: assignment_ids)
+
+ participant_ids = participants.map(&:id)
+ teammate_maps = TeammateReviewResponseMap.where(reviewed_object_id: assignment_ids, reviewee_id: participant_ids)
+ .group_by { |m| [m.reviewed_object_id, m.reviewee_id] }
+ review_maps = ReviewResponseMap.where(reviewed_object_id: assignment_ids, reviewer_id: participant_ids)
+ feedback_maps = FeedbackResponseMap.where(reviewed_object_id: review_maps.select(:id), reviewee_id: participant_ids)
+ .group_by(&:reviewee_id)
student_rows = participants
.group_by(&:user_id)
.values
- .map { |student_participants| build_student_row(assignments, student_participants) }
+ .map { |student_participants| build_student_row(assignments, student_participants, teammate_maps, feedback_maps) }
.sort_by { |row| row[:user_name].downcase }🧰 Tools
🪛 RuboCop (1.86.1)
[convention] 107-107: Line is too long. [121/120]
(Layout/LineLength)
[convention] 108-108: Line is too long. [127/120]
(Layout/LineLength)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/controllers/course_reports_controller.rb` around lines 100 - 131,
build_assignment_cell performs per-row DB calls (participant.team,
teammate_review_maps_for, author_feedback_maps_for, topic_name_for) causing
N+1s; preload all needed data once and replace those lookups with in-memory
lookups. Specifically, before iterating assignments/participants load
participants' teams and teams_participants, SignedUpTeam records,
TeammateReviewResponseMap rows (filter by assignment.id and participant.ids),
ReviewResponseMap rows (filter by assignment.id and reviewer_ids), and
FeedbackResponseMap rows (filter by review_map ids), then update
build_assignment_cell to read from these preloaded hashes/relations instead of
calling participant.team, TeamsParticipant.find_by,
TeammateReviewResponseMap.where, ReviewResponseMap.where and
FeedbackResponseMap.where so that topic_name_for, teammate_review_maps_for and
author_feedback_maps_for use the in-memory collections.
Comment on lines
+157
to
+166
| resources :participants do | ||
| collection do | ||
| get '/user/:user_id', to: 'participants#list_user_participants' | ||
| get '/assignment/:assignment_id', to: 'participants#list_assignment_participants' | ||
| get '/:id', to: 'participants#show' | ||
| post '/:authorization', to: 'participants#add' | ||
| patch '/:id/:authorization', to: 'participants#update_authorization' | ||
| delete '/:id', to: 'participants#destroy' | ||
| end | ||
| end | ||
|
|
||
| resources :student_teams, only: %i[create update] do | ||
| collection do | ||
| get :view | ||
| get :mentor | ||
| delete '/:id', to: 'participants#destroy' | ||
| end | ||
| end |
There was a problem hiding this comment.
Fix indentation/trailing whitespace in the new route blocks.
Line 157-Line 166, Line 168, and Line 172 currently trigger RuboCop layout cops; this can fail lint gates.
Suggested cleanup
- resources :participants do
- collection do
- get '/user/:user_id', to: 'participants#list_user_participants'
- get '/assignment/:assignment_id', to: 'participants#list_assignment_participants'
- get '/:id', to: 'participants#show'
+ resources :participants do
+ collection do
+ get '/user/:user_id', to: 'participants#list_user_participants'
+ get '/assignment/:assignment_id', to: 'participants#list_assignment_participants'
+ get '/:id', to: 'participants#show'
post '/:authorization', to: 'participants#add'
patch '/:id/:authorization', to: 'participants#update_authorization'
- delete '/:id', to: 'participants#destroy'
- end
- end
+ delete '/:id', to: 'participants#destroy'
+ end
+ end
- resources :course_reports, only: [:index]
+ resources :course_reports, only: [:index]
- resources :student_teams, only: %i[create update] do
- collection do
- get :view
- get :mentor
+ resources :student_teams, only: %i[create update] do
+ collection do
+ get :view
+ get :mentorAlso applies to: 170-173
🧰 Tools
🪛 RuboCop (1.86.1)
[convention] 157-166: Inconsistent indentation detected.
(Layout/IndentationConsistency)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/routes.rb` around lines 157 - 166, Fix the layout issues in the
participants routes block by normalizing indentation and removing trailing
whitespace: align the nested collection do ... end block consistently with the
resources :participants line and ensure each route line (e.g., get
'/user/:user_id' -> 'participants#list_user_participants', get
'/assignment/:assignment_id' -> 'participants#list_assignment_participants', get
'/:id' -> 'participants#show', post '/:authorization' -> 'participants#add',
patch '/:id/:authorization' -> 'participants#update_authorization', delete
'/:id' -> 'participants#destroy') has no trailing spaces and uses two-space
indentation (or the project's standard) so RuboCop layout cops no longer fail.
|
|
||
| teams_with_members = existing_teams_with_members + seeded_teams_with_members | ||
|
|
||
| teams_with_members.each_with_index do |(team, _team_participants), team_index| |
There was a problem hiding this comment.
Rename unused block arg to satisfy lint and intent.
Line 334 declares team_index but never uses it.
Suggested fix
- teams_with_members.each_with_index do |(team, _team_participants), team_index|
+ teams_with_members.each_with_index do |(team, _team_participants), _team_index|📝 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
| teams_with_members.each_with_index do |(team, _team_participants), team_index| | |
| teams_with_members.each_with_index do |(team, _team_participants), _team_index| |
🧰 Tools
🪛 RuboCop (1.86.1)
[warning] 334-334: Unused block argument - team_index. If it's necessary, use _ or _team_index as an argument name to indicate that it won't be used.
(Lint/UnusedBlockArgument)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/seeds.rb` at line 334, The block passed to
teams_with_members.each_with_index introduces an unused block argument
team_index; rename it to a discard variable (e.g., _team_index) or remove it to
satisfy the linter and signal intent. Edit the block signature for
teams_with_members.each_with_index do |(team, _team_participants), _team_index|
(or simply drop the index if not needed) so the unused index is clearly ignored.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/course_reports_controller.rb`:
- Around line 71-80: final_review_due_date_for currently picks the latest due
date across all types then checks its type, causing a 500 if a later non-review
deadline exists; update final_review_due_date_for to first filter
assignment.due_dates for entries where deadline_type_id ==
DueDate::REVIEW_DEADLINE_TYPE_ID (e.g., using select or where on the
association), then call max_by(&:due_at) on that filtered collection, return the
due_at if found, and only raise FinalDueDateNotReviewDeadlineError when no
review-type due dates are present; reference the method
final_review_due_date_for, the DueDate::REVIEW_DEADLINE_TYPE_ID constant, and
the raise site to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5546b62b-6ce8-4774-80f5-169b2640be5f
📒 Files selected for processing (1)
app/controllers/course_reports_controller.rb
Comment on lines
+71
to
+80
| def final_review_due_date_for(assignment) | ||
| final_due_date = assignment.due_dates.max_by(&:due_at) | ||
| return final_due_date.due_at if final_due_date&.deadline_type_id == DueDate::REVIEW_DEADLINE_TYPE_ID | ||
|
|
||
| # At this point of the project, all assignments are peer review assignments, | ||
| # so the final deadline is bound to be a review deadline, hence this guard | ||
| # | ||
| #Replace this with code in the incident that non peer review assignments are introduced | ||
| raise FinalDueDateNotReviewDeadlineError, | ||
| "Final due date for assignment #{assignment.id} is not a review deadline" |
There was a problem hiding this comment.
Filter by review-deadline type before picking the “final” review deadline.
At Line 72, max_by(&:due_at) runs across all due dates. If a later non-review deadline exists, Line 73 fails the type check and the endpoint returns 500 even when valid review deadlines are present.
Proposed fix
def final_review_due_date_for(assignment)
- final_due_date = assignment.due_dates.max_by(&:due_at)
- return final_due_date.due_at if final_due_date&.deadline_type_id == DueDate::REVIEW_DEADLINE_TYPE_ID
+ final_review_due_date = assignment.due_dates
+ .select { |d| d.deadline_type_id == DueDate::REVIEW_DEADLINE_TYPE_ID }
+ .max_by(&:due_at)
+ return final_review_due_date.due_at if final_review_due_date
# At this point of the project, all assignments are peer review assignments,
# so the final deadline is bound to be a review deadline, hence this guard
#
`#Replace` this with code in the incident that non peer review assignments are introduced
raise FinalDueDateNotReviewDeadlineError,
"Final due date for assignment #{assignment.id} is not a review deadline"
end🧰 Tools
🪛 RuboCop (1.86.1)
[convention] 77-77: Trailing whitespace detected.
(Layout/TrailingWhitespace)
[convention] 78-78: Missing space after #.
(Layout/LeadingCommentSpace)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/controllers/course_reports_controller.rb` around lines 71 - 80,
final_review_due_date_for currently picks the latest due date across all types
then checks its type, causing a 500 if a later non-review deadline exists;
update final_review_due_date_for to first filter assignment.due_dates for
entries where deadline_type_id == DueDate::REVIEW_DEADLINE_TYPE_ID (e.g., using
select or where on the association), then call max_by(&:due_at) on that filtered
collection, return the due_at if found, and only raise
FinalDueDateNotReviewDeadlineError when no review-type due dates are present;
reference the method final_review_due_date_for, the
DueDate::REVIEW_DEADLINE_TYPE_ID constant, and the raise site to implement this
change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements the backend part of the topic E2612 (course reports) for the CSC517 Final Project. We have started with an empty initial commit to signify the start of this PR commit history. This will be bookended by an empty commit signifying the end of this specific feature's commit history once this feature is ready.
This aims to implement a single controller that exposes endpoints to provide the data to be viewed in the course reports frontend.
New Features
Tests
Chores / Seed data
Files touched
Review / policy notes (legacy Expertiza Danger policy + Rails review quality)
Recommendations before merge