Updated collection device filtering to work for android (bug 975063) #1819

Closed
wants to merge 1 commit into
from

4 participants

@robhudson
Mozilla member

No description provided.

@ngokevin ngokevin and 2 others commented on an outdated diff Feb 27, 2014
mkt/collections/serializers.py
@@ -65,6 +65,16 @@ def to_native_es(self, value):
ser = CollectionESAppSerializer(value, context=self.context, many=True)
return ser.data
+ def _get_device(self, request):
+ dev = request.GET.get('dev')
@ngokevin
Mozilla member
ngokevin added a line comment Feb 27, 2014

dev and device are a bit confusing but I don't suppose it can be changed here. Maybe a comment.

@robhudson
Mozilla member
robhudson added a line comment Feb 27, 2014

This will surely change when we collapse android mobile and tablet together into a single device. I plan to work on that next.

@cvan
Mozilla member
cvan added a line comment Feb 27, 2014

comment or TODO?

@robhudson
Mozilla member
robhudson added a line comment Feb 27, 2014

API docs say:
device (string) – Filters by supported device. One of ‘desktop’, ‘mobile’, ‘tablet’, or ‘firefoxos’.
dev (string) – Enables filtering by device profile if either ‘firefoxos’ or ‘android’.

I actually don't get why these are separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ngokevin ngokevin and 2 others commented on an outdated diff Feb 27, 2014
mkt/collections/tests/test_serializers.py
@@ -67,8 +67,11 @@ def test_to_native(self):
eq_(data[1]['id'], int(self.app2.id))
eq_(data[1]['resource_uri'], self.app2.get_api_url(pk=self.app2.pk))
- def _field_to_native_profile(self, profile='0.0', dev='firefoxos'):
- request = self.get_request({'pro': profile, 'dev': dev})
+ def _field_to_native_profile(self, **kwargs):
+ query = {'pro': '0.0', 'dev': 'firefoxos'}
+ if kwargs:
@ngokevin
Mozilla member
ngokevin added a line comment Feb 27, 2014

do we need to check if kwargs?

@robhudson
Mozilla member
robhudson added a line comment Feb 27, 2014

apparently not, cool.

@cvan
Mozilla member
cvan added a line comment Feb 27, 2014

it'll be an empty dict, so unnecessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ngokevin
Mozilla member

r+

@cvan
Mozilla member

r+

@diox

fixed in a898056

@diox diox closed this Mar 7, 2014
@robhudson robhudson deleted the robhudson:975063-dev-device branch Jul 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment