Repository: ingresso-group/pyticketswitch

Issue:
missing "input_contained_unavailable_order" flag in trolley/reservation response
Currently when you attempt to add something to a trolley that is not available (sold out, performance in the past, max tickets per order exceeded, etc) it just appears as an empty trolley without any indication that something has gone wrong.

The API returns the `input_contained_unavailable_order` flag in it's response from `trolley.v1` and `reserve.v1`, and this should be added to the trolley object. I would suggest we should look at raising an exception as well.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=h1) Report
> Merging [#70](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=desc) into [master](https://codecov.io/gh/ingresso-group/pyticketswitch/commit/a22c4a3679174b1798acda89e59559930eb1f1a3?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/graphs/tree.svg?height=150&width=650&token=LsKUTNdnRb&src=pr)](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   99.48%   99.51%   +0.02%     
==========================================
  Files          37       37              
  Lines        1764     2046     +282     
==========================================
+ Hits         1755     2036     +281     
- Misses          9       10       +1
```


| [Impacted Files](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pyticketswitch/reservation.py](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/diff?src=pr&el=tree#diff-cHl0aWNrZXRzd2l0Y2gvcmVzZXJ2YXRpb24ucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [pyticketswitch/exceptions.py](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/diff?src=pr&el=tree#diff-cHl0aWNrZXRzd2l0Y2gvZXhjZXB0aW9ucy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [pyticketswitch/trolley.py](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/diff?src=pr&el=tree#diff-cHl0aWNrZXRzd2l0Y2gvdHJvbGxleS5weQ==) | `94.73% <100%> (+0.09%)` | :arrow_up: |
| [pyticketswitch/client.py](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/diff?src=pr&el=tree#diff-cHl0aWNrZXRzd2l0Y2gvY2xpZW50LnB5) | `99.31% <100%> (+0.13%)` | :arrow_up: |
| [pyticketswitch/availability.py](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/diff?src=pr&el=tree#diff-cHl0aWNrZXRzd2l0Y2gvYXZhaWxhYmlsaXR5LnB5) | `100% <0%> (ø)` | :arrow_up: |
| [pyticketswitch/mixins.py](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70/diff?src=pr&el=tree#diff-cHl0aWNrZXRzd2l0Y2gvbWl4aW5zLnB5) | `100% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=footer). Last update [a22c4a3...3a2e91d](https://codecov.io/gh/ingresso-group/pyticketswitch/pull/70?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pyticketswitch/client.py b/pyticketswitch/client.py
index 8b00be5..7726b43 100644
--- a/pyticketswitch/client.py
+++ b/pyticketswitch/client.py
@@ -1081,7 +1081,8 @@ class Client(object):
     def get_trolley(self, token=None, number_of_seats=None, discounts=None,
                     seats=None, send_codes=None, ticket_type_code=None,
                     performance_id=None, price_band_code=None,
-                    item_numbers_to_remove=None, **kwargs):
+                    item_numbers_to_remove=None,
+                    raise_on_unavailable_order=False, **kwargs):
 
         """Retrieve the contents of a trolley from the API.
 
@@ -1097,14 +1098,17 @@ class Client(object):
             seats (list): list of seat IDs.
             send_codes (dict): send codes indexed on backend source
                 code.
-            ticket_type_code: (string): code of ticket type to add to
+            ticket_type_code (string): code of ticket type to add to
                 the trolley.
-            performance_id: (string): id of the performance to add to
+            performance_id (string): id of the performance to add to
                 the trolley.
-            price_band_code: (string): code of price band to add to
+            price_band_code (string): code of price band to add to
                 the trolley.
-            item_numbers_to_remove: (list): list of item numbers to
+            item_numbers_to_remove (list): list of item numbers to
                 remove from trolley.
+            raise_on_unavailable_order (bool): When set to ``True`` this method
+                will raise an exception when the API was not able to add an
+                order to the trolley as it was unavailable.
             **kwargs: arbitary additional raw keyword arguments to add the
                 parameters.
 
@@ -1116,6 +1120,9 @@ class Client(object):
         Raises:
             InvalidParametersError: when there is an issue with the provided
                 parameters.
+            OrderUnavailableError: when ``raise_on_unavailable_order`` is set
+                to ``True`` and the requested addition to a trolley was
+                unavailable.
 
         .. _`/f13/trolley.v1`: http://docs.ingresso.co.uk/#trolley
 
@@ -1133,6 +1140,11 @@ class Client(object):
         trolley = Trolley.from_api_data(response)
         meta = CurrencyMeta.from_api_data(response)
 
+        if raise_on_unavailable_order:
+            if trolley and trolley.input_contained_unavailable_order:
+                raise exceptions.OrderUnavailableError(
+                    "inputs contained unavailable order")
+
         return trolley, meta
 
     def get_upsells(self, token=None, number_of_seats=None, discounts=None,
@@ -1278,7 +1290,8 @@ class Client(object):
     def make_reservation(self, token=None, number_of_seats=None, discounts=None,
                          seats=None, send_codes=None, ticket_type_code=None,
                          performance_id=None, price_band_code=None,
-                         item_numbers_to_remove=None, **kwargs):
+                         item_numbers_to_remove=None,
+                         raise_on_unavailable_order=False, **kwargs):
 
         """Attempt to reserve all the items in the given trolley
 
