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

Drf 3.0 #6

Merged
merged 1 commit into from
Mar 31, 2015
Merged

Drf 3.0 #6

merged 1 commit into from
Mar 31, 2015

Conversation

jannon
Copy link
Contributor

@jannon jannon commented Mar 13, 2015

Okay, here's a Django Rest Framework 3.0+ compatibility PR (addresses #4). Actually, this is DRF 3.1+ compatibility update because the ModelSerializer API for custom field mapping needed for the schema hstore fields only returned on 3.1. DRF 3.x contains non-backward-compatible changes. As such, this update will no longer be compatible with DRF < 3.1. The main points are as follows:

  • HStoreField inherits from Field instead of the non-existent WritableField
  • HStoreField.to_native -> HStoreField.to_representation
  • HStoreField.from_native -> HStoreField.to_internal_value
  • HStoreSerializer.restore_object -> HStoreSerializer.create and HStoreSerializer.update
  • HStoreSerializer.get_field -> HStoreSerializer.build_standard_field and modified to use new ModelSerializer API
  • test fixtures changed, mostly to account for DRF 3.0's new field type coercion
  • pep8 cleanup (pruning whitespace)
  • adding MIDDLEWARE_CLASSES to test settings to silence Django 1.7 warnings
  • 95% code coverage

@coveralls
Copy link

Coverage Status

Coverage increased (+0.66%) to 94.85% when pulling e153d22 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.66%) to 94.85% when pulling e153d22 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.66%) to 94.85% when pulling 681c232 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@jannon
Copy link
Contributor Author

jannon commented Mar 13, 2015

Ugh, stupid python 2.7. I'll try to figure why the tests aren't working in 2.7 soon. Or if anyone happens to see the reason sooner, happy to take a tip

@coveralls
Copy link

Coverage Status

Coverage increased (+0.66%) to 94.85% when pulling 3e384a4 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@landscape-bot
Copy link

Code Health
Repository health increased by 2% when pulling 3e384a4 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.81%) to 95.0% when pulling f3431bf on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@landscape-bot
Copy link

Code Health
Repository health increased by 4% when pulling f3431bf on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@nemesifier
Copy link
Member

hi @jannon, thank you very much for contributing.

It looks good, I'll look for some spare time to analyze soon.

Could you squash all the commits into one commit please?

self.assertEqual(s['email'], 'test@test.com')
self.assertEqual(s['ip'], '10.10.10.10')
self.assertEqual(s['url'], 'http://test.com')
# should be hidden
self.assertTrue('data' not in s)

# self.assertTrue('data' not in s) # Why?
Copy link
Member

Choose a reason for hiding this comment

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

by design, the dictionary field is hidden in schema mode, this is because dictionary keys should be accessed via the virtual fields.

Copy link
Member

Choose a reason for hiding this comment

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

have you read this?

Copy link
Member

Choose a reason for hiding this comment

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

hey @jannon, have you read the preceding comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, just had a super busy week. Right, so the only thing I know for sure right now is that the 'data' item is present after upgrading to DRF3.1 and the changes needed to support it didn't add or remove anything related to the 'data' item. So it seemed to me that this was just new behavior and the test needed to be updated. If that's not the case, I can find some time to see the best way to explicitly remove 'data'

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see, you could also just uncomment the line and let the test fail. We can take care of this in subsequent commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, done

@jannon
Copy link
Contributor Author

jannon commented Mar 18, 2015

I'll get to squashing soon

@landscape-bot
Copy link

Code Health
Repository health increased by 4% when pulling 049581d on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.81%) to 95.0% when pulling 049581d on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@nemesifier
Copy link
Member

great, only 1 minor issue remaining, I left an inline comment

@coveralls
Copy link

Coverage Status

Coverage increased (+0.81%) to 95.0% when pulling 4596497 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@landscape-bot
Copy link

Code Health
Repository health increased by 4% when pulling 4596497 on jannon:drf-3.0 into 13ffd8c on djangonauts:master.

@jannon
Copy link
Contributor Author

jannon commented Mar 30, 2015

the test of the 'data' dictionary item is uncommented and everything should be good to go

@nemesifier nemesifier merged commit 4596497 into djangonauts:master Mar 31, 2015
@nemesifier
Copy link
Member

I had to edit your commit because your email did not match your github account and you wouldn't have shown in the contributors.
Now your contribution is displayed here: https://github.com/djangonauts/django-rest-framework-hstore/graphs/contributors
Thanks!

@jannon jannon deleted the drf-3.0 branch April 1, 2015 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants