Merge pull request #171 from WikiMovimentoBrasil/2025-02-small-fixes

fix: REMOVE_QUAL and REMOVE_REF throw errors && CREATE updates their entity_d

Author: arcstur

src/core/exceptions.py

@@ -60,6 +60,16 @@ def __init__(self, entity_id, property_id, value):
         return super().__init__(message)
 
 
+class NoQualifiers(ApiException):
+    def __init__(self):
+        return super().__init__("There are no qualifiers with the given value")
+
+
+class NoReferenceParts(ApiException):
+    def __init__(self):
+        return super().__init__("There are no reference parts with the given value")
+
+
 class UserError(ApiException):
     def __init__(self, status, response_code, response_message, response_json):
         self.status = status

src/core/models.py

@@ -20,6 +20,8 @@
 from .exceptions import UserError
 from .exceptions import NoStatementsForThatProperty
 from .exceptions import NoStatementsWithThatValue
+from .exceptions import NoQualifiers
+from .exceptions import NoReferenceParts
 from .exceptions import NonexistantPropertyOrNoDataType
 
 logger = logging.getLogger("qsts3")
@@ -376,6 +378,8 @@ class Error(models.TextChoices):
         OP_NOT_IMPLEMENTED = "op_not_implemented", _("Operation not implemented")
         NO_STATEMENTS_PROPERTY = "no_statements_property", _("No statements for given property")
         NO_STATEMENTS_VALUE = "no_statements_value", _("No statements with given value")
+        NO_QUALIIFERS = "no_qualifiers", _("No qualifiers with given value")
+        NO_REFERENCE_PARTS = "no_reference_parts", _("No reference parts with given value")
         SITELINK_INVALID = "sitelink_invalid", _("The sitelink id is invalid")
         COMBINING_COMMAND_FAILED = "combining_failed", _("The next command failed")
         API_USER_ERROR = "api_user_error", _("API returned a User error")
@@ -402,6 +406,8 @@ def start(self):
     def finish(self):
         logger.info(f"[{self}] finished")
         self.status = BatchCommand.STATUS_DONE
+        if self.operation == BatchCommand.Operation.CREATE_ITEM:
+            self.set_entity_id(self.response_id())
         self.save()
         self.propagate_status_to_previous_commands()
 
@@ -449,10 +455,9 @@ def entity_id(self):
     def set_entity_id(self, value):
         if self.json.get("item", None):
             self.json["item"] = value
-        elif self.json.get("entity", {}).get("id", None):
-            self.json["entity"]["id"] = value
         else:
-            raise ValueError("This command has no entity to update its id.")
+            self.json.setdefault("entity", {})
+            self.json["entity"]["id"] = value
 
     def entity_url(self):
         entity_id = self.entity_id()
@@ -703,6 +708,10 @@ def run(self, client: Client):
             self.error_with_value(self.Error.NO_STATEMENTS_PROPERTY)
         except NoStatementsWithThatValue:
             self.error_with_value(self.Error.NO_STATEMENTS_VALUE)
+        except NoQualifiers:
+            self.error_with_value(self.Error.NO_QUALIIFERS)
+        except NoReferenceParts:
+            self.error_with_value(self.Error.NO_REFERENCE_PARTS)
         except UserError as e:
             if e.response_message == "Invalid path parameter: 'site_id'":
                 self.error_with_value(self.Error.SITELINK_INVALID)
@@ -896,15 +905,22 @@ def _remove_qualifier_or_reference(self, entity: dict):
         Removes a qualifier or a reference from the entity.
         """
         statement = self._get_statement(entity)
-        if statement is None:
-            return
+        statement = statement if statement else {}
+        found_qualifier = False
+        found_ref_part = False
         for i, qual in enumerate(statement.get("qualifiers", [])):
             if self.is_in_qualifiers(qual):
                 statement["qualifiers"].pop(i)
+                found_qualifier = True
         for i, ref in enumerate(statement.get("references", [])):
             for j, part in enumerate(ref["parts"]):
                 if self.is_part_in_references(part):
                     statement["references"][i]["parts"].pop(j)
+                    found_ref_part = True
+        if not found_qualifier and len(self.qualifiers_for_api()) > 0:
+            raise NoQualifiers()
+        if not found_ref_part and len(self.references_for_api()) > 0:
+            raise NoReferenceParts()
 
     def _remove_entity_statement(self, entity: dict):
         """

