evenicoulddoit / django-rest-framework-serializer-extensions

Extensions to help DRY up Django Rest Framework serializers
Other
247 stars 23 forks source link

Modification in case self.request is None (when using DRF js client) #10

Closed jojolalpin closed 7 years ago

jojolalpin commented 7 years ago

Modification related to issue #9

codecov-io commented 7 years ago

Codecov Report

Merging #10 into master will decrease coverage by 0.3%. The diff coverage is 92.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
- Coverage     100%   99.69%   -0.31%     
==========================================
  Files           5        5              
  Lines         323      325       +2     
==========================================
+ Hits          323      324       +1     
- Misses          0        1       +1
Impacted Files Coverage Δ
rest_framework_serializer_extensions/views.py 97.43% <92.3%> (-2.57%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f3510fc...9001268. Read the comment docs.

evenicoulddoit commented 7 years ago

Does this make sense to go within the for loop? I suppose it means that you could define the extensions fields without a request, but in practical terms wouldn't it be easier to just put a:

# Request is unset during API client discovery
if self.request is None:
    return context

at the top of the method? Happy either way, but I think I'll add a comment to remind myself why request could ever be None

jojolalpin commented 7 years ago

Sure, request updated.

jojolalpin commented 7 years ago

Hi Ian, I am not a specialist of travis and codecov. I do not know what to do. Give me an hint and I will do the changes. note: First commit was Ok

evenicoulddoit commented 7 years ago

@jojolalpin - it's complaining because code coverage has noticed that the coverage has decreased. Although you weren't testing the logic previously, because it was on a line which was already tested, it assumed that it was. Now it's in it's own condition, it realises that the code isn't tested.

On the weekend I'm happy to write a basic test that asserts that the method returns an empty dict when self.request is None, a la:

def test_returns_empty_dict_when_no_request(self):
    view = self.get_instance(view_class)
    view.format_kwarg = 'json'
    context = view.get_serializer_context()
    self.assertEqual(dict(), context)