@@ -1314,6 +1327,9 @@ class Client(object):
                 the trolley
             item_numbers_to_remove: (list): list of item numbers to
                 remove from trolley.
+            raise_on_unavailable_order (bool): When set to ``True`` this method
+                will raise an exception when the API was not able to add an
+                order to the trolley as it was unavailable.
             **kwargs: arbitary additional raw keyword arguments to add the
                 parameters.
 
@@ -1325,6 +1341,9 @@ class Client(object):
         Raises:
             InvalidParametersError: when there is an issue with the provided
                 parameters.
+            OrderUnavailableError: when ``raise_on_unavailable_order`` is set
+                to ``True`` and the requested addition to a trolley was
+                unavailable.
 
         .. _`/f13/reserve.v1`: http://docs.ingresso.co.uk/#reserve
 
@@ -1342,15 +1361,22 @@ class Client(object):
         reservation = Reservation.from_api_data(response)
         meta = CurrencyMeta.from_api_data(response)
 
+        if raise_on_unavailable_order:
+            if reservation and reservation.input_contained_unavailable_order:
+                raise exceptions.OrderUnavailableError(
+                    "inputs contained unavailable order")
+
         return reservation, meta
 
-    def release_reservation(self, transaction_uuid):
+    def release_reservation(self, transaction_uuid, **kwargs):
         """Release an existing reservation.
 
         Wraps `/f13/release.v1`_
 
         Args:
             transaction_uuid (str): the identifier of the reservaiton.
+            **kwargs: arbitary additional raw keyword arguments to add the
+                parameters.
 
         Returns:
             bool: :obj:`True` if the reservation was successfully released
@@ -1361,7 +1387,8 @@ class Client(object):
         """
 
         params = {'transaction_uuid': transaction_uuid}
-        response = self.make_request('release.v1', params, method=POST)
+        kwargs.update(params)
+        response = self.make_request('release.v1', kwargs, method=POST)
 
         return response.get('released_ok', False)
 
diff --git a/pyticketswitch/exceptions.py b/pyticketswitch/exceptions.py
index f88f636..3aef367 100644
--- a/pyticketswitch/exceptions.py
+++ b/pyticketswitch/exceptions.py
@@ -51,3 +51,7 @@ class BackendThrottleError(BackendError):
 
 class CallbackGoneError(APIError):
     pass
+
+
+class OrderUnavailableError(PyticketswitchError):
+    pass
diff --git a/pyticketswitch/reservation.py b/pyticketswitch/reservation.py
index b12d2bb..a75087e 100644
--- a/pyticketswitch/reservation.py
+++ b/pyticketswitch/reservation.py
@@ -46,9 +46,12 @@ class Reservation(Status):
 
     """
 
-    def __init__(self, unreserved_orders=None, *args, **kwargs):
+    def __init__(self, unreserved_orders=None,
+                 input_contained_unavailable_order=False, *args, **kwargs):
+
         super(Reservation, self).__init__(*args, **kwargs)
         self.unreserved_orders = unreserved_orders
+        self.input_contained_unavailable_order = input_contained_unavailable_order
 
     @classmethod
     def from_api_data(cls, data):
@@ -75,7 +78,9 @@ class Reservation(Status):
                 for order in raw_unreserved_orders
             ]
 
-        inst.unreserved_orders=unreserved_orders
+        inst.unreserved_orders = unreserved_orders
+        inst.input_contained_unavailable_order = data.get(
+            'input_contained_unavailable_order', False)
 
         return inst
 
diff --git a/pyticketswitch/trolley.py b/pyticketswitch/trolley.py
index 0a78e2e..df54c75 100644
--- a/pyticketswitch/trolley.py
+++ b/pyticketswitch/trolley.py
@@ -25,11 +25,14 @@ class Trolley(JSONMixin, object):
         order_count (int): the number of orders in the trolley.
         purchase_result (:class:`PurchaseResult <pyticketswitch.callout.Callout>`):
             the result of the purchase attempt when available.
-
+        input_contained_unavailable_order (bool): indicates that the call used
+            to create or modify this trolley object included at least one order
+            that was not available.
     """
     def __init__(self, token=None, transaction_uuid=None, transaction_id=None,
                  bundles=None, discarded_orders=None, minutes_left=None,
-                 order_count=None, purchase_result=None):
+                 order_count=None, purchase_result=None,
+                 input_contained_unavailable_order=False):
         self.token = token
         self.transaction_uuid = transaction_uuid
         self.transaction_id = transaction_id
