Merge pull request #160 from WikiMovimentoBrasil/2025-01-error-visualization

refactor: improve error visualization

Author: mgalves

src/core/models.py

@@ -381,6 +381,8 @@ class Error(models.TextChoices):
         NO_STATEMENTS_VALUE = "no_statements_value", _("No statements 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")
+        API_SERVER_ERROR = "api_server_error", _("API returned a server error")
 
     error = models.TextField(
         null=True,
@@ -406,9 +408,12 @@ def finish(self):
         self.save()
         self.propagate_status_to_previous_commands()
 
-    def error_with_value(self, value: Error):
+    def error_with_value(self, value: Error, message: str = None):
         self.error = value
-        self.error_with_message(value.label)
+        if message is not None:
+            self.error_with_message(message)
+        else:
+            self.error_with_message(value.label)
 
     def error_with_exception(self, exception: Exception):
         message = getattr(exception, "message", str(exception))
@@ -705,7 +710,9 @@ def run(self, client: Client):
             if e.response_message == "Invalid path parameter: 'site_id'":
                 self.error_with_value(self.Error.SITELINK_INVALID)
             else:
-                self.error_with_exception(e)
+                self.error_with_value(self.Error.API_USER_ERROR, e.message)
+        except ServerError as e:
+            self.error_with_value(self.Error.API_SERVER_ERROR, e.message)
         except (ApiException, Exception) as e:
             self.error_with_exception(e)
 
@@ -1033,7 +1040,11 @@ def get_label(self, client: Client, preferred_language="en"):
         if id is None or id == "LAST":
             return None
 
-        labels = client.get_labels(id)
+        try:
+            labels = client.get_labels(id)
+        except ApiException as e:
+            logger.warning(f"[{self}] label call failed: {e.message}")
+            return None
 
         preferred = labels.get(preferred_language)
 

src/core/tests/test_api.py

@@ -188,6 +188,14 @@ def patch_item_successful(cls, mocker, item_id, json_result):
             status_code=200,
         )
 
+    @classmethod
+    def patch_item_fail(cls, mocker, item_id, status_code, json_result):
+        mocker.patch(
+            cls.wikibase_url(f"/entities/items/{item_id}"),
+            json=json_result,
+            status_code=status_code,
+        )
+
     @classmethod
     def add_statement_successful(cls, mocker, item_id):
         mocker.patch(

src/core/tests/test_batch_processing.py

@@ -399,3 +399,55 @@ def test_set_sitelink_invalid(self, mocker):
         self.assertEqual(commands[0].operation, BatchCommand.Operation.SET_SITELINK)
         self.assertEqual(commands[0].status, BatchCommand.STATUS_ERROR)
         self.assertEqual(commands[0].error, BatchCommand.Error.SITELINK_INVALID)
+
+    @requests_mock.Mocker()
+    def test_all_errors(self, mocker):
+        ApiMocker.wikidata_property_data_types(mocker)
+        ApiMocker.is_autoconfirmed(mocker)
+        ApiMocker.item_empty(mocker, "Q1234")
+        ApiMocker.item_empty(mocker, "Q5")
+        ApiMocker.item_empty(mocker, "Q7")
+        ApiMocker.property_data_type(mocker, "P5", "quantity")
+        ApiMocker.sitelink_invalid(mocker, "Q1234", "ptwikix")
+        ApiMocker.patch_item_fail(mocker, "Q5", 400,  {"code": "code", "message": "message"})
+        ApiMocker.patch_item_fail(mocker, "Q7", 500,  {"code": "code", "message": "message"})
+        ApiMocker.statements(mocker, "Q9", {})
+        ApiMocker.statements(mocker, "Q11", {
+            "P5": [{
+                "id": "Q1234$abcdefgh-uijkl",
+                "value": {
+                    "type": "value",
+                    "content": {"amount": "+32", "unit": "1"},
+                },
+            }],
+        })
+        batch = self.parse("""
+        CREATE_PROPERTY|url
+        Q1234|P5|12
+        Q1234|Sptwikix|"Cool article"
+        Q5|P5|123
+        Q7|P5|321
+        -Q9|P5|123
+        -Q11|P5|123
+        """)
+        batch.combine_commands = True
+        batch.run()
+        self.assertEqual(batch.status, Batch.STATUS_DONE)
+        commands = batch.commands()
+        self.assertEqual(commands[0].operation, BatchCommand.Operation.CREATE_PROPERTY)
+        self.assertEqual(commands[0].error, BatchCommand.Error.OP_NOT_IMPLEMENTED)
+        self.assertEqual(commands[1].operation, BatchCommand.Operation.SET_STATEMENT)
+        self.assertEqual(commands[1].error, BatchCommand.Error.COMBINING_COMMAND_FAILED)
+        self.assertEqual(commands[2].operation, BatchCommand.Operation.SET_SITELINK)
+        self.assertEqual(commands[2].error, BatchCommand.Error.SITELINK_INVALID)
+        self.assertEqual(commands[3].operation, BatchCommand.Operation.SET_STATEMENT)
+        self.assertEqual(commands[3].error, BatchCommand.Error.API_USER_ERROR)
+        self.assertEqual(commands[4].operation, BatchCommand.Operation.SET_STATEMENT)
+        self.assertEqual(commands[4].error, BatchCommand.Error.API_SERVER_ERROR)
+        self.assertEqual(commands[5].operation, BatchCommand.Operation.REMOVE_STATEMENT_BY_VALUE)
+        self.assertEqual(commands[5].error, BatchCommand.Error.NO_STATEMENTS_PROPERTY)
+        self.assertEqual(commands[6].operation, BatchCommand.Operation.REMOVE_STATEMENT_BY_VALUE)
+        self.assertEqual(commands[6].error, BatchCommand.Error.NO_STATEMENTS_VALUE)
+        self.assertEqual(len(commands), 7)
+        for command in commands:
+            self.assertEqual(command.status, BatchCommand.STATUS_ERROR)

src/web/templates/batch_commands.html

@@ -11,25 +11,25 @@
           </span>
         </td>
 
+        <td >
+            {% if command.display_label %}
+            {{ command.display_label }}
+            {% endif %}
+            {% if command.entity_url %}<a href="{{ command.entity_url }}">{% endif %}
+            {{ command.entity_info }}
+            {% if command.entity_url %}</a>{% endif %}
+        </td>
+
         {% if command.is_error_status %}
 
-          <td colspan="2">
+          <td>
             {{command.message}}
           </td>
           <td>
             {{command.raw}}
           </td>
 
         {% else %}
-
-          <td >
-              {% if command.display_label %}
-              {{ command.display_label }}
-              {% endif %}
-              {% if command.entity_url %}<a href="{{ command.entity_url }}">{% endif %}
-              {{ command.entity_info }}
-              {% if command.entity_url %}</a>{% endif %}
-          </td>
 
           <td >
             <span class="action action_{{ command.get_action_display.lower }}">