Skip to content

Commit

Permalink
Merge pull request #4338 from GeotrekCE/fix_agreg_route_unpublished_s…
Browse files Browse the repository at this point in the history
…teps

🐛 [BUG] Fix errors in Aggregator when retrieving unpublished Route Steps (refs #3569)
  • Loading branch information
Chatewgne authored Oct 21, 2024
2 parents 474f8b9 + 8b7e550 commit c6d255b
Show file tree
Hide file tree
Showing 15 changed files with 173 additions and 33 deletions.
8 changes: 6 additions & 2 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ CHANGELOG

- Improve development quickstart documentation

**Bug fixes**

- Fix missing unpublished related categories in Aggregator when retrieving unpublished Tour Steps (#3569)


2.109.2 (2024-09-19)
----------------------------
Expand All @@ -46,8 +50,8 @@ CHANGELOG
- ApidaeTrekParser duration import is fixed for multiple-days treks
- Apidae tourism parser now handles missing contact properties
- ApidaeTrekParser now handles missing source website
- Fix Aggregator does not retrieve unpublished Tour Steps (#3569)"
- Fix missing Annotation Categories in APIv2 for annotations other than Points (#4032)"
- Fix Aggregator does not retrieve unpublished Tour Steps (#3569)
- Fix missing Annotation Categories in APIv2 for annotations other than Points (#4032)
- Change default CORS configuration to 'always' : see https://github.com/GeotrekCE/Geotrek-rando-v3/issues/1257

**Documentation**
Expand Down
28 changes: 28 additions & 0 deletions geotrek/api/v2/views/trekking.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ class TrekRatingScaleViewSet(api_viewsets.GeotrekViewSet):
queryset = trekking_models.RatingScale.objects \
.order_by('pk') # Required for reliable pagination

@cache_response_detail()
def retrieve(self, request, pk=None, format=None):
# Allow to retrieve objects even if not visible in list view
elem = get_object_or_404(trekking_models.RatingScale, pk=pk)
serializer = api_serializers.TrekRatingScaleSerializer(elem, many=False, context={'request': request})
return Response(serializer.data)


class TrekRatingViewSet(api_viewsets.GeotrekViewSet):
filter_backends = api_viewsets.GeotrekViewSet.filter_backends + (
Expand All @@ -134,6 +141,13 @@ class TrekRatingViewSet(api_viewsets.GeotrekViewSet):
queryset = trekking_models.Rating.objects \
.order_by('order', 'name', 'pk') # Required for reliable pagination

@cache_response_detail()
def retrieve(self, request, pk=None, format=None):
# Allow to retrieve objects even if not visible in list view
elem = get_object_or_404(trekking_models.Rating, pk=pk)
serializer = api_serializers.TrekRatingSerializer(elem, many=False, context={'request': request})
return Response(serializer.data)


class NetworkViewSet(api_viewsets.GeotrekViewSet):
filter_backends = api_viewsets.GeotrekViewSet.filter_backends + (api_filters.TrekRelatedPortalFilter,)
Expand Down Expand Up @@ -189,12 +203,26 @@ class AccessibilityViewSet(api_viewsets.GeotrekViewSet):
serializer_class = api_serializers.AccessibilitySerializer
queryset = trekking_models.Accessibility.objects.all()

@cache_response_detail()
def retrieve(self, request, pk=None, format=None):
# Allow to retrieve objects even if not visible in list view
elem = get_object_or_404(trekking_models.Accessibility, pk=pk)
serializer = api_serializers.AccessibilitySerializer(elem, many=False, context={'request': request})
return Response(serializer.data)


class AccessibilityLevelViewSet(api_viewsets.GeotrekViewSet):
filter_backends = api_viewsets.GeotrekViewSet.filter_backends + (api_filters.TrekRelatedPortalFilter,)
serializer_class = api_serializers.AccessibilityLevelSerializer
queryset = trekking_models.AccessibilityLevel.objects.all()

@cache_response_detail()
def retrieve(self, request, pk=None, format=None):
# Allow to retrieve objects even if not visible in list view
elem = get_object_or_404(trekking_models.AccessibilityLevel, pk=pk)
serializer = api_serializers.AccessibilityLevelSerializer(elem, many=False, context={'request': request})
return Response(serializer.data)


class RouteViewSet(api_viewsets.GeotrekViewSet):
filter_backends = api_viewsets.GeotrekViewSet.filter_backends + (api_filters.TrekRelatedPortalFilter,)
Expand Down
4 changes: 4 additions & 0 deletions geotrek/common/tests/mixins.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import json
import os

from geotrek.common.parsers import DownloadImportError


def dictfetchall(cursor):
"Return all rows from a cursor as a dict"
Expand All @@ -16,5 +18,7 @@ def mock_json(self):
filename = os.path.join('geotrek', self.mock_json_order[self.mock_time][0], 'tests', 'data', 'geotrek_parser_v2',
self.mock_json_order[self.mock_time][1])
self.mock_time += 1
if "trek_not_found" in filename or "trek_unpublished_practice_not_found" in filename:
raise DownloadImportError("404 Does not exist")
with open(filename, 'r') as f:
return json.load(f)
4 changes: 4 additions & 0 deletions geotrek/common/tests/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,8 @@ def test_geotrek_aggregator_parser(self, mocked_head, mocked_get):
('trekking', 'trek_children.json'),
('trekking', 'trek_published_step.json'),
('trekking', 'trek_unpublished_step.json'),
('trekking', 'trek_unpublished_structure.json'),
('trekking', 'trek_unpublished_practice.json'),
('trekking', 'poi_ids.json'),
('trekking', 'poi.json'),
('tourism', 'informationdesk_ids.json'),
Expand Down Expand Up @@ -895,6 +897,8 @@ def test_geotrek_aggregator_parser(self, mocked_head, mocked_get):
('trekking', 'trek_children.json'),
('trekking', 'trek_published_step.json'),
('trekking', 'trek_unpublished_step.json'),
('trekking', 'trek_unpublished_structure.json'),
('trekking', 'trek_unpublished_practice.json'),
('trekking', 'poi_ids.json'),
('trekking', 'poi.json'),
('tourism', 'informationdesk_ids.json'),
Expand Down
43 changes: 40 additions & 3 deletions geotrek/trekking/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from geotrek.common.models import Label, Theme
from geotrek.common.parsers import (ApidaeBaseParser, AttachmentParserMixin,
GeotrekParser, GlobalImportError, Parser,
RowImportError, ShapeParser)
RowImportError, ShapeParser, DownloadImportError)
from geotrek.core.models import Path, Topology
from geotrek.trekking.models import (POI, Accessibility, DifficultyLevel,
OrderedTrekChild, Service, Trek,
Expand Down Expand Up @@ -195,19 +195,53 @@ def filter_points_reference(self, src, val):
geom = GEOSGeometry(json.dumps(val))
return geom.transform(settings.SRID, clone=True)

def fetch_missing_categories_for_tour_steps(self, step):
# For all categories, search missing values in mapping
for category, route in self.url_categories.items():
category_mapping = self.field_options.get(category, {}).get('mapping', {})
category_values = step.get(category)
if not isinstance(category_values, list):
category_values = [category_values]
for value in category_values:
# If category PK is not found in mapping, fetch it from provider
if value is not None and value not in list(category_mapping.keys()):
if self.categories_keys_api_v2.get(category):
try:
result = self.request_or_retry(f"{self.url}/api/v2/{route}/{int(value)}").json()
except DownloadImportError:
self.add_warning(f"Could not download {category} with id {value} from {self.provider}"
f" for Tour step {step.get('uuid')}")
continue
# Update mapping like we usually do
label = result[self.categories_keys_api_v2[category]]
if isinstance(result[self.categories_keys_api_v2[category]], dict):
if label[settings.MODELTRANSLATION_DEFAULT_LANGUAGE]:
self.field_options[category]["mapping"][value] = self.replace_mapping(label[settings.MODELTRANSLATION_DEFAULT_LANGUAGE], route)
else:
if label:
self.field_options[category]["mapping"][value] = self.replace_mapping(label, category)

def end(self):
"""Add children after all treks imported are created in database."""
self.next_url = f"{self.url}/api/v2/tour"
portals = self.portals_filter
try:
params = {
'in_bbox': ','.join([str(coord) for coord in self.bbox.extent]),
'fields': 'steps,id'
'fields': 'steps,id,uuid,published',
'portals': ','.join(portals) if portals else ''
}
response = self.request_or_retry(f"{self.next_url}", params=params)
results = response.json()['results']
steps_to_download = 0
final_children = {}
for result in results:
final_children[result['uuid']] = [step['id'] for step in result['steps']]
final_children[result['uuid']] = []
for step in result['steps']:
final_children[result['uuid']].append(step['id'])
if not any(step['published'].values()):
steps_to_download += 1
self.nb = steps_to_download

for key, value in final_children.items():
if value:
Expand All @@ -219,6 +253,9 @@ def end(self):
for child_id in value:
response = self.request_or_retry(f"{self.url}/api/v2/trek/{child_id}")
child_trek = response.json()
# The Tour step might be linked to categories that are not published,
# we have to retrieve the missing ones first
self.fetch_missing_categories_for_tour_steps(child_trek)
self.parse_row(child_trek)
trek_child_instance = self.obj
OrderedTrekChild.objects.update_or_create(parent=trek_parent_instance[0],
Expand Down
14 changes: 12 additions & 2 deletions geotrek/trekking/tests/data/geotrek_parser_v2/trek_children.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,20 @@
"uuid": "9e70b294-1134-4c50-9c56-d722720cacf1",
"steps": [
{
"id": 10439
"id": 10439,
"published": {
"fr": true,
"en": false,
"it": false
}
},
{
"id": 10442
"id": 10442,
"published": {
"fr": false,
"en": false,
"it": false
}
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,20 @@
"uuid": "9e70b294-1134-4c50-9c56-d722720cacf1",
"steps": [
{
"id": 1234
"id": 1234,
"published": {
"fr": true,
"en": false,
"it": false
}
},
{
"id": 1235
"id": 1235,
"published": {
"fr": false,
"en": false,
"it": false
}
}
]
},
Expand All @@ -34,7 +44,12 @@
"uuid": "b2aea666-5e6e-4daa-a750-7d2ee52d3fe1",
"steps": [
{
"id": 457
"id": 457,
"published": {
"fr": false,
"en": false,
"it": false
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"id": 2,
"label": {
"fr": "PR",
"en": null,
"en": "PR",
"es": null,
"it": null
},
Expand All @@ -17,7 +17,7 @@
"id": 4,
"label": {
"fr": "VTT",
"en": null,
"en": "Bike",
"es": null,
"it": null
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
},
"points_reference": null,
"portal": [],
"practice": 4,
"practice": 1,
"ratings": [],
"ratings_description": {
"fr": "",
Expand All @@ -197,7 +197,7 @@
},
"reservation_system": null,
"reservation_id": "",
"route": 3,
"route": 1,
"second_external_id": null,
"source": [],
"structure": 3,
Expand Down
6 changes: 3 additions & 3 deletions geotrek/trekking/tests/data/geotrek_parser_v2/trek_route.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"pictogram": "https://foo.fr/media/upload/route-loop.svg",
"route": {
"fr": "Boucle",
"en": null,
"en": "Loop",
"es": null,
"it": null
}
Expand All @@ -18,7 +18,7 @@
"pictogram": "https://foo.fr/media/upload/route-return.svg",
"route": {
"fr": "Aller-retour",
"en": null,
"en": "Return trip",
"es": null,
"it": null
}
Expand All @@ -28,7 +28,7 @@
"pictogram": "https://foo.fr/media/upload/route-cross.svg",
"route": {
"fr": "Traversée",
"en": null,
"en": "Crossing",
"es": null,
"it": null
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"id": 5,
"name": {
"fr": "Pratique non publiée",
"en": "Unpublished practice",
"es": null,
"it": null
},
"order": 3,
"pictogram": "https://foo.fr/media/upload/practice-mountainbike.svg"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"detail":"No Practice matches the given query."
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
"en": "",
"it": ""
},
"accessibilities": [
1
],
"accessibilities": [],
"accessibility_advice": {
"fr": "",
"en": "",
Expand Down Expand Up @@ -219,7 +217,7 @@
]
},
"portal": [],
"practice": 3,
"practice": 5,
"ratings": [],
"ratings_description": {
"fr": "",
Expand All @@ -233,8 +231,8 @@
"it": ""
},
"published": {
"fr": true,
"en": true,
"fr": false,
"en": false,
"it": false
},
"reservation_system": null,
Expand All @@ -244,10 +242,10 @@
"source": [
2
],
"structure": 1,
"structure": 5,
"themes": [
8,
4
1,
2
],
"update_datetime": "2022-05-16T12:10:59.927409Z",
"url": "https://foo.fr/api/v2/trek/2/",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"id": 5,
"name": "Struct5"
}
Loading

0 comments on commit c6d255b

Please sign in to comment.