From d987abc44352912ae3a4060fbf71c5a2c72d54b3 Mon Sep 17 00:00:00 2001
From: Andreas Ellewsen <ae@uio.no>
Date: Thu, 6 May 2021 15:36:05 +0200
Subject: [PATCH] Refactor models and methods for batches

Instead of mixing the Batch model for batches sent *to* with batches
received *from* SETRA, we switch to using an InputBatch and an
OutputBatch. The InputBatch contains the fields that can be used when
sending a Batch (with Vouchers and Transactions), and the OutputBatch
contains all the information one receives from SETRA when fetching a
single batch from the batch/ endpoint, or multiple using the search
functionality.

The get_batch method is reduced to only fetch a single batch, while the
new search_batches method takes the role of searching based on creation
date and interface. In addition, the return_list_of_obj parameter is
removed in favor of the return_objects class attribute.

This commit expects that SETRA includes the id field in Batches.
---
 setra_client/client.py                        | 76 ++++++++++++-------
 setra_client/models.py                        | 37 +++++++--
 tests/fixtures/batch_fixture.json             |  2 +
 .../fixtures/batch_without_voucher_field.json |  2 +
 tests/test_client.py                          | 50 ++++++------
 tests/test_models.py                          | 13 ++--
 6 files changed, 113 insertions(+), 67 deletions(-)

diff --git a/setra_client/client.py b/setra_client/client.py
index 80a747f..8b024c8 100644
--- a/setra_client/client.py
+++ b/setra_client/client.py
@@ -3,13 +3,17 @@ import logging
 import urllib.parse
 from datetime import date
 from typing import Union, Optional, List
+
 import requests
 
