Skip to content

Commit 155eaf3

Browse files
paynejdclaude
andcommitted
OpenConceptLab/ocl_online#456 | Order a concept's mappings by id in API responses
The per-concept mapping querysets had no ORDER BY, so two identical requests could list a concept's mappings in different orders once rows were rewritten (the first request saves their checksums). That made test_inverse_mappings_are_unchanged fail intermittently on CI. Both get_mappings implementations now order by id, as the batched path from #949 already does. The test compares plain lists, so a failure shows the differing element, and checks the mapping order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TjM9dGow2KV1Gwr2dWQdph
1 parent de6304c commit 155eaf3

2 files changed

Lines changed: 9 additions & 20 deletions

File tree

‎core/concepts/serializers.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ def get_mappings(self, obj): # pylint: disable=too-many-locals
297297
) if is_collection else obj.get_bidirectional_mappings()
298298
if not include_retired:
299299
mappings = mappings.filter(retired=False)
300-
return serializer_class(mappings, many=True).data
300+
return serializer_class(mappings.order_by('id'), many=True).data
301301
if self.include_direct_mappings:
302302
mappings = obj.get_unidirectional_mappings_for_collection(
303303
parent_uri, collection_version) if is_collection else obj.get_unidirectional_mappings()
@@ -307,7 +307,7 @@ def get_mappings(self, obj): # pylint: disable=too-many-locals
307307
mappings = mappings.filter(to_source_url__in=target_repo_urls)
308308
if not include_retired:
309309
mappings = mappings.filter(retired=False)
310-
return serializer_class(mappings, many=True).data
310+
return serializer_class(mappings.order_by('id'), many=True).data
311311

312312
return []
313313

@@ -726,11 +726,11 @@ def get_mappings(self, obj):
726726
mappings = obj.get_bidirectional_mappings_for_collection(
727727
parent_uri, collection_version
728728
) if is_collection else obj.get_bidirectional_mappings()
729-
return MappingDetailSerializer(mappings, many=True, context=context).data
729+
return MappingDetailSerializer(mappings.order_by('id'), many=True, context=context).data
730730
if self.include_direct_mappings:
731731
mappings = obj.get_unidirectional_mappings_for_collection(
732732
parent_uri, collection_version) if is_collection else obj.get_unidirectional_mappings()
733-
return MappingDetailSerializer(mappings, many=True, context=context).data
733+
return MappingDetailSerializer(mappings.order_by('id'), many=True, context=context).data
734734

735735
return []
736736

‎core/integration_tests/tests_concepts.py‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4631,8 +4631,6 @@ def test_post_429_refused_by_capacity_limit(self, reranker_mock):
46314631

46324632

46334633
class ConceptListIncludeMappingsTest(OCLAPITestCase):
4634-
maxDiff = None
4635-
46364634
def setUp(self):
46374635
super().setUp()
46384636
self.source = OrganizationSourceFactory()
@@ -4704,17 +4702,8 @@ def test_inverse_mappings_are_unchanged(self):
47044702
batched = self.client.get(url).data
47054703
with patch.object(ConceptAbstractSerializer, 'preload_page', lambda *args: None):
47064704
per_concept = self.client.get(url).data
4707-
if batched != per_concept: # temporary diagnostic, removed before merge
4708-
import operator
4709-
def unequal(left, right, path=''):
4710-
if isinstance(left, dict) and isinstance(right, dict):
4711-
for key in set(left) | set(right):
4712-
yield from unequal(left.get(key), right.get(key), f'{path}.{key}')
4713-
elif isinstance(left, list) and isinstance(right, list) and len(left) == len(right):
4714-
for index, (lval, rval) in enumerate(zip(left, right)):
4715-
yield from unequal(lval, rval, f'{path}[{index}]')
4716-
elif left != right:
4717-
yield f'{path}: {left!r} ({type(left).__name__}, self-equal={operator.eq(left, left)}) != ' \
4718-
f'{right!r} ({type(right).__name__}, self-equal={operator.eq(right, right)})'
4719-
print('\nDIAG-UNEQUAL', list(unequal(batched, per_concept))[:20])
4720-
self.assertEqual(batched, per_concept)
4705+
# plain lists, so a failure shows the differing element
4706+
self.assertEqual(list(batched), list(per_concept))
4707+
for row in batched:
4708+
mapping_ids = [int(mapping['uuid']) for mapping in row['mappings']]
4709+
self.assertEqual(mapping_ids, sorted(mapping_ids))

0 commit comments

Comments
 (0)