Merge pull request #217 from OpenUpSA/isoalte-rbac-fix

Refactor bulk_send_to_elastic function and enhance SQL query to inclu…

Author: desafinadude

helpers.py

@@ -85,45 +85,7 @@ def load_json_schema(filename: str) -> Dict[str, Any]:
 )
 
 
-# Bulk ES helper
-def bulk_send_to_elastic(documents):
-    """Send a batch of documents to Elasticsearch using the _bulk API."""
-    if not documents:
-        return True
 
-    bulk_lines = []
-    for doc in documents:
-            # Flatten isolate_data if present (same logic as send_to_elastic2)
-            if 'isolate_data' in doc and doc['isolate_data']:
-                isolate_data = doc['isolate_data']
-                if isinstance(isolate_data, str):
-                    try:
-                        isolate_data = json.loads(isolate_data)
-                    except Exception:
-                        isolate_data = {}
-                if isinstance(isolate_data, dict):
-                    for key, value in isolate_data.items():
-                        if key not in doc:
-                            doc[key] = value
-                del doc['isolate_data']
-            doc_id = doc.get("id")
-            action = {"index": {"_id": doc_id}} if doc_id else {"index": {}}
-            bulk_lines.append(json.dumps(action))
-            # Use json_serial for datetime/date serialization
-            bulk_lines.append(json.dumps(doc, default=json_serial))
-    bulk_data = "\n".join(bulk_lines) + "\n"
-
-    url = f"{settings.ELASTICSEARCH_URL}/{settings.ELASTICSEARCH_INDEX}/_bulk"
-    headers = {"Content-Type": "application/x-ndjson"}
-    try:
-        response = requests.post(url, data=bulk_data, headers=headers, timeout=30)
-        if response.status_code not in (200, 201):
-            print(f"Bulk indexing failed: {response.text}")
-            return False
-        return True
-    except Exception as e:
-        print(f"Exception during bulk ES indexing: {e}")
-        return False
 
 sg_api_key = SENDGRID_API_KEY
 sg_from_email = SENDGRID_FROM_EMAIL
@@ -1462,4 +1424,44 @@ def delete_from_elastic(submission_id):
             return False
     except Exception as e:
         print(f"Error deleting documents from Elasticsearch: {e}")
+        return False
+    
+# Bulk ES helper
+def bulk_send_to_elastic(documents):
+    """Send a batch of documents to Elasticsearch using the _bulk API."""
+    if not documents:
+        return True
+
+    bulk_lines = []
+    for doc in documents:
+            # Flatten isolate_data if present (same logic as send_to_elastic2)
+            if 'isolate_data' in doc and doc['isolate_data']:
+                isolate_data = doc['isolate_data']
+                if isinstance(isolate_data, str):
+                    try:
+                        isolate_data = json.loads(isolate_data)
+                    except Exception:
+                        isolate_data = {}
+                if isinstance(isolate_data, dict):
+                    for key, value in isolate_data.items():
+                        if key not in doc:
+                            doc[key] = value
+                del doc['isolate_data']
+            doc_id = doc.get("id")
+            action = {"index": {"_id": doc_id}} if doc_id else {"index": {}}
+            bulk_lines.append(json.dumps(action))
+            # Use json_serial for datetime/date serialization
+            bulk_lines.append(json.dumps(doc, default=json_serial))
+    bulk_data = "\n".join(bulk_lines) + "\n"
+
+    url = f"{settings.ELASTICSEARCH_URL}/{settings.ELASTICSEARCH_INDEX}/_bulk"
+    headers = {"Content-Type": "application/x-ndjson"}
+    try:
+        response = requests.post(url, data=bulk_data, headers=headers, timeout=30)
+        if response.status_code not in (200, 201):
+            print(f"Bulk indexing failed: {response.text}")
+            return False
+        return True
+    except Exception as e:
+        print(f"Exception during bulk ES indexing: {e}")
         return False

test/test_search.py

@@ -600,26 +600,10 @@ def test_search_access_control_private_project_all_roles(
 @pytest.mark.search
 @pytest.mark.rbac
 @pytest.mark.e2e
-@pytest.mark.xfail(
-    reason="BUG: publish2 endpoint doesn't fetch visibility field (app.py:2688-2693). "
-    "Should add 'p.privacy as visibility' to the SELECT query. "
-    "Currently visibility field is missing from ES documents, so access control fails."
-)
 def test_search_access_control_private_project_external_user(
     client, external_user_token, private_project_with_submission
 ):
-    """Test that external users cannot search private project data
-
-    KNOWN ISSUE: This test currently fails because the publish2 endpoint doesn't
-    include the visibility field when indexing documents to Elasticsearch.
-    The SELECT query needs to be updated to include: p.privacy as visibility, p.name as project_name
-    """
-    # Debug: Print project details
-    print(f"\\nPrivate project ID: {private_project_with_submission['project']['id']}")
-    print(
-        f"Private project privacy: {private_project_with_submission['project']['privacy']}"
-    )
-
+    """Test that external users cannot search private project data"""
     search_query = {
         "query": {
             "match": {"project_id": private_project_with_submission["project"]["id"]}
@@ -638,16 +622,6 @@ def test_search_access_control_private_project_external_user(
     assert response.status_code == 200
     result = response.get_json()
 
-    # Debug
-    print(f"Got {result['hits']['total']['value']} results")
-    if result["hits"]["hits"]:
-        print(
-            f"First result visibility: {result['hits']['hits'][0]['_source'].get('visibility', 'NOT SET')}"
-        )
-        print(
-            f"First result project_id: {result['hits']['hits'][0]['_source'].get('project_id', 'NOT SET')}"
-        )
-
     # External users should not see private project data
     # The access filter should prevent this
     assert result["hits"]["total"]["value"] == 0, (

worker.py

@@ -99,7 +99,7 @@ async def process_sequence_validation(job):
 
             # Get updated isolate data for bulk ES update
             cursor.execute("""
-                SELECT i.*, s.project_id, p.pathogen_id
+                SELECT i.*, s.project_id, p.pathogen_id, p.privacy as visibility
                 FROM isolates i
                 LEFT JOIN submissions s ON i.submission_id = s.id
                 LEFT JOIN projects p ON s.project_id = p.id