@@ -38,6 +41,7 @@ class Trolley(JSONMixin, object):
         self.minutes_left = minutes_left
         self.order_count = order_count
         self.purchase_result = purchase_result
+        self.input_contained_unavailable_order = input_contained_unavailable_order
 
     @classmethod
     def from_api_data(cls, data):
@@ -82,6 +86,8 @@ class Trolley(JSONMixin, object):
             'transaction_uuid': raw_contents.get('transaction_uuid'),
             'transaction_id': raw_contents.get('transaction_id'),
             'order_count': data.get('trolley_order_count'),
+            'input_contained_unavailable_order': data.get(
+                'input_contained_unavailable_order', False),
         }
 
         minutes = data.get('minutes_left_on_reserve')


Test patch:
diff --git a/tests/test_client.py b/tests/test_client.py
index 6dfc234..c339059 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -1320,6 +1320,34 @@ class TestClient:
         assert 'gbp' in meta.currencies
         assert meta.default_currency_code == 'gbp'
 
+    def test_get_trolley_with_unavailable_order(self, client, monkeypatch):
+        """
+        This test is to check that an unavailable order doesn't raise
+        any exceptions unless `raise_on_unavailable_order` is set to true
+        """
+        response = {
+            'trolley_contents': {},
+            'trolley_token': 'DEF456',
+            'currency_code': 'gbp',
+            'input_contained_unavailable_order': True,
+            'currency_details': {
+                'gbp': {
+                    'currency_code': 'gbp',
+                }
+            }
+        }
+
+        mock_make_request = Mock(return_value=response)
+        monkeypatch.setattr(client, 'make_request', mock_make_request)
+
+        # this should not raise any exceptions
+        client.get_trolley()
+
+        # but this should
+        with pytest.raises(exceptions.OrderUnavailableError):
+            client.get_trolley(raise_on_unavailable_order=True)
+
+
     def test_get_upsells(self, client, monkeypatch):
         # fakes
         response = {
@@ -1409,6 +1437,26 @@ class TestClient:
         assert 'gbp' in meta.currencies
         assert meta.default_currency_code == 'gbp'
 
+    def test_make_reservation_with_unavailable_order(self, client, monkeypatch):
+        """
+        This test is to check that an unavailable order doesn't raise
+        any exceptions unless `raise_on_unavailable_order` is set to true
+        """
+        data = {
+            "input_contained_unavailable_order": True,
+            "unreserved_orders": [],
+        }
+
+        mock_make_request = Mock(return_value=data)
+        monkeypatch.setattr(client, 'make_request', mock_make_request)
+
+        # this should not raise any exceptions
+        client.make_reservation()
+
+        # but this should
+        with pytest.raises(exceptions.OrderUnavailableError):
+            client.make_reservation(raise_on_unavailable_order=True)
+
     def test_get_status(self, client, monkeypatch):
         response = {
             'trolley_contents': {
diff --git a/tests/test_reservation.py b/tests/test_reservation.py
index 91c0895..28bbf74 100644
--- a/tests/test_reservation.py
+++ b/tests/test_reservation.py
@@ -59,3 +59,13 @@ class TestReservation:
 
         assert len(reservation.unreserved_orders) == 1
         assert reservation.minutes_left == 15
+
+    def test_from_api_data_with_unavailable_orders(self):
+        data = {
+            "input_contained_unavailable_order": True,
+            "unreserved_orders": [],
+        }
+
+        reservation = Reservation.from_api_data(data)
+
+        assert reservation.input_contained_unavailable_order is True
diff --git a/tests/test_trolley.py b/tests/test_trolley.py
index fb9b9df..0370757 100644
--- a/tests/test_trolley.py
+++ b/tests/test_trolley.py
@@ -68,6 +68,23 @@ class TestTrolley:
         assert trolley.discarded_orders[0].item == 3
         assert trolley.discarded_orders[1].item == 6
 
+    def test_from_api_data_with_empty_trolley(self):
+        data = {
+            "discarded_orders": [],
+            "input_contained_unavailable_order": True,
+            "trolley_token": "abc123",
+            "trolley_token_contents": {
+                "trolley_bundle_count": 0,
+                "trolley_order_count": 0
+            }
+        }
+
+        trolley = Trolley.from_api_data(data)
+
+        assert trolley.token == 'abc123'
+        assert trolley.input_contained_unavailable_order is True
+
+
     def test_get_events(self):
 
         event_one = Event(id_='abc123')