Skip to content

Commit 1bd58e1

Browse files
committed
Merge remote-tracking branch 'upstream/main' into upstream_main
2 parents 1e9d9dd + 8dfd85b commit 1bd58e1

File tree

4 files changed

+31
-21
lines changed

4 files changed

+31
-21
lines changed

osmchadjango/changeset/serializers.py

+9-10
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,17 @@ class ChangesetSerializer(ChangesetSerializerToStaff):
5656
tags = SerializerMethodField()
5757

5858
def get_reasons(self, obj):
59-
return BasicSuspicionReasonsSerializer(
60-
obj.reasons.filter(is_visible=True),
61-
many=True,
62-
read_only=True
63-
).data
59+
# Using obj.reasons.filter() would generate a new query, which in the context
60+
# of ChangesetListAPIView would waste the prefetching we've done and cause
61+
# N+1 total queries. To avoid this, we instead filter the reasons in Python.
62+
visible_reasons = [reason for reason in obj.reasons.all() if reason.is_visible]
63+
return BasicSuspicionReasonsSerializer(visible_reasons, many=True, read_only=True).data
6464

6565
def get_tags(self, obj):
66-
return BasicTagSerializer(
67-
obj.tags.filter(is_visible=True),
68-
many=True,
69-
read_only=True
70-
).data
66+
# Filter the tags in Python rather than using obj.tags.filter() which would
67+
# generate a new query (see above)
68+
visible_tags = [tag for tag in obj.tags.all() if tag.is_visible]
69+
return BasicTagSerializer(visible_tags, many=True, read_only=True).data
7170

7271

7372
class UserWhitelistSerializer(ModelSerializer):

osmchadjango/changeset/tests/test_changeset_views.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ def test_set_good_changeset_unlogged(self):
759759
self.assertIsNone(self.changeset.check_date)
760760

761761
def test_set_harmful_changeset_not_allowed(self):
762-
"""User can't mark his own changeset as harmful."""
762+
"""User can't mark their own changeset as harmful."""
763763
self.client.login(username=self.user.username, password='password')
764764
response = self.client.put(
765765
reverse('changeset:set-harmful', args=[self.changeset])
@@ -772,7 +772,7 @@ def test_set_harmful_changeset_not_allowed(self):
772772
self.assertIsNone(self.changeset.check_date)
773773

774774
def test_set_good_changeset_not_allowed(self):
775-
"""User can't mark his own changeset as good."""
775+
"""User can't mark their own changeset as good."""
776776
self.client.login(username=self.user.username, password='password')
777777
response = self.client.put(
778778
reverse('changeset:set-good', args=[self.changeset])

osmchadjango/changeset/views.py

+7-6
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ def add_reviewed_feature(self, type, id, harmful):
247247

248248
if changeset.uid in self.request.user.social_auth.values_list('uid', flat=True):
249249
return Response(
250-
{'detail': 'User can not check features on his own changeset.'},
250+
{'detail': 'User can not check features on their own changeset.'},
251251
status=status.HTTP_403_FORBIDDEN
252252
)
253253

@@ -279,7 +279,7 @@ def remove_harmful_feature(self, request, pk, type, id):
279279

280280
if changeset.uid in self.request.user.social_auth.values_list('uid', flat=True):
281281
return Response(
282-
{'detail': 'User can not check features on his own changeset.'},
282+
{'detail': 'User can not check features on their own changeset.'},
283283
status=status.HTTP_403_FORBIDDEN
284284
)
285285

@@ -343,7 +343,7 @@ def set_harmful(self, request, pk):
343343
)
344344
if changeset.uid in request.user.social_auth.values_list('uid', flat=True):
345345
return Response(
346-
{'detail': 'User can not check his own changeset.'},
346+
{'detail': 'User can not check their own changeset.'},
347347
status=status.HTTP_403_FORBIDDEN
348348
)
349349
if request.data:
@@ -372,7 +372,7 @@ def set_good(self, request, pk):
372372
)
373373
if changeset.uid in request.user.social_auth.values_list('uid', flat=True):
374374
return Response(
375-
{'detail': 'User can not check his own changeset.'},
375+
{'detail': 'User can not check their own changeset.'},
376376
status=status.HTTP_403_FORBIDDEN
377377
)
378378
if request.data:
@@ -440,7 +440,7 @@ def add_tag(self, request, pk, tag_pk):
440440

441441
if changeset.uid in request.user.social_auth.values_list('uid', flat=True):
442442
return Response(
443-
{'detail': 'User can not add tags to his own changeset.'},
443+
{'detail': 'User can not add tags to their own changeset.'},
444444
status=status.HTTP_403_FORBIDDEN
445445
)
446446
if changeset.checked and (
@@ -468,7 +468,7 @@ def remove_tag(self, request, pk, tag_pk):
468468

469469
if changeset.uid in request.user.social_auth.values_list('uid', flat=True):
470470
return Response(
471-
{'detail': 'User can not remove tags from his own changeset.'},
471+
{'detail': 'User can not remove tags from their own changeset.'},
472472
status=status.HTTP_403_FORBIDDEN
473473
)
474474
if changeset.checked and (
@@ -698,6 +698,7 @@ def filter_primary_tags(feature):
698698
class SetChangesetTagChangesAPIView(ModelViewSet):
699699
queryset = Changeset.objects.all()
700700
permission_classes = (IsAdminUser,)
701+
throttle_classes = [] # do not rate limit the tag changes endpoint
701702
# The serializer is not used in this view. It's here only to avoid errors
702703
# in docs schema generation.
703704
serializer_class = ChangesetStatsSerializer

osmchadjango/supervise/views.py

+13-3
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ class AOIListChangesetsFeedView(Feed):
119119
"""
120120

121121
def get_object(self, request, pk):
122-
self.feed_id = pk
123122
return AreaOfInterest.objects.get(pk=pk)
124123

125124
def title(self, obj):
@@ -131,7 +130,16 @@ def link(self, obj):
131130
return reverse('supervise:aoi-detail', args=[obj.id])
132131

133132
def items(self, obj):
134-
return obj.changesets()[:50]
133+
items = obj.changesets()[:50]
134+
# HACK: we want the <link> for each feed <item> to contain both the
135+
# changeset ID and the AOI ID, but only the changeset is available in
136+
# item_link() (passed in as the 'item' argument). As a workaround we'll
137+
# attach the AOI ID to each changeset object. Storing the AOI ID on
138+
# 'self' is tempting, but not thread-safe, since a single instance of
139+
# the Feed class is used to serve all feed requests.
140+
for item in items:
141+
item.aoi_id = obj.id
142+
return items
135143

136144
def item_title(self, item):
137145
return 'Changeset {} by {}'.format(item.id, item.user)
@@ -140,7 +148,9 @@ def item_geometry(self, item):
140148
return item.bbox
141149

142150
def item_link(self, item):
143-
return "{}/changesets/{}/?aoi={}".format(settings.OSMCHA_URL, item.id, self.feed_id)
151+
# item.aoi_id is obj.id, i.e. the UUID (pk) from the request URL,
152+
# as set by us in items(self, obj) above.
153+
return "{}/changesets/{}/?aoi={}".format(settings.OSMCHA_URL, item.id, item.aoi_id)
144154

145155
def item_pubdate(self, item):
146156
return item.date

0 commit comments

Comments
 (0)