This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Scraper API Term & Course Search #154
Merged
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
Results are not yet filtered or sorted in any way before they are returned though
gannonprudhomme
changed the title
rachelconn
suggested changes
Feb 27, 2020
rachelconn
reviewed
Feb 27, 2020
rachelconn
reviewed
Feb 27, 2020
|
See #155 and my comments on |
gannonprudhomme
suggested changes
Feb 28, 2020
rachelconn
reviewed
Mar 2, 2020
rachelconn
reviewed
Mar 2, 2020
rachelconn
suggested changes
Mar 3, 2020
|
Also, given that a lot of these commits intersect with #142, I think it's best that we merge this with |
|
Remaining requested changes have been made. |
gannonprudhomme
suggested changes
Mar 21, 2020
There was a problem hiding this comment.
Great jobs with the tests!
Also, I got the tests setup locally and can confirm that they're passing.
I'd also like to merge backend/master into this soon, so lemme know when you're done making your next set of changes @ThatJuanGuy
|
Kinda late but I made the latest requested changes. |
|
Merged |
gannonprudhomme
suggested changes
Mar 21, 2020
|
Latest requested changes have been made |
rachelconn
suggested changes
Mar 21, 2020
|
Made the changes |
gannonprudhomme
approved these changes
Mar 22, 2020
-contains changed to startswith so that when 201 is in the 'search' parameter it doesn't return every course that takes in a 201X term instead of courses with the number 201. However, department must be inputted before the course number now. Suggested formatting and typo fixes Co-Authored-By: Gannon Prudhomme <gannonprudhomme@users.noreply.github.com> Co-Authored-By: Ryan Conn <rconn478@gmail.com>
Co-Authored-By: Ryan Conn <rconn478@gmail.com> Co-Authored-By: Gannon Prudhomme <gannonprudhomme@users.noreply.github.com> Fixes the remaining formatting issues
Removes /api/sections and /api/course
Added test for course names including numbers Added test for search with lowercase text
Test for "CSCE 3" now has to filter results with non-matching numbers
Added test for lowercase string with numbers ("csce 3")
Updated test for "csce" to include newly added course
Changed setUp method to setUpTestData Changed model saving to use bulk create Updated api/terms test to use bulk_create Made docstrings more descriptive
This was so they could be called in api_tests.py
Also deleted some extra empty lines and added pylint silencing comments to get_desc and get_course. Removed another empty line Silenced parameters differ pylint warning Fixes description for test_api_term_serializer_gives_expected_output_professional Co-Authored-By: Gannon Prudhomme <gannonprudhomme@users.noreply.github.com>
test_api_term_serializer_handles_(un)defined_season_correctly is now called test_season_num_to_string_handles(un)defined_season_correctly to be more clear as to what is actually being tested. This was also done to campus_num_to_string related tests.
String formatting to remove spaces in search parameter is now done when search variable is assigned to rather than during the .filter in RetrieveCourseSearchView
Fixed description of some tests The description on some tests was inacurrately referencing section serializer instead of the serializer actually beng tested
…ed in search parameter
Code readability improvements Silences "Too many public methods" pylint warning in api_tests.py Co-Authored-By: Gannon Prudhomme <gannonprudhomme@users.noreply.github.com> Removes unecesary department saving in api_tests.py Code readability improvements
rachelconn
force-pushed
the
backend/scraper/api/course-term
branch
from
3276b57
to
f3397d6
Compare
rachelconn
approved these changes
Mar 22, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Implements api/terms and api/course/search. For api/course/search, white space between the department and course number is trimmed. Department must also be entered before course number in the search parameter.