CSC/ECE 517/Spring 2023/E2308. Refactor course.rb and course team.rb models
Team Contact
Team Members
- Kartik Rawool, (unityID:khrawool, GitHub:kartikrawool)
- Ameya Vaichalkar, (unityID:agvaicha, GitHub:ameyagv)
- Vikram Pande, (unityID:vspande, GitHub:vikrampande7)
Mentor
- Kartiki Bhandakkar, kbhanda3
Overview of Expertiza
Expertiza is an open-source software written using Ruby on Rails which functions as a learning management software system. It has many different functions and abilities including the ability to create assignments, quizzes, assignment groups, and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates. Part of its functionality is a system containing information about the course and its associated teams respectively. The Course
and CourseTeam
which are the models primarily addressed in this project are both critical components in providing this functionality.
Description of Project
auth_controller
is used for authentication purposes. The controller completes a variety of tasks including handling the correct user logins, incorrect passwords, unknown users, and making sure the session and role information is updated at all points in that process. The original problem description listed three issues, two of which had since been corrected by other code changes since the document was written. The remaining problem was that some logger messages were included in methods that could be placed more cleanly in before_action
methods. Also, I personally noticed that there was a few lines of repeated code involved in resetting the role cache that needed to be combined into a shared private method.
The password_retrieval_controller
deals with the process of updating and resetting a user password. The send_password
method generates a token and appends it to a password reset URL. If a user submits a valid email address on the password_retrieval/forgotten
view, the URL is sent to the user's email. When a user goes to the password reset URL, the token parameter is decrypted and checked for expiration. Next, the password_retrieval/reset_password
view is loaded where a user enters an updated password and is sent back to the home page. In this project, the method was refactored in the following ways: to adhere to DRY principles, removal of hardwired constants, renaming of methods and variables, and enhanced comments. In addition, RSpec testing coverage of the controller was improved from 63.33% to 91.8% through a series of new tests that validate the functionality of the update_password
and send_password
methods.
Files Modified
Changes to app/models/course.rb
# | Change | Rationale | Commit Link |
---|---|---|---|
1 | Checked and Removed get_participant method
|
Checked the usage of Method through IDE whether the function is used, the only call was from course_spec.rb and not from anywhere else. Hence, removed from course.rb
|
Commit |
2 | Renamed copy_participants method
|
Renamed copy_participants to copy_assignment_participants which makes more sense and a clear idea of how method works and what is the purpose of method.
|
Commit |
3 | Removed add_member method from course_team model
|
Assignment_team and Course_team are subclasses of Team. Both should follow the similar logic of adding members of respective teams. As of for, add_participants, usage is different for course.rb and course_team.rb, hence it should be present in both. However, to make the code more consistent among classes, team, course_team, and assignment_team, the add_member method should only be present in the team class. | Commit |
Changes to app/models/course_team.rb
# | Change | Rationale | Commit Link |
---|---|---|---|
1 | Removed assignment_id
|
CourseTeam and AssignmentTeam inherit from Team class. The teams' table has an attribute parent_id that helps determine whether the team is AssigmentTeam or CourseTeam. Thus, there is no need for assignment_id in the CourseTeam class and no need for course_id in AssignmentTeam class. It will be a bad coding practice to call the assignment_id method on the CourseTeam object, calling the course_id method on the AssignmentTeam object as it would be inappropriate.
|
Commit |
2 | Renamed copy method as copy_to_assignment_team
|
As this method is copying a CourseTeam to an AssignmentTeam, renamed it as copy_to_assignment_team . A suggestion for copy method in AssignmentTeam would be to rename it as copy_to_course_team .
|
Commit |
Changes to spec/models/course_spec.rb
# | Change | Rationale | Commit Link | |
---|---|---|---|---|
1 | Removed the test case for get_paprticipant method test case
|
Removed the test case for get_participant method as the method was not used anywhere else in the code.
|
Commit | |
2 | Renamed copy_participants method test case
|
Renamed the respective method of copy_participants to copy_assignment_participants in rspec file as well.
|
Commit |
Changes to spec/models/course_team_spec.rb
# | Change | Rationale | Commit Link | |
---|---|---|---|---|
1 | Renamed copy to copy_to_assignment_team
|
Renamed copy to copy_to_assignment_team in rspec file as well.
|
Commit |
Changes to app/controllers/teams_controller.rb
# | Change | Rationale | Commit Link |
---|---|---|---|
1 | Renamed copy to copy_to_assignment_team
|
The change was directly related to changes of copy method in course_team.rb .
|
Commit |
Test Plan
All the suggested changes were made to course.rb
and course_team.rb
, all these models were amended and as a result we need to update the tests. We added or amended tests in their respective .spec files
- course_team
- course
- assignment_team
Relevant Links
- Github Repository: https://github.com/kartikrawool/expertiza/tree/refactor_course
- Pull Request: https://github.com/expertiza/expertiza/pull/2548
- VCL Server: http://152.7.178.105:8080/
Contributors to this project
- Kartik Rawool, (unityID:khrawool, GitHub:kartikrawool)
- Ameya Vaichalkar, (unityID:agvaicha, GitHub:ameyagv)
- Vikram Pande, (unityID:vspande, GitHub:vikrampande7)