E2550: reimplement responses hierarchy responses controller back end#292
E2550: reimplement responses hierarchy responses controller back end#292bhandariprerak wants to merge 63 commits intoexpertiza:mainfrom
Conversation
…e-mapping Response Type Mapping
…bestinlalu#9, bestinlalu#20] Implement ResponseController class with all APIs and testing done using rspec
[bestinlalu#4, bestinlalu#5, bestinlalu#6, bestinlalu#7, bestinlalu#8, bestinlalu#9, bestinlalu#20] Implement ResponseController class with all APIs and testing done using rspec
…-back-end into ScoreLogic Adding controller updates to the ScoreLogic
Implemented name link from Edit to Update
Removed save_draft endpoint, Removed using Reviewer role, Updated rspec
…e-mapping edited case descriptions
Removing mock data from rails_helper
Code clean up, removing output statements
Increase puma version to 6.4.2
Update the swagger with new response endpoints
Changes as per few review comments in the response_controller
…and clarified comments
Updated response specs and clarified response_map helpers
…prove user messages
…s; prefer upcoming for deadlines; reduce rounding in report diffs
| resp.map.reviewer&.id == current_user.id | ||
| end | ||
|
|
||
| # Variant helpers for parent-admin checks |
There was a problem hiding this comment.
Shouldn't these be more general parent_admin methods? Putting them in the response class makes them vulnerable to a DRY problem if another class needs to do the same check.
There was a problem hiding this comment.
Keep these wrappers local to this controller for now. The shared primitives (find_assignment_instructor, current_user_ancestor_of?, role checks) are already provided by Authorization. The policy here is response-specific: allow an Administrator only when they are an ancestor of the assignment's instructor, which is the rule used to reopen/delete responses and to let an admin act on a response map during creation.
| reviewer_assignment.assignment_questionnaires.find_by(used_in_round: self.round).questionnaire | ||
| end | ||
|
|
||
| # Backward-compatible wrapper around ResponseMap#response_map_label. |
There was a problem hiding this comment.
What does "backward-compatible" mean? There should only be one format for ResponseMaps.
|
|
||
| # calculates the average score of all other responses | ||
| average_score = total / count | ||
| # Calculate average of peers by dividing once at the end |
There was a problem hiding this comment.
This seems like a potential DRY violation. Is there any way this code could be rewritten to leverage the mixin that does grade calculation?
| end | ||
|
|
||
| # returns the assignment related to the response map | ||
| # Returns the assignment context for this map, derived from the reviewer participant. |
| record.respond_to?(:updated_at) && record.updated_at.present? | ||
| end | ||
|
|
||
| def reviewee_exposes_team?(reviewee_record) |
There was a problem hiding this comment.
All methods need clear & concise method comments!
| reviewee_record.respond_to?(:team) && reviewee_record.team.present? | ||
| end | ||
|
|
||
| def team_exposes_memberships?(team_record) |
6 participants
All the changes for this project are documented here:
https://docs.google.com/document/d/1zMv7-xwsOt8cV7ej6gEaZWNpQoYq9zbJIn71rWMvUXo/edit?usp=sharing