src/core/tests/test_batch_processing.py

@@ -490,3 +490,112 @@ def test_all_errors(self, mocker):
         self.assertEqual(len(commands), 7)
         for command in commands:
             self.assertEqual(command.status, BatchCommand.STATUS_ERROR)
+
+    @requests_mock.Mocker()
+    def test_get_label(self, mocker):
+        ApiMocker.wikidata_property_data_types(mocker)
+        ApiMocker.is_autoconfirmed(mocker)
+        ApiMocker.create_item(mocker, "Q1")
+        ApiMocker.item_empty(mocker, "Q1")
+        ApiMocker.item_empty(mocker, "Q2")
+        ApiMocker.property_data_type(mocker, "P1", "quantity")
+        ApiMocker.patch_item_successful(mocker, "Q1", {})
+        ApiMocker.patch_item_successful(mocker, "Q2", {})
+        batch = self.parse("""
+        CREATE
+        LAST|P1|12
+        Q2|P1|15
+        """)
+        batch.combine_commands = True
+        commands = batch.commands()
+        client = ApiClient.from_username(batch.user)
+        ApiMocker.labels(mocker, client, "Q1", {"pt": "pt1", "en": "en1"})
+        ApiMocker.labels(mocker, client, "Q2", {"pt": "pt2", "en": "en2"})
+        self.assertEqual(commands[0].get_label(client, "pt"), None)
+        self.assertEqual(commands[1].get_label(client, "pt"), None)
+        self.assertEqual(commands[2].get_label(client, "pt"), "pt2")
+        self.assertEqual(commands[0].get_label(client, "de"), None)
+        self.assertEqual(commands[1].get_label(client, "de"), None)
+        self.assertEqual(commands[2].get_label(client, "de"), "en2")
+        self.assertEqual(commands[0].get_label(client, "en"), None)
+        self.assertEqual(commands[1].get_label(client, "en"), None)
+        self.assertEqual(commands[2].get_label(client, "en"), "en2")
+        batch.run() # -> load Q1 into commands[0] and commands[1]
+        self.assertEqual(commands[0].get_label(client, "pt"), "pt1")
+        self.assertEqual(commands[1].get_label(client, "pt"), "pt1")
+        self.assertEqual(commands[2].get_label(client, "pt"), "pt2")
+        self.assertEqual(commands[0].get_label(client, "de"), "en1")
+        self.assertEqual(commands[1].get_label(client, "de"), "en1")
+        self.assertEqual(commands[2].get_label(client, "de"), "en2")
+        self.assertEqual(commands[0].get_label(client, "en"), "en1")
+        self.assertEqual(commands[1].get_label(client, "en"), "en1")
+        self.assertEqual(commands[2].get_label(client, "en"), "en2")
+
+    @requests_mock.Mocker()
+    def test_remove_qual_or_ref_errors(self, mocker):
+        ApiMocker.item(
+            mocker,
+            "Q1",
+            {
+                "statements": {
+                    "P5": [
+                        {
+                            "id": "Q1234$abcdefgh-uijkl",
+                            "value": {
+                                "type": "value",
+                                "content": "Q12",
+                            },
+                            "qualifiers": [
+                                {
+                                    "property": {"id": "P65", "data_type": "quantity"},
+                                    "value": {
+                                        "type": "value",
+                                        "content": {"amount": "+84", "unit": "1"},
+                                    },
+                                },
+                            ],
+                            "references": [
+                                {
+                                    "hash": "i_am_ahash",
+                                    "parts": [
+                                        {
+                                            "property": {"id": "P93", "data_type": "url"},
+                                            "value": {
+                                                "type": "value",
+                                                "content": "https://kernel.org/",
+                                            },
+                                        },
+                                    ],
+                                },
+                            ],
+                        },
+                    ],
+                },
+            },
+        )
+        ApiMocker.wikidata_property_data_types(mocker)
+        ApiMocker.is_autoconfirmed(mocker)
+        ApiMocker.property_data_type(mocker, "P5", "wikibase-item")
+        ApiMocker.patch_item_successful(mocker, "Q1", {})
+        batch = self.parse("""
+        REMOVE_QUAL|Q1|P5|Q12|P123|123
+        REMOVE_QUAL|Q1|P5|Q999|P65|84
+        REMOVE_QUAL|Q1|P5|Q12|P65|84
+        REMOVE_REF|Q1|P5|Q12|S93|"https://kernel.xyz"
+        REMOVE_REF|Q1|P5|Q999|S93|"https://kernel.org/"
+        REMOVE_REF|Q1|P5|Q12|S93|"https://kernel.org/"
+        """)
+        commands = batch.commands()
+        batch.run()
+        # qualifiers
+        self.assertEqual(commands[0].status, BatchCommand.STATUS_ERROR)
+        self.assertEqual(commands[0].error, BatchCommand.Error.NO_QUALIIFERS)
+        self.assertEqual(commands[1].status, BatchCommand.STATUS_ERROR)
+        self.assertEqual(commands[1].error, BatchCommand.Error.NO_QUALIIFERS)
+        self.assertEqual(commands[2].status, BatchCommand.STATUS_DONE)
+        # references
+        self.assertEqual(commands[3].status, BatchCommand.STATUS_ERROR)
+        self.assertEqual(commands[3].error, BatchCommand.Error.NO_REFERENCE_PARTS)
+        self.assertEqual(commands[4].status, BatchCommand.STATUS_ERROR)
+        self.assertEqual(commands[4].error, BatchCommand.Error.NO_REFERENCE_PARTS)
+        self.assertEqual(commands[5].status, BatchCommand.STATUS_DONE)

