-
Notifications
You must be signed in to change notification settings - Fork 116
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
E2480. Implement testing for new Bookmarks Controller #131
Open
Grochocinski
wants to merge
52
commits into
expertiza:main
Choose a base branch
from
Grochocinski:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
Modify dependency method whose name has been changed in the reimplementation.
Merge Functionality that Didn't Exist 8 Months Ago
Integrate authorization helper
Also include the AuthorizationHelper in the BookmarksController.
Users can only update/destroy bookmarks if they are the creator of the bookmark, or they have TA privileges or above. action_allowed implementation is waiting on response from the instructor.
Various action allowed functionality depending on user role. All users with at least Student privileges may view list. All users with only student privileges (who can see the commands in the UI) can create, rate, and save ratings for bookmarks. Editing, updating, and destroying bookmarks can be done by: Students who created the bookmark, TAs of the assignment the bookmark is under, the Instructor of the assignment the bookmark is under, the Administrator who created said instructor, and Super Admins (no catch). This was done with the help of methods in role.rb and authorization_helper.rb (some of whose methods have questionable names).
Also database changes from attempts to run tests in rspec (the normal way).
Implement action allowed
action_allowed? did not check index, now it does (in the same group as list)
Add `rspec` + authentication functionality
action_allowed? has all methods covered now
Database now seeds all 5 user types
Converted seeded data into factories
FInished testing index and show
Add the tests to the spec file, and create the bookmark_ratings.rb factory. Modify the model file to take a rating (this should have been included originally).
Add get_bookmark_rating_score tests
Testing grindset
TaMapping table now aligns with the other pieces that try to use it (ta_id instead of user_id). Modified bookmark controller and test to reflect this.
Required modification to bookmark_rating model, specifying that rating must be an integer between 0 and 5 (inclusive). Required modification to the save rating method, to make save! call (triggering errors on invalid input) Only thing that is missing is copying the tests to users other than students. ALSO, we may want to add more bounds tests ALSO we need to run the rubocop thing and fix the test up
Ta mapping
Also disabling certain rubocop features: * Metrics/BlockLength disabled because our blocks are purposefully segmented by: Mainblock//User Type//Request Type, and attempting to follow the rubocop guidelines would interfere with our purposeful test construction. * Metrics/LineLength disabled only AFTER applying changes to fixable LineLength violations. All remaining LineLength violations were in the form of test case descriptions (```it '<...>'``` was too long). The test descriptions should not be shortened, as doing so will lose specificity, and these longer test descriptions were for more complex tests, and remain intentionally.
Previous schema changes removed a foreign key mapping. While this appeared to have no immediate side effects, I modified the ta_mapping.rb implementation to accommodate a proper foreign key of this type, and added it to the schema.
Fix descriptor line length violations by splitting the string across two lines.
delete initial bookmark testing helper
…-comments Adding the commented version of session[] reliant action_allowed?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Authorization and Permissions:
Added an action_allowed? method to enforce role-based permissions (Student, TA, Instructor, Admin, or Super Admin) for specific actions, using associations with the relevant course.
CRUD Operations and Error Handling
Enhanced the controller with authorization checks for CRUD operations. For instance, students can only modify bookmarks they created, and TAs must have specific mappings to access a course’s bookmarks.
New actions like
check_action_allowed
ensure only authorized users can proceed, returning an unauthorized JSON response otherwise.Role-Specific Testing
Tests were expanded to cover various user roles, verifying that only users with appropriate permissions could access specific functionalities. Additional tests check bookmark creation, updates, and deletion by different user types.
Code Quality Improvements
Rubocop and Code Climate were used to refactor the code, addressing issues like redundant methods and code smells. Specific refactoring included renaming ambiguous methods, removing unused code, and improving readability.
Factory Setup and Test Automation
Added factory files to automate the creation of relevant data models (e.g., topics, courses, users) for testing purposes. This also includes factories for bookmark ratings and user roles.
Database and Schema Adjustments
Made schema changes to align
ta_mappings
with ata_id
reference, improving test accuracy and resolving conflicts with existing mappings.RSpec and Swagger Documentation
Initially, Swagger UI was used to handle authenticated requests in RSpec tests via an authentication token. With the creation of the authentication_helper, RSpec now handles automatic logins, allowing all 110 tests to run seamlessly without manual intervention.
See the Expertiza wiki entry for more details.