Fix pokemon moves lookup assuming contiguous ids

pokemon_v2/serializers.py

@@ -4847,14 +4847,26 @@ def get_pokemon_cries(self, obj):
         }
     )
     def get_pokemon_moves(self, obj):
-        version_objects = VersionGroup.objects.all()
-        version_data = VersionGroupSummarySerializer(
-            version_objects, many=True, context=self.context
-        ).data
-        method_objects = MoveLearnMethod.objects.all()
-        method_data = MoveLearnMethodSummarySerializer(
-            method_objects, many=True, context=self.context
-        ).data
+        version_objects = VersionGroup.objects.all().order_by("id")
+        version_data = {
+            version_object.id: data
+            for version_object, data in zip(
+                version_objects,
+                VersionGroupSummarySerializer(
+                    version_objects, many=True, context=self.context
+                ).data,
+            )
+        }
+        method_objects = MoveLearnMethod.objects.all().order_by("id")
+        method_data = {
+            method_object.id: data
+            for method_object, data in zip(
+                method_objects,
+                MoveLearnMethodSummarySerializer(
+                    method_objects, many=True, context=self.context
+                ).data,
+            )
+        }
 
         # Get moves related to this pokemon and pull out unique Move IDs.
         # Note that it's important to order by the same column we're using to
@@ -4884,11 +4896,9 @@ def get_pokemon_moves(self, obj):
                 version_detail = OrderedDict()
 
                 version_detail["level_learned_at"] = move["level"]
-                version_detail["version_group"] = version_data[
-                    move["version_group"] - 1
-                ]
+                version_detail["version_group"] = version_data[move["version_group"]]
                 version_detail["move_learn_method"] = method_data[
-                    move["move_learn_method"] - 1
+                    move["move_learn_method"]
                 ]
                 version_detail["order"] = move["order"]
 

pokemon_v2/tests.py

@@ -5282,6 +5282,62 @@ def test_pokemon_api(self):
         self.assertEqual(response.data["count"], 1)
         self.assertEqual(response.data["results"][0]["name"], pokemon.name)
 
+    def test_pokemon_moves_with_non_contiguous_version_group_and_method_ids(self):
+        # Regression test for #1567: get_pokemon_moves used to look up
+        # VersionGroup/MoveLearnMethod summaries by list position
+        # (id - 1), assuming ids were a contiguous 1-indexed sequence.
+        # Deleting a row (or otherwise creating a gap) breaks that
+        # assumption and returns the wrong version group / learn method,
+        # or raises an IndexError.
+        pokemon_species = self.setup_pokemon_species_data(
+            name="pkmn species for id gap test"
+        )
+        pokemon = self.setup_pokemon_data(
+            pokemon_species=pokemon_species, name="pkm for id gap test"
+        )
+        self.setup_pokemon_sprites_data(pokemon=pokemon)
+        self.setup_pokemon_cries_data(pokemon, latest=True, legacy=True)
+
+        # Create and delete a version group and a move learn method so
+        # that the ids actually used below are not contiguous starting
+        # from 1.
+        doomed_version_group = self.setup_version_group_data(name="doomed ver grp")
+        doomed_version_group.delete()
+
+        doomed_move_learn_method = self.setup_move_learn_method_data(
+            name="doomed mv lrn mthd"
+        )
+        doomed_move_learn_method.delete()
+
+        move = self.setup_move_data(name="mv for id gap test")
+        version_group = self.setup_version_group_data(name="ver grp after gap")
+        pokemon_move = self.setup_pokemon_move_data(
+            pokemon=pokemon, move=move, version_group=version_group, level=5
+        )
+
+        response = self.client.get(
+            "{}/pokemon/{}/".format(API_V2, pokemon.pk), headers={"host": "testserver"}
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        version_detail = response.data["moves"][0]["version_group_details"][0]
+        self.assertEqual(version_detail["version_group"]["name"], version_group.name)
+        self.assertEqual(
+            version_detail["version_group"]["url"],
+            "{}{}/version-group/{}/".format(TEST_HOST, API_V2, version_group.pk),
+        )
+        self.assertEqual(
+            version_detail["move_learn_method"]["name"],
+            pokemon_move.move_learn_method.name,
+        )
+        self.assertEqual(
+            version_detail["move_learn_method"]["url"],
+            "{}{}/move-learn-method/{}/".format(
+                TEST_HOST, API_V2, pokemon_move.move_learn_method.pk
+            ),
+        )
+
     def test_pokemon_form_api(self):
         pokemon_species = self.setup_pokemon_species_data()
         pokemon = self.setup_pokemon_data(pokemon_species=pokemon_species)