src/core/tests/test_entity_patching.py

@@ -2,6 +2,8 @@
 from django.test import TestCase
 
 from core.parsers.v1 import V1CommandParser
+from core.exceptions import NoQualifiers
+from core.exceptions import NoReferenceParts
 
 
 class RemoveQualRefTests(TestCase):
@@ -180,6 +182,7 @@ def test_remove_qualifier(self):
         text = """
         REMOVE_QUAL|Q12345678|P65|42|P84267|-5
         REMOVE_QUAL|Q12345678|P31|somevalue|P18|+2025-01-15T00:00:00Z/11
+        REMOVE_QUAL|Q12345678|P65|42|P84267|999
         """
         batch = self.parse(text)
         entity = copy.deepcopy(self.INITIAL)
@@ -196,6 +199,12 @@ def test_remove_qualifier(self):
         self.assertQualCount(entity, "P31", 1)
         remove_p31_qual.update_entity_json(entity)
         self.assertQualCount(entity, "P31", 0)
+        # -----
+        remove_nothing = batch.commands()[2]
+        self.assertQualCount(entity, "P65", 1)
+        with self.assertRaises(NoQualifiers):
+            remove_nothing.update_entity_json(entity)
+        self.assertQualCount(entity, "P65", 1)
 
     def test_remove_reference(self):
         text = """
@@ -207,7 +216,8 @@ def test_remove_reference(self):
         # -----
         remove_nothing = batch.commands()[0]
         self.assertRefCount(entity, "P65", 0)
-        remove_nothing.update_entity_json(entity)
+        with self.assertRaises(NoReferenceParts):
+            remove_nothing.update_entity_json(entity)
         self.assertRefCount(entity, "P65", 0)
         # -----
         remove_part_mediawiki = batch.commands()[1]