Merge pull request #33 from okfn/30-pending

Modificando como se muestran los estados

Author: germankay

ckanext/validate/actions/action.py

@@ -62,7 +62,6 @@ def resource_validate(context, data_dict):
         "Frictionless validation completed for resource %s: valid=%s",
         resource_id, report.valid,
     )
-    log.debug("Validation report for resource %s: %s", resource_id, report.to_descriptor())
 
     status = "success" if report.valid else "failure"
 

ckanext/validate/assets/css/validate.css

@@ -29,3 +29,14 @@
   background-color: #6c757d;
   color: #fff;
 }
+
+.validate-badge {
+  display: inline-flex;
+  align-items: center;
+  gap: 4px;
+}
+
+.validate-badge i {
+  margin-right: 4px;
+}
+

ckanext/validate/jobs.py

@@ -13,42 +13,45 @@
 ckan -c /etc/ckan/default/ckan.ini jobs worker
 
 https://docs.ckan.org/en/2.11/maintaining/cli.html
-
 """
 
 
-def run_resource_validation_job(resource_id):
+def run_resource_validation_job(resource_id, job_id):
     """
-    Step 5:
-    execute validation in the background job and ensure the resource
+    Execute validation in the background job and ensure the resource
     is updated with a final result, including the error case.
     """
-    log.info("Starting background validation for resource %s", resource_id)
+    log.info(
+        "Starting background validation for resource %s (job_id=%s)",
+        resource_id,
+        job_id,
+    )
 
     site_user = toolkit.get_action("get_site_user")({"ignore_auth": True}, {})
     context = {"ignore_auth": True, "user": site_user["name"]}
 
-    ValidationJob.create(resource_id=resource_id, status=JobStatus.RUNNING)
+    try:
+        ValidationJob.update_by_id(job_id, JobStatus.RUNNING)
+    except ValueError:
+        log.error("Job record with id %s not found, creating new job record for resource %s", job_id, resource_id)
+        return
+
     try:
         toolkit.get_action("resource_validate")(
             context,
             {"id": resource_id},
         )
-        log.info("Finished background validation for resource %s", resource_id)
-        ValidationJob.update(resource_id=resource_id, status=JobStatus.FINISHED)
-
-    except ValueError as exc:
-        log.error(
-            "No existing validation job found for resource %s when trying to update status: %s",
+        log.info(
+            "Finished background validation for resource %s (job_id=%s)",
             resource_id,
-            str(exc),
+            job_id,
         )
-        ValidationJob.update(resource_id=resource_id, status=JobStatus.ERROR)
+        ValidationJob.update_by_id(job_id, JobStatus.FINISHED)
 
     except Exception:
         log.exception(
-            "Background validation failed for resource %s",
+            "Background validation failed for resource %s (job_id=%s)",
             resource_id,
+            job_id,
         )
-
-        ValidationJob.create(resource_id=resource_id, status=JobStatus.ERROR)
+        ValidationJob.update_by_id(job_id, JobStatus.ERROR)

ckanext/validate/model/validation_jobs.py

@@ -29,11 +29,25 @@ class JobStatus(str, enum.Enum):
 
     @classmethod
     def pending_statuses(cls):
-        return {cls.QUEUED, cls.STARTED, cls.DEFERRED, cls.SCHEDULED, cls.RUNNING}
+        return {cls.QUEUED, cls.STARTED, cls.DEFERRED, cls.SCHEDULED}
+
+    @classmethod
+    def running_statuses(cls):
+        return {cls.RUNNING}
 
     @classmethod
     def error_statuses(cls):
-        return {cls.FAILED, cls.STOPPED, cls.CANCELED}
+        return {cls.FAILED, cls.STOPPED, cls.CANCELED, cls.ERROR}
+
+    @classmethod
+    def terminal_statuses(cls):
+        return {
+            cls.FINISHED,
+            cls.FAILED,
+            cls.STOPPED,
+            cls.CANCELED,
+            cls.ERROR,
+        }
 
 
 class ValidationJob(toolkit.BaseModel, ActiveRecordMixin):
@@ -61,23 +75,40 @@ def __repr__(self):
     def create(cls, resource_id, status):
         record = cls(
             resource_id=resource_id,
-            status=status,
+            status=status.value if isinstance(status, JobStatus) else status,
         )
         record.save()
         return record
 
+    @classmethod
+    def get(cls, job_id):
+        return Session.query(cls).filter(cls.id == job_id).first()
+
     @classmethod
     def update(cls, resource_id, status):
         record = cls.get_latest_job_for_resource(resource_id)
-        if record:
-            record.status = status
-            if status in (JobStatus.FINISHED, JobStatus.ERROR):
-                record.finish_timestamp = datetime.datetime.utcnow()
-            record.commit()
-            log.info("ValidationJob for resource_id %s updated to status %s", resource_id, status)
-            return record
-        else:
+        if not record:
             raise ValueError(f"No existing job found for resource_id {resource_id}")
+        return cls.update_by_id(record.id, status)
+
+    @classmethod
+    def update_by_id(cls, job_id, status):
+        record = cls.get(job_id)
+        if not record:
+            raise ValueError(f"No existing job found for job_id {job_id}")
+
+        record.status = status.value if isinstance(status, JobStatus) else status
+        if status in JobStatus.terminal_statuses():
+            record.finish_timestamp = datetime.datetime.utcnow()
+
+        record.commit()
+        log.info(
+            "ValidationJob id=%s for resource_id=%s updated to status=%s",
+            record.id,
+            record.resource_id,
+            status,
+        )
+        return record
 
     @classmethod
     def get_latest_job_for_resource(cls, resource_id):

ckanext/validate/resource_hooks.py

@@ -3,6 +3,7 @@
 import ckan.plugins.toolkit as toolkit
 
 from ckanext.validate import jobs
+from ckanext.validate.model.validation_jobs import JobStatus, ValidationJob
 
 log = logging.getLogger(__name__)
 
@@ -28,20 +29,31 @@ def is_resource_eligible_for_auto_validation(resource_dict):
     return True
 
 
-def build_validation_job_id(resource_id):
-    return f"validate-resource-{resource_id}"
+def enqueue_resource_validation_job(resource_id):
+    latest_status = ValidationJob.get_latest_job_status_for_resource(resource_id)
 
+    # TODO: handel running jobs scenario
+
+    if latest_status in JobStatus.pending_statuses():
+        log.debug(
+            "Validation job already pending for resource %s (status=%s), skipping enqueue",
+            resource_id,
+            latest_status,
+        )
+        return None
+
+    # Crear el registro ANTES de encolar para que la UI pueda mostrar Pending
+    job = ValidationJob.create(resource_id=resource_id, status=JobStatus.QUEUED)
 
-def enqueue_resource_validation_job(resource_id):
     try:
         return toolkit.enqueue_job(
             jobs.run_resource_validation_job,
-            args=[resource_id],
+            args=[resource_id, job.id],
             title=f"Validate resource {resource_id}",
-            rq_kwargs={"job_id": build_validation_job_id(resource_id)},
         )
     except Exception:
-        log.debug("Validation job already enqueued for resource %s, skipping", resource_id)
+        log.exception("Failed to enqueue validation job for resource %s", resource_id)
+        ValidationJob.update_by_id(job.id, JobStatus.ERROR)
         return None
 
 

ckanext/validate/templates/package/resource_read.html

@@ -17,7 +17,12 @@
           {% elif status == 'error' %}
             <span class="validate-badge validate-badge--error">{{ _('Error') }}</span>
           {% elif status == 'pending' %}
-            <span class="validate-badge validate-badge--pending">{{ _('Pending') }}</span>
+            <span class="validate-badge validate-badge--pending">
+              <i class="fa fa-spinner fa-spin"></i>
+              {{ _('Pending') }}
+            </span>
+          {% else %}
+            <span class="validate-badge validate-badge--pending">{{ _('Not validated') }}</span>
           {% endif %}
         </a>
       </li>

ckanext/validate/templates/package/resource_validate.html

@@ -18,7 +18,10 @@ <h1>{{ _('Resource Validation') }}</h1>
       <h3>{{ _('Current Status') }}</h3>
       {% set status = h.get_resource_validation_state(res) %}
       {% if status == 'pending' %}
-        <span class="validate-badge validate-badge--pending">{{ _('Pending') }}</span>
+        <span class="validate-badge validate-badge--pending">
+          <i class="fa fa-spinner fa-spin"></i>
+          {{ _('Pending') }}
+        </span>
         <p class="mt-2">{{ _('Validation is running in the background.') }}</p>
 
       {% elif status == 'success' %}

ckanext/validate/templates/package/snippets/resource_item.html

@@ -11,7 +11,12 @@
     {% elif status == 'error' %}
       <span class="validate-badge validate-badge--error">{{ _('Error') }}</span>
     {% elif status == 'pending' %}
-      <span class="validate-badge validate-badge--pending">{{ _('Pending') }}</span>
+      <span class="validate-badge validate-badge--pending">
+        <i class="fa fa-spinner fa-spin"></i>
+        {{ _('Pending') }}
+      </span>
+    {% else %}
+      <span class="validate-badge validate-badge--pending">{{ _('Not validated') }}</span>
     {% endif %}
   {% endif %}
 {% endblock %}

ckanext/validate/templates/scheming/display_snippets/validation_status.html

@@ -1,14 +1,17 @@
 {# Mostrar badges solo a usuarios logueados y que sean sysadmin o admin #}
 {% if h.check_access('sysadmin') or h.check_access('package_update', {'id': data['id']}) %}
-  {% if data[field.field_name] %}
-    {% set status = data[field.field_name] %}
-    {% if status == 'success' %}
-      <span class="validate-badge validate-badge--valid">{{ _('Valid') }}</span>
-    {% elif status == 'failure' %}
-      <span class="validate-badge validate-badge--invalid">{{ _('Invalid') }}</span>
-    {% elif status == 'error' %}
-      <span class="validate-badge validate-badge--error">{{ _('Error') }}</span>
-    {% endif %}
+  {% set status = h.get_resource_validation_state(data) %}
+  {% if status == 'success' %}
+    <span class="validate-badge validate-badge--valid">{{ _('Valid') }}</span>
+  {% elif status == 'failure' %}
+    <span class="validate-badge validate-badge--invalid">{{ _('Invalid') }}</span>
+  {% elif status == 'error' %}
+    <span class="validate-badge validate-badge--error">{{ _('Error') }}</span>
+  {% elif status == 'pending' %}
+    <span class="validate-badge validate-badge--pending">
+      <i class="fa fa-spinner fa-spin"></i>
+      {{ _('Pending') }}
+    </span>
   {% else %}
     <span class="validate-badge validate-badge--pending">{{ _('Not validated') }}</span>
   {% endif %}

ckanext/validate/tests/test_jobs.py

@@ -1,3 +1,5 @@
+from types import SimpleNamespace
+
 from ckanext.validate import jobs
 from ckanext.validate.model.validation_jobs import JobStatus
 
@@ -27,23 +29,23 @@ def fake_get_action(name):
     monkeypatch.setattr(
         jobs.ValidationJob,
         "create",
-        lambda resource_id, status: state_calls.append(("create", resource_id, status)),
+        lambda resource_id, status: SimpleNamespace(id=123),
     )
     monkeypatch.setattr(
         jobs.ValidationJob,
-        "update",
-        lambda resource_id, status: state_calls.append(("update", resource_id, status)),
+        "update_by_id",
+        lambda job_id, status: state_calls.append(("update_by_id", job_id, status)),
     )
 
-    jobs.run_resource_validation_job("res-1")
+    jobs.run_resource_validation_job("res-1", job_id=123)
 
     assert captured == {
         "context": {"ignore_auth": True, "user": "site-user"},
         "data_dict": {"id": "res-1"},
     }
     assert state_calls == [
-        ("create", "res-1", JobStatus.RUNNING),
-        ("update", "res-1", JobStatus.FINISHED),
+        ("update_by_id", 123, JobStatus.RUNNING),
+        ("update_by_id", 123, JobStatus.FINISHED),
     ]
 
 
@@ -67,17 +69,51 @@ def fake_get_action(name):
     monkeypatch.setattr(
         jobs.ValidationJob,
         "create",
-        lambda resource_id, status: state_calls.append(("create", resource_id, status)),
+        lambda resource_id, status: SimpleNamespace(id=456),
     )
     monkeypatch.setattr(
         jobs.ValidationJob,
-        "update",
-        lambda resource_id, status: state_calls.append(("update", resource_id, status)),
+        "update_by_id",
+        lambda job_id, status: state_calls.append(("update_by_id", job_id, status)),
     )
 
-    jobs.run_resource_validation_job("res-2")
+    jobs.run_resource_validation_job("res-2", job_id=456)
+
+    assert state_calls == [
+        ("update_by_id", 456, JobStatus.RUNNING),
+        ("update_by_id", 456, JobStatus.ERROR),
+    ]
+
+
+def test_run_resource_validation_job_returns_early_when_job_record_does_not_exist(monkeypatch):
+    resource_validate_called = []
+    state_calls = []
+
+    def fake_get_site_user(context, data_dict):
+        return {"name": "site-user"}
+
+    def fake_resource_validate(context, data_dict):
+        resource_validate_called.append(True)
+        return {"id": "res-3"}
+
+    def fake_get_action(name):
+        if name == "get_site_user":
+            return fake_get_site_user
+        if name == "resource_validate":
+            return fake_resource_validate
+        raise AssertionError(f"Unexpected action requested: {name}")
+
+    def fake_update_by_id(job_id, status):
+        state_calls.append(("update_by_id", job_id, status))
+        if status == JobStatus.RUNNING:
+            raise ValueError("No existing job found for job_id")
+
+    monkeypatch.setattr(jobs.toolkit, "get_action", fake_get_action)
+    monkeypatch.setattr(jobs.ValidationJob, "update_by_id", fake_update_by_id)
+
+    jobs.run_resource_validation_job("res-3", job_id=321)
 
+    assert resource_validate_called == []
     assert state_calls == [
-        ("create", "res-2", JobStatus.RUNNING),
-        ("create", "res-2", JobStatus.ERROR),
+        ("update_by_id", 321, JobStatus.RUNNING),
     ]