Skip to content
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

Fix empty pk detection in HyperlinkRelatedField.get_url #3962

Merged

Conversation

jslang
Copy link
Contributor

@jslang jslang commented Feb 24, 2016

This implementation allows detection of empty values that are non-nullable, allowing the field to return None values for such cases. Fixes #3959

@jpadilla
Copy link
Member

jpadilla commented Feb 25, 2016

@jslang lgtm, thanks!

@jpadilla jpadilla added this to the 3.3.3 Release milestone Feb 25, 2016
@xordoquy xordoquy modified the milestones: 3.4.0 Release, 3.3.3 Release Feb 25, 2016
@@ -87,6 +87,19 @@ def test_pk_representation(self):
assert representation == self.instance.pk.int


class TestHyperlinkedRelatedField(APISimpleTestCase):
def setUp(self):
self.instance = MockObject(pk=1, name='foo')
Copy link
Member

@jpadilla jpadilla Feb 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get rid of this self.instance which is not used in this test case.

@jslang jslang force-pushed the fix_null_check_in_hyperlinkrelation branch from 125334d to 186c0e3 Compare Feb 29, 2016
This implementation allows detection of empty values that are non-nullable, allowing the field to return None values for such cases
@jslang jslang force-pushed the fix_null_check_in_hyperlinkrelation branch from 186c0e3 to 7ac8cc7 Compare Feb 29, 2016
@xordoquy
Copy link
Collaborator

xordoquy commented Feb 29, 2016

Looks good to me too.
Just a question here, would it make sense to define the list of empty values in the class ?
Just in case someone wanted to override this. I have no idea whether this would be a common need or whether it's a too specific use case.

@tomchristie tomchristie added the Bug label Mar 4, 2016
tomchristie added a commit that referenced this pull request Mar 4, 2016
Fix empty pk detection in HyperlinkRelatedField.get_url
@tomchristie tomchristie merged commit 82ec6e8 into encode:master Mar 4, 2016
2 checks passed
@tomchristie
Copy link
Member

tomchristie commented Mar 4, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants