From b414ea12d849cef21bf55384f28dbbfca753f6d2 Mon Sep 17 00:00:00 2001
From: manukala6 <manukala6@gmail.com>
Date: Wed, 26 Jun 2024 14:28:09 -0700
Subject: [PATCH] GTC-2618 Simplify append logic

---
 app/models/pydantic/versions.py        |  2 +-
 app/routes/datasets/versions.py        | 13 ++++++++-----
 tests/routes/datasets/test_versions.py | 21 ++-------------------
 3 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/app/models/pydantic/versions.py b/app/models/pydantic/versions.py
index 60d5a434..1f40313a 100644
--- a/app/models/pydantic/versions.py
+++ b/app/models/pydantic/versions.py
@@ -66,7 +66,7 @@ class VersionAppendIn(StrictBaseModel):
     layers: Optional[List[str]] = Field(
         None,
         description="List of layer names to append to version. "
-        "If not set, all layers in source_uri will be appended.",
+        "Only required for .gdb and .gpkg.",
     )
 
 class VersionResponse(Response):
diff --git a/app/routes/datasets/versions.py b/app/routes/datasets/versions.py
index 8dee2ebc..6a09530f 100644
--- a/app/routes/datasets/versions.py
+++ b/app/routes/datasets/versions.py
@@ -237,6 +237,7 @@ async def append_to_version(
     #  file(s) with ogrinfo
     #  See https://gfw.atlassian.net/browse/GTC-2234
 
+    # Construct creation_options for the append request
     # For the background task, we only need the new source uri from the request
     input_data = {"creation_options": deepcopy(default_asset.creation_options)}
     input_data["creation_options"]["source_uri"] = request.source_uri
@@ -252,20 +253,22 @@ async def append_to_version(
                 detail="source_driver must match the original source_driver"
             )
 
-        # Use layers from request if provided, otherwise leave empty
+        # Use layers from request if provided, else set to None if layers are in version creation_options
         if request.layers is not None:
             input_data["creation_options"]["layers"] = request.layers
         else:
-            input_data["creation_options"]["layers"] = None
-            
+            if input_data["creation_options"].get("layers") is not None:
+                input_data["creation_options"]["layers"] = None
+
+    # Use the modified input_data to append the new data
     background_tasks.add_task(
         append_default_asset, dataset, version, input_data, default_asset.asset_id
     )
 
-    # We now want to append the new uris to the existing ones and update the asset
+    # Now update the version's creation_options to reflect the changes from the append request
     update_data = {"creation_options": deepcopy(default_asset.creation_options)}
     update_data["creation_options"]["source_uri"] += request.source_uri 
-    if input_data["creation_options"].get("layers") is not None:
+    if request.layers is not None:
         if update_data["creation_options"]["layers"] is not None:
             update_data["creation_options"]["layers"] += request.layers
         else:
diff --git a/tests/routes/datasets/test_versions.py b/tests/routes/datasets/test_versions.py
index 6a0d07e1..2fbd1b3d 100755
--- a/tests/routes/datasets/test_versions.py
+++ b/tests/routes/datasets/test_versions.py
@@ -473,7 +473,7 @@ async def test_version_post_append(async_client: AsyncClient):
     response = await async_client.get(f"/dataset/{dataset}/{version}")
     assert response.status_code == 200
 
-    # Test appending with same source uri
+    # Test appending existing source_uri
     response = await async_client.post(
         f"/dataset/{dataset}/{version}/append",
         json = {
@@ -484,9 +484,6 @@ async def test_version_post_append(async_client: AsyncClient):
     )
     assert response.status_code == 200
 
-    # Test appending with new source uri
-
-    # Test appending with unspecified layers
     # Test appending with invalid source uri
     bad_uri = "s3://doesnotexist"
     response = await async_client.post(
@@ -515,21 +512,7 @@ async def test_version_post_append(async_client: AsyncClient):
     assert response.json()["status"] == "failed"
     assert (response.json()["message"] == "source_driver must match the original source_driver")
 
-    # Test appending with missing source driver
-    response = await async_client.post(
-        f"/dataset/{dataset}/{version}/append",
-        json = {
-            "source_uri": [f"s3://{BUCKET}/{GPKG_NAME}"],
-        }
-    )
-    assert response.status_code == 400
-    assert response.json() == {"status": "failed"}
-    assert response.json()["status"] == "failed"
-    assert (
-        response.json()["message"]
-        == "source_driver must be specified for non-datapump requests"
-    )
-
+    ## TODO: test with missing layers
 
 @pytest.mark.hanging
 @pytest.mark.asyncio