feat(po-returns): make warehouse_id optional and refactor validation logic

- Change warehouse_id column to nullable in PoReturn model
- Update validation to work without warehouse context
- Simplify PO items validation against received/returned quantities
- Add UUID type casting for SQL query parameters
- Create database migration to alter warehouse_id column constraint
- Add migration scripts for applying SQL fixes and managing functions
- Update purchase orders service to handle has_grn filter with status mapping
- Enhance test coverage for purchase return flow
- Remove dependency on stock table for return validation
- Allow creating returns without specifying warehouse for improved flexibility

PO_RETURN_WAREHOUSE_OPTIONAL_SUMMARY.md

@@ -0,0 +1,112 @@
+# PO Return Warehouse ID Optional - Implementation Summary
+
+## Overview
+Made `warehouse_id` optional in PO Returns to allow creating returns without specifying a warehouse.
+
+## Changes Made
+
+### 1. Schema Changes (`app/po_returns/schemas/schema.py`)
+- Changed `warehouse_id` from required to optional: `Optional[str] = Field(None, ...)`
+- Added validator to normalize empty strings to `None`
+
+### 2. Model Changes (`app/po_returns/models/model.py`)
+- Changed `warehouse_id` column to `nullable=True`
+
+### 3. Service Changes (`app/po_returns/services/service.py`)
+- Made `warehouse_id` parameter optional in `get_po_items_for_return()`
+- Simplified validation logic in `validate_return_items()`:
+  - Removed dependency on warehouse-specific stock checks
+  - Validates against PO items directly (received qty, returned qty, rejected qty)
+  - No longer requires items to be in stock table
+- Added proper type casting for UUID parameters in SQL queries
+
+### 4. Controller Changes (`app/po_returns/controllers/router.py`)
+- Added `Optional` to imports
+- Made `warehouse_id` an optional query parameter in `get_po_items_for_return` endpoint
+
+### 5. Database Migration
+**File:** `db/migrations/alter_po_return_warehouse_nullable.sql`
+
+```sql
+ALTER TABLE trans.scm_po_return 
+ALTER COLUMN warehouse_id DROP NOT NULL;
+
+COMMENT ON COLUMN trans.scm_po_return.warehouse_id IS 'Warehouse ID (optional) - can be NULL if warehouse not specified';
+```
+
+**Status:** ✅ Applied manually
+
+## Validation Logic Changes
+
+### Before
+- Required warehouse_id
+- Validated against `fn_get_po_items_for_purchase_return()` stored procedure
+- Required items to exist in stock table with `qty_available > 0`
+- Failed if items were sold/dispatched
+
+### After
+- Optional warehouse_id (can be empty string or null)
+- Validates directly against PO items table
+- Checks:
+  - Item exists in PO
+  - Item has been received (`rcvd_qty > 0`)
+  - Return quantity ≤ (received - rejected - already returned)
+  - Batch number is provided
+- Works regardless of current stock levels
+
+## API Usage
+
+### Valid Payloads
+
+**With warehouse_id:**
+```json
+{
+    "po_id": "uuid",
+    "warehouse_id": "warehouse-uuid",
+    "items": [...]
+}
+```
+
+**Without warehouse_id (empty string):**
+```json
+{
+    "po_id": "uuid",
+    "warehouse_id": "",
+    "items": [...]
+}
+```
+
+**Without warehouse_id (null):**
+```json
+{
+    "po_id": "uuid",
+    "warehouse_id": null,
+    "items": [...]
+}
+```
+
+## Testing
+
+Test the API with:
+```bash
+curl -X POST http://localhost:3001/po-returns/ \
+  -H "Content-Type: application/json" \
+  -d '{
+    "po_id": "uuid",
+    "po_return_date": "2026-02-27",
+    "supplier_id": "SUPPLIER-001",
+    "warehouse_id": "",
+    "reason_code": "DAMAGED",
+    "items": [...]
+  }'
+```
+
+## Benefits
+
+1. **Flexibility:** Can create returns without warehouse context
+2. **Simpler Logic:** No dependency on stock table state
+3. **Better UX:** Works even if items have been sold/dispatched
+4. **Accurate Validation:** Validates against actual PO data
+
+## Date
+2026-02-27

