Simplify required fields #31
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
Closes #22
Pomax
previously requested changes
Jan 19, 2017
| content_url = models.URLField() | ||
| thumbnail_url = models.URLField() | ||
| description = models.CharField(max_length=300, blank=True) | ||
| content_url = models.URLField(blank=True) |
There was a problem hiding this comment.
Are we sure about this one? Are people supposed to be able to post entries without a URL to point to the thing the entry was supposed to be for?
There was a problem hiding this comment.
You're right, I goofed. Fixed now.
alanmoo
dismissed
Pomax’s stale review
I want to see what happens when I dismiss a review
Pomax
previously requested changes
Jan 19, 2017
| @@ -19,7 +19,7 @@ def test_post_minimum_entry(self): | |||
| """ | |||
| Test posting an entry with minimum amount of content | |||
| """ | |||
| payload = self.generatePostPayload(data={'title':'title test_post_minimum_entry'}) | |||
| payload = self.generatePostPayload(data={'title':'title test_post_minimum_entry', 'tags':''}) | |||
There was a problem hiding this comment.
We probably also want to make sure we have full test coverage for required fields now.
tests for title, but withcontent_url also being required, we want to make sure that's a 400 as well?
Pomax
previously requested changes
Jan 19, 2017
| @@ -36,23 +36,18 @@ def test_post_duplicate_title(self): | |||
| self.assertEqual(len(entriesJson), 4) | |||
|
|
|||
| def test_post_empty_title(self): | |||
| """Make sure entries require a title""" | |||
| """Make sure entries require a title and content_url""" | |||
There was a problem hiding this comment.
can we split this up into two tests?
Pomax
approved these changes
Jan 20, 2017
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 #22
How are migrations handled when a server is already live? Do I need to just
makemigrationstarting from a database that's identical to the one that's on the server and then commit the migration files?