Merge pull request #1948 from boxwise/agreement-shipment-fixes

Avoid creating agreement without active partner bases

back/boxtribute_server/business_logic/box_transfer/agreement/crud.py

@@ -13,6 +13,7 @@
     InvalidTransferAgreementDates,
     InvalidTransferAgreementOrganisation,
     InvalidTransferAgreementState,
+    NoActivePartnerBases,
 )
 from ....models.definitions.base import Base
 from ....models.definitions.transfer_agreement import TransferAgreement
@@ -154,6 +155,8 @@ def create_transfer_agreement(
     is not part of the source/target organisation.
     Raise InvalidTransferAgreementDates exception if valid_from is not earlier than
     valid_until.
+    Raise NoActivePartnerBases exception if specified partner bases (default: all bases
+    of the partner organisation) are inactive.
     """
     if initiating_organisation_id == partner_organisation_id:
         raise InvalidTransferAgreementOrganisation()
@@ -176,6 +179,9 @@ def create_transfer_agreement(
     else:
         partner_organisation_base_ids = set(partner_organisation_base_ids)
 
+    if not partner_organisation_base_ids:
+        raise NoActivePartnerBases()
+
     _validate_unique_transfer_agreement(
         organisation_ids={initiating_organisation_id, partner_organisation_id},
         base_ids=initiating_organisation_base_ids.union(partner_organisation_base_ids),

back/boxtribute_server/business_logic/box_transfer/shipment/crud.py

@@ -316,7 +316,8 @@ def _remove_boxes_from_shipment(
     """With `box_state=InStock`, return boxes to stock; with `box_state=NotDelivered`,
     mark boxes as lost during shipment. Soft-delete corresponding shipment details.
     If boxes are requested to be removed that are not contained in the given shipment,
-    they are silently discarded.
+    or in an invalid state for the operation (e.g. only MarkedForShipment boxes can be
+    moved back to InStock during shipment preparation), they are silently discarded.
     """
     box_label_identifiers = box_label_identifiers or []
     if not box_label_identifiers:
@@ -326,13 +327,16 @@ def _remove_boxes_from_shipment(
 
     if box_state == BoxState.InStock:
         fields = [ShipmentDetail.removed_on, ShipmentDetail.removed_by]
+        old_box_states = [BoxState.MarkedForShipment]
     else:
         fields = [ShipmentDetail.lost_on, ShipmentDetail.lost_by]
+        old_box_states = [BoxState.InTransit, BoxState.Receiving]
 
     details = []
     for detail in _retrieve_shipment_details(
         shipment_id,
         (Box.label_identifier << box_label_identifiers),
+        Box.state << old_box_states,
     ):
         setattr(detail, fields[0].name, now)
         setattr(detail, fields[1].name, user_id)

back/boxtribute_server/business_logic/core/organisation/queries.py

@@ -9,7 +9,7 @@
 @query.field("organisations")
 def resolve_organisations(*_):
     authorize(permission="organisation:read")
-    return Organisation.select()
+    return Organisation.select().where(Organisation.deleted_on.is_null())
 
 
 @query.field("organisation")

back/boxtribute_server/exceptions.py

@@ -134,6 +134,13 @@ def __init__(self, *args, agreement_id, **kwargs):
         }
 
 
+class NoActivePartnerBases(Exception):
+    extensions = {
+        "code": "BAD_USER_INPUT",
+        "description": "None of the requested partner bases is active.",
+    }
+
+
 class InvalidShipmentState(_InvalidResourceState):
     def __init__(self, *args, expected_states, actual_state, **kwargs):
         super().__init__(

back/boxtribute_server/graph_ql/definitions/protected/queries.graphql

@@ -5,7 +5,7 @@ type Query {
   base(id: ID!): Base
   " Return [`Organisation`]({{Types.Organisation}}) with specified ID. "
   organisation(id: ID!): Organisation
-  " Return all [`Organisations`]({{Types.Organisation}}). "
+  " Return all non-deleted [`Organisations`]({{Types.Organisation}}). "
   organisations: [Organisation!]!
   " Return [`User`]({{Types.User}}) with specified ID. Some fields might be restricted. "
   user(id: ID): User

back/boxtribute_server/models/definitions/organisation.py

@@ -14,7 +14,7 @@ class Organisation(db.Model):  # type: ignore
         on_delete="SET NULL",
         on_update="CASCADE",
     )
-    deleted = DateTimeField(null=True, default=None)
+    deleted_on = DateTimeField(null=True, default=None, column_name="deleted")
     name = CharField(column_name="label")
     modified = DateTimeField(null=True)
     modified_by = UIntForeignKeyField(

back/init.sql

@@ -1346,6 +1346,8 @@ LOCK TABLES `organisations` WRITE;
 /*!40000 ALTER TABLE `organisations` DISABLE KEYS */;
 INSERT INTO `organisations` VALUES (1,'BoxAid',NULL,NULL,NULL,NULL,NULL),
   (2,'BoxCare',NULL,NULL,NULL,NULL,NULL),
+  (3,'NoCampsOrg',NULL,NULL,NULL,NULL,NULL),
+  (4,'TheDeletedOrg',NULL,NULL,'2023-12-31 11:22:33',NULL,NULL),
   (100000000,'TestOrganisation','2019-07-10 08:05:56',1,NULL,NULL,NULL),
   (100000001,'DummyTestOrgWithBoxes','2019-09-29 08:05:56',1,NULL,NULL,NULL);
 /*!40000 ALTER TABLE `organisations` ENABLE KEYS */;

back/minimal.sql

@@ -2089,6 +2089,8 @@ LOCK TABLES `organisations` WRITE;
 INSERT INTO `organisations` VALUES
   (1,'BoxAid',NULL,NULL,NULL,NULL,NULL),
   (2,'BoxCare',NULL,NULL,NULL,NULL,NULL),
+  (3,'NoCampsOrg',NULL,NULL,NULL,NULL,NULL),
+  (4,'TheDeletedOrg',NULL,NULL,'2023-12-31 11:22:33',NULL,NULL),
   (100000000,'TestOrganisation','2019-07-10 08:05:56',1,NULL,NULL,NULL),
   (100000001,'DummyTestOrgWithBoxes','2019-09-29 08:05:56',1,NULL,NULL,NULL);
 /*!40000 ALTER TABLE `organisations` ENABLE KEYS */;

back/test/auth0_integration_tests/test_operations.py

@@ -50,7 +50,7 @@ def _assert_successful_request(*args, **kwargs):
             "shipments",
             "users",
         ],
-        [6, 4, 31, 18, 72, 5, 10, 43],
+        [6, 5, 31, 18, 72, 5, 10, 43],
     ):
         query = f"query {{ {resource} {{ id }} }}"
         response = _assert_successful_request(auth0_client, query, field=resource)

back/test/data/organisation.py

@@ -1,20 +1,26 @@
 import pytest
 from boxtribute_server.models.definitions.organisation import Organisation
+from boxtribute_server.models.utils import utcnow
+
+now = utcnow()
 
 
 def data():
     return [
         {
             "id": 1,
             "name": "CoolOrganisation",
+            "deleted_on": None,
         },
         {
             "id": 2,
             "name": "Helpers",
+            "deleted_on": None,
         },
         {
             "id": 3,
             "name": "Inactives",
+            "deleted_on": now,
         },
     ]
 

back/test/endpoint_tests/test_organisation.py

@@ -76,4 +76,4 @@ def test_organisations_query(read_only_client, organisations):
     # Test case 99.1.7
     query = """query { organisations { name } }"""
     queried_organisations = assert_successful_request(read_only_client, query)
-    assert queried_organisations == [{"name": org["name"]} for org in organisations]
+    assert queried_organisations == [{"name": org["name"]} for org in organisations[:2]]

back/test/endpoint_tests/test_transfer_agreement.py

@@ -230,6 +230,13 @@ def _create_mutation(creation_input):
         validUntil: "{valid_until}" """
     assert_bad_user_input(client, _create_mutation(creation_input))
 
+    creation_input = f"""partnerOrganisationId: {another_organisation['id']},
+        initiatingOrganisationId: {default_organisation['id']}
+        initiatingOrganisationBaseIds: [2]
+        partnerOrganisationBaseIds: []
+        type: {TransferAgreementType.Bidirectional.name} """
+    assert_bad_user_input(client, _create_mutation(creation_input))
+
     # Test case 2.2.2
     creation_input = f"""partnerOrganisationId: {another_organisation['id']},
         initiatingOrganisationId: {default_organisation['id']}

front/src/views/Transfers/CreateTransferAgreement/CreateTransferAgreementView.tsx

@@ -143,8 +143,9 @@ function CreateTransferAgreementView() {
     bases: globalPreferences?.availableBases,
   } as IBasesForOrganisationData;
 
+  // Filter out organisations without active bases
   const partnerOrganisationsWithTheirBasesData = allOrgsAndTheirBases?.filter(
-    (organisation) => organisation.id !== globalPreferences.organisation?.id,
+    (organisation) => (organisation.id !== globalPreferences.organisation?.id) && ((organisation.bases?.length || 0) > 0),
   );
 
   // Handle Submission

graphql/generated/schema.graphql

@@ -491,7 +491,7 @@ type Query {
   base(id: ID!): Base
   " Return [`Organisation`]({{Types.Organisation}}) with specified ID. "
   organisation(id: ID!): Organisation
-  " Return all [`Organisations`]({{Types.Organisation}}). "
+  " Return all non-deleted [`Organisations`]({{Types.Organisation}}). "
   organisations: [Organisation!]!
   " Return [`User`]({{Types.User}}) with specified ID. Some fields might be restricted. "
   user(id: ID): User