app/po_returns/models/model.py

@@ -48,7 +48,7 @@ class PoReturn(Base):
     # Parties
     supplier_id = Column(String(64), nullable=False, index=True)
     client_id = Column(String(64), nullable=False, index=True) 
-    warehouse_id = Column(String(64), nullable=False, index=True)
+    warehouse_id = Column(String(64), nullable=True, index=True)  # Optional - can be NULL
     
     # Return details
     return_date = Column(DateTime, nullable=False)

app/po_returns/services/service.py

@@ -130,12 +130,12 @@ class PoReturnService:
                 return []
             
             query = text("""
-                SELECT *
-                FROM trans.fn_get_po_items_for_purchase_return(
-                    CAST(:po_id AS uuid),
-                    CAST(:warehouse_id AS uuid)
-                )
-            """)
+            SELECT *
+            FROM trans.fn_get_po_items_for_purchase_return(
+                CAST(:po_id AS uuid),
+                CAST(:warehouse_id AS varchar)
+            )
+        """)
 
             result = await db.execute(
                 query,
@@ -155,15 +155,14 @@ class PoReturnService:
                 PoItemForReturn(
                     po_item_id=row["po_item_id"],
                     catalogue_id=row["catalogue_id"],
-                    catalogue_name=row["catalogue_name"],
+                    sku=row["sku"],
+                    ordered_qty=row["ordered_qty"],
                     received_qty=row["received_qty"],
                     returned_qty=row["returned_qty"],
-                    dispatched_qty=row["dispatched_qty"],
                     returnable_qty=row["returnable_qty"],
+                    unit_price=row["unit_price"],
                     batch_no=row["batch_no"],
                     expiry_date=row["expiry_date"],
-                    qty_available=row["qty_available"],
-                    cost_price=row["cost_price"],
                 )
                 for row in rows
             ]

app/purchases/orders/services/service.py

@@ -326,18 +326,32 @@ class OrdersService:
                     for s in status_filter:
                         if s == "completed":
                             mapped_statuses.append("closed")
+                            # If filtering for has_grn=True, also include statuses that validly have a GRN
+                            # This ensures users see POs that have been received but not yet formally closed
+                            if filters.get("has_grn") is True:
+                                mapped_statuses.extend(["approved", "partial_received"])
                         else:
                             mapped_statuses.append(s)
                     
+                    # Deduplicate statuses
+                    mapped_statuses = list(set(mapped_statuses))
+                    
                     placeholders = [f":status_{i}" for i in range(len(mapped_statuses))]
                     where_conditions.append(f"p.status IN ({','.join(placeholders)})")
                     for i, status in enumerate(mapped_statuses):
                         params[f"status_{i}"] = status
                 else:
                     if status_filter == "completed":
-                        status_filter = "closed"
-                    where_conditions.append("p.status = :status")
-                    params["status"] = status_filter
+                        status_filter = "closed" # Basic mapping for single value
+                    
+                    # Special handling if has_grn is True for single value too? 
+                    # Complex to do with single parameter, but usually filters uses list for status.
+                    # If single value is used and it's 'completed' and has_grn=True:
+                    if status_filter == "closed" and filters.get("has_grn") is True:
+                         where_conditions.append("p.status IN ('closed', 'approved', 'partial_received')")
+                    else:
+                         where_conditions.append("p.status = :status")
+                         params["status"] = status_filter
             
             if "buyer_id" in filters and not merchant_id:
                 where_conditions.append("p.buyer_id = :buyer_id")

app/sql/fn_get_po_items_for_purchase_return.sql

@@ -1,11 +1,26 @@
--- FUNCTION: trans.fn_get_po_items_for_purchase_return(uuid, uuid)
+-- FUNCTION: trans.fn_get_po_items_for_purchase_return(uuid, character varying)
 
+-- DROP FUNCTION IF EXISTS trans.fn_get_po_items_for_purchase_return(uuid, character varying);
 -- DROP FUNCTION IF EXISTS trans.fn_get_po_items_for_purchase_return(uuid, uuid);
 
 CREATE OR REPLACE FUNCTION trans.fn_get_po_items_for_purchase_return(
 	p_po_id uuid,
-	p_warehouse_id uuid)
-    RETURNS TABLE(po_item_id uuid, catalogue_id uuid, catalogue_name text, received_qty numeric, returned_qty numeric, dispatched_qty numeric, returnable_qty numeric, batch_no text, expiry_date date, qty_available numeric, cost_price numeric) 
+	p_warehouse_id character varying)
+    RETURNS TABLE(
+        po_item_id uuid, 
+        catalogue_id uuid, 
+        catalogue_name text, 
+        sku text,
+        ordered_qty numeric,
+        received_qty numeric, 
+        returned_qty numeric, 
+        dispatched_qty numeric, 
+        returnable_qty numeric, 
+        batch_no text, 
+        expiry_date date, 
+        qty_available numeric, 
+        unit_price numeric
+    ) 
     LANGUAGE 'plpgsql'
     COST 100
     VOLATILE PARALLEL UNSAFE
@@ -18,6 +33,8 @@ BEGIN
         pi.po_item_id,
         pi.catalogue_id,
         cr.catalogue_name,
+        pi.sku::text,
+        pi.ord_qty,
         pi.rcvd_qty,
         pi.returned_qty,
         pi.dispatched_qty,
@@ -36,7 +53,7 @@ BEGIN
     JOIN trans.catalogue_ref cr
         ON cr.catalogue_id = pi.catalogue_id
     JOIN trans.scm_stock st
-        ON st.catalogue_id = pi.catalogue_id
+        ON st.catalogue_id::uuid = pi.catalogue_id
        AND st.warehouse_id = p_warehouse_id
 
     WHERE pi.po_id = p_po_id
@@ -46,5 +63,5 @@ BEGIN
 END;
 $BODY$;
 
-ALTER FUNCTION trans.fn_get_po_items_for_purchase_return(uuid, uuid)
+ALTER FUNCTION trans.fn_get_po_items_for_purchase_return(uuid, character varying)
     OWNER TO trans_owner;

db/migrations/alter_po_return_warehouse_nullable.sql

@@ -0,0 +1,10 @@
+-- Migration: Make warehouse_id nullable in scm_po_return table
+-- Date: 2026-02-27
+-- Reason: Allow PO returns without specifying warehouse
+
+-- Make warehouse_id nullable
+ALTER TABLE trans.scm_po_return 
+ALTER COLUMN warehouse_id DROP NOT NULL;
+
+-- Add comment
+COMMENT ON COLUMN trans.scm_po_return.warehouse_id IS 'Warehouse ID (optional) - can be NULL if warehouse not specified';

scripts/apply_sql_fix.py

@@ -0,0 +1,49 @@
+
+import asyncio
+import os
+import sys
+
+# Add project root to python path
+sys.path.append(os.getcwd())
+
+from sqlalchemy import text
+from app.sql import async_session
+
+async def apply_sql_fix():
+    print("Applying SQL fix for fn_get_po_items_for_purchase_return...")
+    
+    sql_file_path = "app/sql/fn_get_po_items_for_purchase_return.sql"
+    
+    with open(sql_file_path, "r") as f:
+        sql_content = f.read()
+        
+    async with async_session() as session:
+        try:
+            # Split the SQL content into statements
+            # The file contains a function definition ending with $BODY$; and an ALTER statement
+            parts = sql_content.split("$BODY$;")
+            
+            if len(parts) >= 2:
+                # Reconstruct the CREATE FUNCTION statement
+                create_func_stmt = parts[0] + "$BODY$;"
+                print("Executing CREATE FUNCTION statement...")
+                await session.execute(text(create_func_stmt))
+                
+                # The rest might contain the ALTER statement
+                remaining = parts[1].strip()
+                if remaining:
+                    print(f"Executing remaining statement: {remaining[:50]}...")
+                    await session.execute(text(remaining))
+            else:
+                # Fallback if split fails (unexpected format), try executing as is (might fail)
+                print("Could not split by $BODY$;, attempting to execute as single statement...")
+                await session.execute(text(sql_content))
+                
+            await session.commit()
+            print("✅ SQL fix applied successfully.")
+        except Exception as e:
+            print(f"❌ Error applying SQL fix: {e}")
+            await session.rollback()
+
+if __name__ == "__main__":
+    asyncio.run(apply_sql_fix())

scripts/drop_functions.py

@@ -0,0 +1,16 @@
+
+import asyncio
+from app.sql import connect_to_database, async_session
+from sqlalchemy import text
+
+async def drop_functions():
+    await connect_to_database()
+    async with async_session() as session:
+        print("Dropping functions...")
+        await session.execute(text("DROP FUNCTION IF EXISTS trans.fn_get_po_items_for_purchase_return(uuid, character varying);"))
+        await session.execute(text("DROP FUNCTION IF EXISTS trans.fn_get_po_items_for_purchase_return(uuid, uuid);"))
+        await session.commit()
+        print("Functions dropped.")
+
+if __name__ == "__main__":
+    asyncio.run(drop_functions())

scripts/run_po_return_migration.py

@@ -0,0 +1,89 @@
+#!/usr/bin/env python3
+"""
+Run PO Return warehouse_id nullable migration
+Reads database credentials from .env file
+"""
+import os
+import sys
+import asyncio
+from pathlib import Path
+from dotenv import load_dotenv
+import asyncpg
+
+# Add parent directory to path
+sys.path.insert(0, str(Path(__file__).parent.parent))
+
+# Load environment variables
+env_path = Path(__file__).parent.parent / '.env'
+load_dotenv(env_path)
+
+async def run_migration():
+    """Run the migration to make warehouse_id nullable"""
+    
+    # Get database URL from environment
+    database_url = os.getenv('DATABASE_URL')
+    
+    if not database_url:
+        print("ERROR: DATABASE_URL not found in .env file")
+        return False
+    
+    # Convert SQLAlchemy URL to asyncpg format and ensure SSL
+    database_url = database_url.replace('postgresql+asyncpg://', 'postgresql://')
+    
+    # Add SSL mode if not present
+    if '?' not in database_url:
+        database_url += '?sslmode=require'
+    elif 'sslmode' not in database_url:
+        database_url += '&sslmode=require'
+    
+    print(f"Connecting to database...")
+    print(f"URL: {database_url.split('@')[0]}@***")  # Hide credentials in log
+    
+    # Read migration SQL
+    migration_file = Path(__file__).parent.parent / 'db' / 'migrations' / 'alter_po_return_warehouse_nullable.sql'
+    
+    if not migration_file.exists():
+        print(f"ERROR: Migration file not found: {migration_file}")
+        return False
+    
+    with open(migration_file, 'r') as f:
+        migration_sql = f.read()
+    
+    print(f"\nMigration SQL:\n{migration_sql}\n")
+    
+    try:
+        # Connect to database using connection string with SSL
+        conn = await asyncpg.connect(database_url)
+        
+        print("Connected to database successfully")
+        
+        # Execute migration
+        await conn.execute(migration_sql)
+        
+        print("✅ Migration executed successfully!")
+        
+        # Verify the change
+        result = await conn.fetchrow("""
+            SELECT column_name, is_nullable, data_type
+            FROM information_schema.columns
+            WHERE table_schema = 'trans'
+              AND table_name = 'scm_po_return'
+              AND column_name = 'warehouse_id'
+        """)
+        
+        if result:
+            print(f"\nVerification:")
+            print(f"  Column: {result['column_name']}")
+            print(f"  Type: {result['data_type']}")
+            print(f"  Nullable: {result['is_nullable']}")
+        
+        await conn.close()
+        return True
+        
+    except Exception as e:
+        print(f"❌ ERROR: {e}")
+        return False
+
+if __name__ == "__main__":
+    success = asyncio.run(run_migration())
+    sys.exit(0 if success else 1)

tests/test_purchase_return_flow.py

@@ -10,6 +10,7 @@ import os
 sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
 
 from app.sql import connect_to_database, async_session
+from sqlalchemy import text
 from app.purchases.orders.services.service import OrdersService
 from app.purchases.orders.schemas.schema import POCreate, POItemCreate, POStatusChange
 from app.purchases.receipts.services.service import ReceiptsService
@@ -34,7 +35,28 @@ async def test_purchase_return_flow():
             catalogue_id = uuid.uuid4()
             buyer_id = str(uuid.uuid4())
             supplier_id = str(uuid.uuid4())
+            warehouse_id = str(uuid.uuid4())
+            sku_val = f"SKU_TEST_RETURN_{uuid.uuid4().hex[:8]}"
             
+            # Create dummy catalogue item
+            await db.execute(text("""
+                INSERT INTO trans.catalogue_ref (
+                    catalogue_id, catalogue_code, catalogue_type, catalogue_name, sku, status, created_at
+                ) VALUES (
+                    :id, :code, :type, :name, :sku, :status, NOW()
+                )
+                ON CONFLICT (catalogue_id) DO NOTHING
+            """), {
+                "id": str(catalogue_id),
+                "code": f"TEST-CAT-{uuid.uuid4().hex[:8]}",
+                "type": "product",
+                "name": "Test Product for Return",
+                "sku": sku_val,
+                "status": "active"
+            })
+            await db.commit()
+            print(f"✅ Dummy catalogue item created: {catalogue_id}")
+
             po_create = POCreate(
                 buyer_id=buyer_id,
                 buyer_type="retail",
@@ -52,7 +74,7 @@ async def test_purchase_return_flow():
                 items=[
                     POItemCreate(
                         catalogue_id=catalogue_id,
-                        sku="SKU_TEST_RETURN",
+                        sku=sku_val,
                         ord_qty=Decimal("10.000"),
                         uom="PCS",
                         ord_uom_qty=Decimal("10.000"),
@@ -65,8 +87,8 @@ async def test_purchase_return_flow():
             )
             
             orders_service = OrdersService(db)
-            po = await orders_service.create_po(po_create)
-            print(f"✅ PO Created: {po.po_no} ({po.po_id})")
+            po_response = await orders_service.create_po(po_create)
+            print(f"✅ PO Created: {po_response['po_no']} ({po_response['po_id']})")
             
             # 2. Approve PO
             print("\n2. Approving PO...")
@@ -75,38 +97,43 @@ async def test_purchase_return_flow():
                 remarks="Approving for test"
             )
             # Submit first
-            await orders_service.submit_po(str(po.po_id), status_change)
+            await orders_service.submit_po(po_response['po_id'], status_change)
             # Then Approve
-            await orders_service.approve_po(str(po.po_id), status_change)
+            await orders_service.approve_po(po_response['po_id'], status_change)
             print("✅ PO Approved")
             
             # 3. Create GRN
             print("\n3. Creating GRN...")
             # Reload PO to get items
-            po_dict = await orders_service.get_po(str(po.po_id), include_items=True)
-            po_item_id = po_dict["items"][0]["po_item_id"]
+            po_dict = await orders_service.get_po(po_response['po_id'], include_items=True)
+            po_item = po_dict["items"][0]
+            po_item_id = po_item["po_item_id"]
+            catalogue_id_val = po_item["catalogue_id"]
+            sku_val = po_item["sku"]
             
             grn_create = GRNCreate(
                 po_id=uuid.UUID(po_dict["po_id"]),
                 supplier_id=supplier_id,
                 receiver_id=buyer_id,
-                wh_location="WH_MAIN",
+                wh_location=warehouse_id,
                 status="received",
-                total_qty=Decimal("10.000"),
+                # total_qty is not in GRNCreate, it's calculated
                 remarks="Test GRN for Return",
                 created_by="USER_001",
+                shipment_id=str(uuid.uuid4()),
+                received_by="USER_001",
+                transporter="DHL",
                 items=[
                     GRNItemCreate(
                         po_item_id=uuid.UUID(po_item_id),
-                        catalogue_id=catalogue_id,
-                        sku="SKU_TEST_RETURN",
+                        catalogue_id=uuid.UUID(str(catalogue_id_val)),
+                        sku=sku_val,
                         recv_qty=Decimal("10.000"),
+                        txn_qty=Decimal("10.000"),
                         acc_qty=Decimal("10.000"),
-                        rej_qty=Decimal("0.000"),
+                        txn_uom="PCS",
                         uom="PCS",
-                        batch_no="BATCH_RETURN_001",
-                        mfg_dt=date.today(),
-                        exp_dt=date(2025, 12, 31)
+                        remarks="All good"
                     )
                 ]
             )
@@ -115,14 +142,36 @@ async def test_purchase_return_flow():
             grn = await receipts_service.create_grn(grn_create)
             print(f"✅ GRN Created: {grn.grn_no} ({grn.grn_id})")
             
+            # 3.1 Create dummy stock entry (since GRN service doesn't update stock automatically yet)
+            print("\n3.1 Creating dummy stock entry...")
+            await db.execute(text("""
+                INSERT INTO trans.scm_stock (
+                    stock_id, merchant_id, warehouse_id, catalogue_id, sku, 
+                    qty_on_hand, qty_available, batch_no
+                ) VALUES (
+                    :id, :merchant_id, :warehouse_id, :catalogue_id, :sku, 
+                    :qty, :qty, :batch_no
+                )
+            """), {
+                "id": uuid.uuid4(),
+                "merchant_id": buyer_id,
+                "warehouse_id": warehouse_id,
+                "catalogue_id": str(catalogue_id_val),
+                "sku": sku_val,
+                "qty": 10.0,
+                "batch_no": "BATCH-001"
+            })
+            await db.commit()
+            print("✅ Dummy stock entry created")
+            
             # 4. Check items for return
             print("\n4. Checking items available for return...")
             items_for_return = await PoReturnService.get_po_items_for_return(
-                db, uuid.UUID(po_dict["po_id"]), warehouse_id="WH_MAIN"
+                db, uuid.UUID(po_dict["po_id"]), warehouse_id=warehouse_id
             )
             print(f"Items available: {len(items_for_return)}")
             for item in items_for_return:
-                print(f" - Item: {item.catalogue_name}, Returnable: {item.returnable_qty}, Batch: {item.batch_no}")
+                print(f" - Item: {item.sku}, Returnable: {item.returnable_qty}, Batch: {item.batch_no}")
             
             if not items_for_return:
                 print("❌ No items available for return!")
@@ -135,9 +184,9 @@ async def test_purchase_return_flow():
             po_return_create = PoReturnCreate(
                 po_id=uuid.UUID(po_dict["po_id"]),
                 po_return_date=date.today(),
-                supplier_id=supplier_id,
-                buyer_id=buyer_id,
-                warehouse_id="WH_MAIN",
+                supplier_id=str(supplier_id),
+                buyer_id=str(buyer_id),
+                warehouse_id=str(warehouse_id),
                 reason_code=ReturnReasonCodeEnum.DAMAGED,
                 remarks="Test Return Creation",
                 items=[
@@ -161,6 +210,34 @@ async def test_purchase_return_flow():
                 print(f"❌ Failed to create return: {errors}")
             else:
                 print(f"✅ PO Return Created: {po_return.po_return_no} ({po_return.po_return_id})")
+
+            # 6. Verify list_pos filter fix
+            print("\n6. Verifying list_pos filter fix...")
+            
+            # First, check current status
+            po_check = await orders_service.get_po(po_response['po_id'])
+            print(f"Current PO Status: {po_check['status']}")
+            
+            # Scenario A: Filter by current status + has_grn=True
+            filters_a = {"status": [po_check['status']], "has_grn": True}
+            results_a, count_a = await orders_service.list_pos(filters=filters_a)
+            print(f"Scenario A (status='{po_check['status']}', has_grn=True): Found {count_a} POs")
+            
+            # Scenario B: Filter by "completed" + has_grn=True (User's request)
+            # With the fix, this should return the PO even if status is 'approved' (not closed yet)
+            
+            filters_b = {"status": ["completed"], "has_grn": True}
+            results_b, count_b = await orders_service.list_pos(filters=filters_b)
+            print(f"Scenario B (status='completed', has_grn=True): Found {count_b} POs")
+            
+            if count_b > 0:
+                found_po = next((p for p in results_b if str(p['po_id']) == str(po_response['po_id'])), None)
+                if found_po:
+                    print(f"✅ SUCCESSFULLY found the target PO with 'completed' filter! has_grn={found_po.get('has_grn')}")
+                else:
+                    print("⚠️ Found POs but not the one we created (might be other data)")
+            else:
+                print("❌ FAILED to find PO with 'completed' filter")
                 
         except Exception as e:
             print(f"❌ Error: {e}")