-from setra_client.models import (Batch,
-                                 CompleteBatch,
-                                 Parameter,
-                                 BatchErrors,
-                                 BatchProgressEnum)
+from setra_client.models import (
+    BatchErrors,
+    BatchProgressEnum,
+    CompleteBatch,
+    InputBatch,
+    OutputBatch,
+    Parameter,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -203,36 +207,52 @@ class SetraClient(object):
             return [cls.from_dict(i) for i in data]
         raise TypeError(f"Expected list or dict, got {type(data)}")
 
-    def get_batch(self,
-                  batch_id: Optional[int] = None,
-                  min_created_date: Optional[date] = None,
-                  max_created_date: Optional[date] = None,
-                  batch_progress: Optional[BatchProgressEnum] = None,
-                  interface: Optional[str] = None,
-                  return_list_of_obj: Optional[bool] = False):
-
+    def search_batches(
+        self,
+        min_created_date: Optional[date] = None,
+        max_created_date: Optional[date] = None,
+        batch_progress: Optional[BatchProgressEnum] = None,
+        interface: Optional[str] = None,
+    ) -> Union[List[dict], dict, List[OutputBatch]]:
         """
-        GETs one or all batches from SETRA.
-        Dates (maximal and minimal creation dates) should 
+        Search batches in SETRA.
+        Dates (maximal and minimal creation dates) should
         be defined like ISO strings - like "2020-01-01".
         Batch Progress and interface - like strings exactly
         as the values that can be found in the corresponding
         fields of Setra.
         """
-        params = None
-        if batch_id is not None:
-            batch_id = str(batch_id)
-        else:
-            params = {'min_created_date': min_created_date,
-                      'max_created_date': max_created_date,
-                      'batch_progress': batch_progress,
-                      'interface': interface}
 
-        url = self.urls.batch(batch_id)
+        params = {
+            'min_created_date': min_created_date,
+            'max_created_date': max_created_date,
+            'batch_progress': batch_progress,
+            'interface': interface,
+        }
+
+        url = self.urls.batch()
         response = self.get(url, params=params)
         data = response.json()
-        if return_list_of_obj:
-            return [Batch(**item) for item in data]
+        response.raise_for_status()
+        if self.return_objects:
+            if isinstance(data, list):
+                return [OutputBatch(**item) for item in data]
+            elif isinstance(data, dict):
+                return [OutputBatch(**data)]
+        else:
+            return data
+
+    def get_batch(self, batch_id: str) -> Union[OutputBatch, dict]:
+        """
+        GETs a batch from SETRA.
+        """
+
+        url = self.urls.batch(str(batch_id))
+        response = self.get(url)
+        data = response.json()
+        response.raise_for_status()
+        if self.return_objects:
+            return OutputBatch(**data)
         else:
             return data
 
@@ -258,7 +278,7 @@ class SetraClient(object):
         response = self.get(url)
         return response.json()
 
-    def post_new_batch(self, batchdata: Batch):
+    def post_new_batch(self, batchdata: InputBatch):
         """
         POST combination of batch, vouchers and transactions
         """
@@ -274,7 +294,7 @@ class SetraClient(object):
             response.raise_for_status()
             return response
 
-    def put_update_batch(self, batchdata: Batch):
+    def put_update_batch(self, batchdata: InputBatch):
         """
         PUT updates an existing batch with vouchers and transactions,
         if the batch exists in setra, and has status=created, or validation failed.
diff --git a/setra_client/models.py b/setra_client/models.py
index ccda600..3b96a03 100644
--- a/setra_client/models.py
+++ b/setra_client/models.py
@@ -58,7 +58,7 @@ class Transaction(BaseModel):
     dim5: Optional[str]
     dim6: Optional[str]
     dim7: Optional[str]
-    sequenceno: Optional[int]
+    sequenceno: int
     taxcode: Optional[str]
     transtype: Optional[str]
     extinvref: Optional[str]
@@ -71,18 +71,45 @@ class Voucher(BaseModel):
     transactions: List[Transaction]
 
 
-class Batch(BaseModel):
+class InputBatch(BaseModel):
     """
     Model representing a batch, with a list of vouchers (and each
-    voucher has a list of transactions)
+    voucher has a list of transactions.)
     """
+
     client: str
     batchid: str
-    period: Optional[str]
+    period: int
+    interface: str
+    vouchertype: str
+    vouchers: List[Voucher]
+
+
+class OutputBatch(BaseModel):
+    """
+    Model representing a batch, with a list of voucher ids connected to that batch.
+    """
+    id: int
+    created: str
+    batchid: str
+    period: Optional[int]
     interface: str
+    client: str
     vouchertype: Optional[str]
+    batch_validated_ok_date: Optional[str]
+    batch_rejected_code: Optional[str]
+    sent_date: Optional[str]
+    http_response_content: Optional[str]
+    http_response_code: Optional[int]
+    orderno: Optional[int]
+    polling_statuscode: Optional[str]
+    polling_statuscode_date: Optional[str]
+    getresult_date: Optional[str]
+    getresult_statuscode: Optional[str]
+    getresult_logg: Optional[str]
+    getresult_report: Optional[str]
     batch_progress: Optional[str]
-    vouchers: Optional[List[Voucher]]
+    vouchers: Optional[List[int]]
 
 
 class ErrorTransaction(BaseModel):
diff --git a/tests/fixtures/batch_fixture.json b/tests/fixtures/batch_fixture.json
index fbbb4ec..9e9738d 100644
--- a/tests/fixtures/batch_fixture.json
+++ b/tests/fixtures/batch_fixture.json
@@ -1,4 +1,6 @@
 {
+  "id": 1,
+  "created": "2020-01-05 03:45",
   "client": 1,
   "batchid": 2,
   "period": 3,
diff --git a/tests/fixtures/batch_without_voucher_field.json b/tests/fixtures/batch_without_voucher_field.json
index 6c52766..485a5a9 100644
--- a/tests/fixtures/batch_without_voucher_field.json
+++ b/tests/fixtures/batch_without_voucher_field.json
@@ -1,4 +1,6 @@
 {
+  "id": 1,
+  "created":"2020-08-20 00:00",
   "client": 10,
   "batchid": 20,
   "period": 30,
diff --git a/tests/test_client.py b/tests/test_client.py
index 0e17a26..f33135b 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -7,7 +7,8 @@ from requests import HTTPError
 
 from setra_client.client import SetraClient
 from setra_client.client import SetraEndpoints
-from setra_client.models import Batch, CompleteBatch, BatchErrors, Parameter
+from setra_client.models import CompleteBatch, BatchErrors, Parameter, InputBatch, \
+    OutputBatch
 
 
 @pytest.fixture
@@ -184,48 +185,43 @@ def test_header_replacement2(client_with_a_header, batch_url, requests_mock, bas
 
 # Test get_batches method
 
-def test_successful_get_all_batches(client, requests_mock, baseurl):
+def test_successful_get_all_batches(client, requests_mock, baseurl, batch_fixture):
     """A working GET call should return HTTP 200, with json content"""
     url = SetraEndpoints(baseurl).batch()
-    requests_mock.get(url, json={'foo': 'bar'}, status_code=200)
+    requests_mock.get(url, json=[batch_fixture], status_code=200)
 
-    response = client.get_batch()
-    assert response == {'foo': 'bar'}
+    response = client.search_batches()
+    assert response == [OutputBatch(**batch_fixture)]
 
 
-def test_successful_get_batches_filtered_by_status(client, requests_mock, baseurl):
+def test_successful_get_batches_filtered_by_status(
+        client, requests_mock, baseurl, batch_fixture
+):
     """A working GET call should return HTTP 200, with json content"""
     url = SetraEndpoints(baseurl).batch()
-    requests_mock.get(url, json={'foo': 'bar'}, status_code=200)
+    requests_mock.get(url, json=[batch_fixture], status_code=200)
 
-    response = client.get_batch(batch_progress="ubw_import_ok")
-    assert response == {'foo': 'bar'}
+    response = client.search_batches(batch_progress="ubw_import_ok")
+    assert response == [OutputBatch(**batch_fixture)]
 
 
-def test_successfully_getting_single_batch(client, requests_mock, baseurl):
+def test_successfully_getting_single_batch(
+        client, requests_mock, baseurl, batch_fixture
+):
     """A working GET call should return HTTP 200, with json content"""
     url = SetraEndpoints(baseurl).batch(batch_id='3')
-    requests_mock.get(url, json={'foo': 'bar'}, status_code=200)
+    requests_mock.get(url, json=batch_fixture, status_code=200)
 
     response = client.get_batch(3)
-    assert response == {'foo': 'bar'}
+    assert response == OutputBatch(**batch_fixture)
 
 
 def test_failing_to_get_all_batches(client, batch_url, requests_mock, baseurl):
-    """A failing GET batches call should still return json"""
-    requests_mock.get(batch_url, json={'error': 'some json error message'}, status_code=404)
-
-    response = client.get_batch()
-    assert response == {'error': 'some json error message'}
-
-
-def test_failing_to_get_all_batches_with_text(client, batch_url, requests_mock, baseurl):
-    """A failing GET batches call that return text instead of json,
-    will raise JSONDecodeError in setra client"""
-    requests_mock.get(batch_url, text='some json error message', status_code=404)
+    """Searching for batches with no matches returns empty list"""
+    requests_mock.get(batch_url, json=[], status_code=200)
 
-    with pytest.raises(JSONDecodeError):
-        client.get_batch()
+    response = client.search_batches()
+    assert response == []
 
 
 # Test get_voucher method
@@ -295,7 +291,7 @@ def test_failing_to_get_all_transactions(client, requests_mock, baseurl):
 def test_successfully_post_batch_with_voucher(client, batch_with_voucher_fixture, requests_mock, baseurl):
     """A working GET call should return HTTP 200, with json content"""
     url = SetraEndpoints(baseurl).post_new_batch()
-    batch = Batch.from_dict(batch_with_voucher_fixture)
+    batch = InputBatch.from_dict(batch_with_voucher_fixture)
     requests_mock.post(url, json={'somestatus': 'ok'}, status_code=200, request_headers={"Content-Type": "application/json"})
 
     response = client.post_new_batch(batch)  # we get a response object back
@@ -308,7 +304,7 @@ def test_successfully_post_batch_with_voucher_and_response(client, batch_with_vo
     url = SetraEndpoints(baseurl).post_new_batch()
     requests_mock.post(url, json={'somestatus': 'ok'}, status_code=200, request_headers={"Content-Type": "application/json"})  #expect json content
 
-    batch = Batch.from_dict(batch_with_voucher_fixture)
+    batch = InputBatch.from_dict(batch_with_voucher_fixture)
     response = client.post_new_batch(batch)  # we get a response object back
     assert response.json() == {'somestatus': 'ok'}
     assert response.status_code == 200
diff --git a/tests/test_models.py b/tests/test_models.py
index c48e0d4..a845e4b 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -4,22 +4,21 @@ import pytest
 from pydantic import ValidationError
 
 from setra_client.models import (
-    Batch,
     BatchErrors,
     CompleteBatch,
     ErrorBatch,
     Parameter,
     Transaction,
-    Voucher,
+    Voucher, InputBatch, OutputBatch,
 )
 
 
 def test_batch(batch_fixture):
-    assert Batch(**batch_fixture)
+    assert InputBatch(**batch_fixture)
 
 
-def test_batch(batch_without_voucher_field):
-    assert Batch(**batch_without_voucher_field)
+def test_batch2(batch_without_voucher_field):
+    assert OutputBatch(**batch_without_voucher_field)
 
 
 def test_voucher(voucher_fixture):
@@ -44,13 +43,13 @@ def test_transaction_fail(trans_fail_fixture):
 
 
 def test_batch_with_voucher(batch_with_voucher_fixture):
-    assert Batch(**batch_with_voucher_fixture)
+    assert InputBatch(**batch_with_voucher_fixture)
 
 
 def test_batch_fail(batch_fail_fixture):
     # Using wrong data format, should fail:
     with pytest.raises(ValidationError):
-        Batch(**batch_fail_fixture)
+        OutputBatch(**batch_fail_fixture)
 
 
 def test_error_batch(error_batch):
-- 
GitLab