Issues 43 & 67 #69
Merged
Issues 43 & 67 #69
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
…questing JWT tokens.
…for URLs and descriptoin.
Added patch method as outlined in issue-20 and started exploring dynamic search. Commented out code may move.
…value from UUID1 to UUID4, added TaggableManager, changed max field lengths for urls to 300 and descriptions to 600 chars.
… problem, but it didn't.
billglover
approved these changes
Feb 18, 2020
There was a problem hiding this comment.
This is awesome work. I've left a couple of comments against specific changes but am in favour of merging this anyway as it moves work on the backend forward significantly. Thank you!
Some things that tripped me up during review:
- migrations appear to be broken, I found it easier to delete DB and start again
- if using Docker to run your local development environment you'll need to run
docker-compose buildto trigger a re-build of the app (and pull in new requirements)
I'm happy for this to be merged, but if we do so without addressing comments the following should be captured as new issues for future work:
- extended characters cause tag names to break (looks like during
sluggeneration) - tag
nameandslugappearing to be swapped from DB schema to JSON - requirement for Taggit are defined twice (base and local)
- tags being a required field in the Resources schema
cbv3_django_prototype/cbv3_django_prototype/users/fixtures/users.json
Outdated
Show resolved
Hide resolved
cbv3_django_prototype/cbv3_django_prototype/userauth/serializers.py
Outdated
Show resolved
Hide resolved
cbv3_django_prototype/cbv3_django_prototype/resources/serializers.py
Outdated
Show resolved
Hide resolved
|
Let me do some cleanup around the areas noted above and do one last push. The fix for tests sadly will need to be in another PR because renaming things is going to make a real mess for merging. |
…resentation() in the TagSerializerField.
…el() call. Set up user is now pulled from users.json.
…opriate categories and alphabetized them.
…into issue-43 Repo rename and rebase.
This was referenced Feb 19, 2020
Closed
billglover
approved these changes
Feb 20, 2020
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.
Closes #67 , Partially Closes #43 - corrected tests still pending.
I've managed to break the testrunner, but thought it was important to push these changes so that others are not blocked. Will follow up with corrected tests and fixtures.