From e7160d0a02428fe6cf8ae77664347f9753011c88 Mon Sep 17 00:00:00 2001 From: Philipp Metzner Date: Mon, 9 Sep 2024 16:07:22 +0200 Subject: [PATCH 1/7] Rename BoxResult type to plural --- .../business_logic/warehouse/box/mutations.py | 12 ++++---- .../definitions/protected/mutations.graphql | 8 ++--- .../definitions/protected/types.graphql | 10 +++---- back/test/endpoint_tests/test_box.py | 30 +++++++++---------- front/src/types/generated/graphql.ts | 20 ++++++------- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/back/boxtribute_server/business_logic/warehouse/box/mutations.py b/back/boxtribute_server/business_logic/warehouse/box/mutations.py index 73b797ecc..117de70d2 100644 --- a/back/boxtribute_server/business_logic/warehouse/box/mutations.py +++ b/back/boxtribute_server/business_logic/warehouse/box/mutations.py @@ -31,7 +31,7 @@ @dataclass(kw_only=True) -class BoxResult: +class BoxesResult: updated_boxes: list[Box] invalid_box_label_identifiers: list[str] @@ -89,7 +89,7 @@ def resolve_update_box(*_, update_input): # - perform the requested action on all valid boxes # - create list of invalid boxes (difference of the set of input label identifiers and # the set of valid label identifiers) -# - return valid, updated boxes and invalid box label identifiers as BoxResult data +# - return valid, updated boxes and invalid box label identifiers as BoxesResult data # structure for GraphQL API @mutation.field("deleteBoxes") @handle_unauthorized @@ -107,7 +107,7 @@ def resolve_delete_boxes(*_, label_identifiers): ) valid_box_label_identifiers = {box.label_identifier for box in boxes} - return BoxResult( + return BoxesResult( updated_boxes=delete_boxes(user_id=g.user.id, boxes=boxes), invalid_box_label_identifiers=sorted( label_identifiers.difference(valid_box_label_identifiers) @@ -142,7 +142,7 @@ def resolve_move_boxes_to_location(*_, update_input): ) valid_box_label_identifiers = {box.label_identifier for box in boxes} - return BoxResult( + return BoxesResult( updated_boxes=move_boxes_to_location( user_id=g.user.id, boxes=boxes, location=location ), @@ -189,7 +189,7 @@ def resolve_assign_tag_to_boxes(*_, update_input): ) valid_box_label_identifiers = {box.label_identifier for box in boxes} - return BoxResult( + return BoxesResult( updated_boxes=assign_tag_to_boxes(user_id=g.user.id, boxes=boxes, tag=tag), invalid_box_label_identifiers=sorted( label_identifiers.difference(valid_box_label_identifiers) @@ -247,7 +247,7 @@ def resolve_unassign_tag_from_boxes(*_, update_input): ) valid_box_label_identifiers = {box.label_identifier for box in boxes} - return BoxResult( + return BoxesResult( updated_boxes=unassign_tag_from_boxes(user_id=g.user.id, boxes=boxes, tag=tag), invalid_box_label_identifiers=sorted( label_identifiers.difference(valid_box_label_identifiers) diff --git a/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql b/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql index 77b54a175..35e5dcf84 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql +++ b/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql @@ -7,13 +7,13 @@ type Mutation { createBox(creationInput: BoxCreationInput): Box " Update one or more properties of a box with specified label identifier. " updateBox(updateInput: BoxUpdateInput): Box - " Any boxes that are non-existing, already deleted, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. " + " Any boxes that are non-existing, already deleted, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. " deleteBoxes(labelIdentifiers: [String!]!): DeleteBoxesResult - " Any boxes that are non-existing, already inside the requested location, inside a different base other than the one of the requested location, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. " + " Any boxes that are non-existing, already inside the requested location, inside a different base other than the one of the requested location, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. " moveBoxesToLocation(updateInput: BoxMoveInput): MoveBoxesResult - " Any boxes that are non-existing, already assigned to the requested tag, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. " + " Any boxes that are non-existing, already assigned to the requested tag, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. " assignTagToBoxes(updateInput: BoxAssignTagInput): AssignTagToBoxesResult - " Any boxes that are non-existing, don't have the requested tag assigned, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. " + " Any boxes that are non-existing, don't have the requested tag assigned, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. " unassignTagFromBoxes(updateInput: BoxAssignTagInput): UnassignTagFromBoxesResult " Create a new beneficiary in a base, using first/last name, date of birth, and group identifier. Optionally pass tags to assign to the beneficiary. " diff --git a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql index 04b4b9d7a..17a6c9453 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql +++ b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql @@ -212,7 +212,7 @@ type BoxPage { """ Utility response type for box bulk-update mutations, containing both updated boxes and invalid boxes (ignored due to e.g. being deleted, in prohibited base, and/or non-existing). """ -type BoxResult { +type BoxesResult { updatedBoxes: [Box!]! invalidBoxLabelIdentifiers: [String!]! } @@ -607,10 +607,10 @@ type DeletedTagError { name: String! } -union MoveBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | DeletedLocationError -union AssignTagToBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | TagTypeMismatchError | DeletedTagError -union UnassignTagFromBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError -union DeleteBoxesResult = BoxResult | InsufficientPermissionError +union MoveBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | DeletedLocationError +union AssignTagToBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | TagTypeMismatchError | DeletedTagError +union UnassignTagFromBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError +union DeleteBoxesResult = BoxesResult | InsufficientPermissionError union CreateCustomProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | EmptyNameError union EditCustomProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | EmptyNameError | ProductTypeMismatchError union DeleteProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | BoxesStillAssignedToProductError | ProductTypeMismatchError diff --git a/back/test/endpoint_tests/test_box.py b/back/test/endpoint_tests/test_box.py index 6d01bdddd..61d7bf3c7 100644 --- a/back/test/endpoint_tests/test_box.py +++ b/back/test/endpoint_tests/test_box.py @@ -306,7 +306,7 @@ def test_box_mutations( # Test case 8.2.22a, 8.2.22c mutation = f"""mutation {{ moveBoxesToLocation( updateInput: {{ labelIdentifiers: [{label_identifiers}], locationId: {location_id} }} ) {{ - ...on BoxResult {{ updatedBoxes {{ + ...on BoxesResult {{ updatedBoxes {{ location {{ id }} lastModifiedOn history {{ changes }} @@ -342,7 +342,7 @@ def test_box_mutations( # Test case 8.2.23a, 8.2.23c mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -354,7 +354,7 @@ def test_box_mutations( mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -367,7 +367,7 @@ def test_box_mutations( generic_tag_id = str(tags[2]["id"]) mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -383,7 +383,7 @@ def test_box_mutations( mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {another_generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -406,7 +406,7 @@ def test_box_mutations( label_identifier = f'"{created_box["labelIdentifier"]}"' mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifier}], tagId: {generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -427,7 +427,7 @@ def test_box_mutations( # Test case 8.2.24c mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {generic_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -448,7 +448,7 @@ def test_box_mutations( # Test case 8.2.24h mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {beneficiary_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -470,7 +470,7 @@ def test_box_mutations( # Test case 8.2.24i mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {deleted_tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -482,7 +482,7 @@ def test_box_mutations( # Test case 8.2.25 mutation = f"""mutation {{ deleteBoxes( labelIdentifiers: [{label_identifiers}] ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ deletedOn history {{ changes }} @@ -506,7 +506,7 @@ def test_box_mutations( label_identifiers = ",".join(f'"{i}"' for i in raw_label_identifiers) mutation = f"""mutation {{ moveBoxesToLocation( updateInput: {{ labelIdentifiers: [{label_identifiers}], locationId: {location_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -519,7 +519,7 @@ def test_box_mutations( # Test case 8.2.23b, 8.2.23d, 8.2.23j mutation = f"""mutation {{ assignTagToBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -532,7 +532,7 @@ def test_box_mutations( # Test case 8.2.24b, 8.2.24d, 8.2.24j mutation = f"""mutation {{ unassignTagFromBoxes( updateInput: {{ labelIdentifiers: [{label_identifiers}], tagId: {tag_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ tags {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" @@ -544,7 +544,7 @@ def test_box_mutations( # Test cases 8.2.26, 8.2.27, 8.2.28 mutation = f"""mutation {{ deleteBoxes( labelIdentifiers: [{label_identifiers}] ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id }} invalidBoxLabelIdentifiers }} }} }}""" @@ -560,7 +560,7 @@ def test_box_mutations( mutation = f"""mutation {{ moveBoxesToLocation( updateInput: {{ labelIdentifiers: [{label_identifiers}], locationId: {another_location_id} }} ) {{ - ...on BoxResult {{ + ...on BoxesResult {{ updatedBoxes {{ id location {{ id }} }} invalidBoxLabelIdentifiers }} }} }}""" diff --git a/front/src/types/generated/graphql.ts b/front/src/types/generated/graphql.ts index 3c3902a5f..7beb6e6f8 100755 --- a/front/src/types/generated/graphql.ts +++ b/front/src/types/generated/graphql.ts @@ -14,7 +14,7 @@ export type Scalars = { Datetime: any; }; -export type AssignTagToBoxesResult = BoxResult | DeletedTagError | InsufficientPermissionError | ResourceDoesNotExistError | TagTypeMismatchError | UnauthorizedForBaseError; +export type AssignTagToBoxesResult = BoxesResult | DeletedTagError | InsufficientPermissionError | ResourceDoesNotExistError | TagTypeMismatchError | UnauthorizedForBaseError; /** * Representation of a base. @@ -247,8 +247,8 @@ export type BoxPage = { }; /** Utility response type for box bulk-update mutations, containing both updated boxes and invalid boxes (ignored due to e.g. being deleted, in prohibited base, and/or non-existing). */ -export type BoxResult = { - __typename?: 'BoxResult'; +export type BoxesResult = { + __typename?: 'BoxesResult'; invalidBoxLabelIdentifiers: Array; updatedBoxes: Array; }; @@ -370,7 +370,7 @@ export type DataCube = { facts?: Maybe>>; }; -export type DeleteBoxesResult = BoxResult | InsufficientPermissionError; +export type DeleteBoxesResult = BoxesResult | InsufficientPermissionError; export type DeleteProductResult = BoxesStillAssignedToProductError | InsufficientPermissionError | Product | ProductTypeMismatchError | ResourceDoesNotExistError | UnauthorizedForBaseError; @@ -651,7 +651,7 @@ export type MetricsNumberOfSalesArgs = { before?: InputMaybe; }; -export type MoveBoxesResult = BoxResult | DeletedLocationError | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; +export type MoveBoxesResult = BoxesResult | DeletedLocationError | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; export type MovedBoxDataDimensions = { __typename?: 'MovedBoxDataDimensions'; @@ -697,7 +697,7 @@ export type Mutation = { addPackingListEntryToDistributionEvent?: Maybe; assignBoxToDistributionEvent?: Maybe; assignTag?: Maybe; - /** Any boxes that are non-existing, already assigned to the requested tag, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. */ + /** Any boxes that are non-existing, already assigned to the requested tag, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. */ assignTagToBoxes?: Maybe; /** Change state of specified shipment to `Canceled`. Only valid for shipments in `Preparing` state. Any boxes marked for shipment are moved back into stock. The client must be member of either source or target base of the shipment. */ cancelShipment?: Maybe; @@ -722,7 +722,7 @@ export type Mutation = { createTransferAgreement?: Maybe; /** Deactivate beneficiary with specified ID. */ deactivateBeneficiary?: Maybe; - /** Any boxes that are non-existing, already deleted, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. */ + /** Any boxes that are non-existing, already deleted, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. */ deleteBoxes?: Maybe; /** Soft-delete the custom product with specified ID. Return errors if the product is still assigned to any boxes. The client must be member of the base that the product is registered in. */ deleteProduct?: Maybe; @@ -738,7 +738,7 @@ export type Mutation = { enableStandardProduct?: Maybe; /** Change state of specified shipment to `Lost`, and state of all contained `InTransit` boxes to `NotDelivered`. Only valid for shipments in `Sent` state. The client must be member of either source or target base of the shipment. */ markShipmentAsLost?: Maybe; - /** Any boxes that are non-existing, already inside the requested location, inside a different base other than the one of the requested location, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. */ + /** Any boxes that are non-existing, already inside the requested location, inside a different base other than the one of the requested location, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. */ moveBoxesToLocation?: Maybe; moveItemsFromBoxToDistributionEvent?: Maybe; moveItemsFromReturnTrackingGroupToBox?: Maybe; @@ -757,7 +757,7 @@ export type Mutation = { startReceivingShipment?: Maybe; unassignBoxFromDistributionEvent?: Maybe; unassignTag?: Maybe; - /** Any boxes that are non-existing, don't have the requested tag assigned, and/or in a base that the user must not access are returned in the `BoxResult.invalidBoxLabelIdentifiers` list. */ + /** Any boxes that are non-existing, don't have the requested tag assigned, and/or in a base that the user must not access are returned in the `BoxesResult.invalidBoxLabelIdentifiers` list. */ unassignTagFromBoxes?: Maybe; /** Update one or more properties of a beneficiary with specified ID. */ updateBeneficiary?: Maybe; @@ -1827,7 +1827,7 @@ export enum TransferAgreementType { SendingTo = 'SendingTo' } -export type UnassignTagFromBoxesResult = BoxResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; +export type UnassignTagFromBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError; export type UnauthorizedForBaseError = { __typename?: 'UnauthorizedForBaseError'; From 4a5e8e51dbd0c48a23f3ff590007cd826c95936d Mon Sep 17 00:00:00 2001 From: Philipp Metzner Date: Mon, 9 Sep 2024 16:00:39 +0200 Subject: [PATCH 2/7] Make qrCode query and QrCode.box return unions The unions may contain error types with detailed info --- .../warehouse/qr_code/fields.py | 3 +- .../warehouse/qr_code/queries.py | 9 ++++-- back/boxtribute_server/graph_ql/bindables.py | 2 ++ .../definitions/protected/queries.graphql | 4 +-- .../definitions/protected/types.graphql | 7 +++-- .../test_operations.py | 4 +-- back/test/endpoint_tests/test_app.py | 14 ++++----- back/test/endpoint_tests/test_box.py | 10 +++---- back/test/endpoint_tests/test_cron.py | 10 ++++--- back/test/endpoint_tests/test_permissions.py | 29 +++++++++++++----- back/test/endpoint_tests/test_qr.py | 13 ++++---- front/src/queries/queries.ts | 26 +++++++++++++--- front/src/types/generated/graphql.ts | 30 +++++++++++-------- front/src/views/BoxCreate/BoxCreateView.tsx | 4 ++- 14 files changed, 107 insertions(+), 58 deletions(-) diff --git a/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py b/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py index a66cc0c43..b6a03eb73 100644 --- a/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py +++ b/back/boxtribute_server/business_logic/warehouse/qr_code/fields.py @@ -1,6 +1,6 @@ from ariadne import ObjectType -from ....authz import authorize_for_reading_box +from ....authz import authorize_for_reading_box, handle_unauthorized from ....models.definitions.box import Box from ....models.definitions.location import Location @@ -8,6 +8,7 @@ @qr_code.field("box") +@handle_unauthorized def resolve_qr_code_box(qr_code_obj, _): try: box = ( diff --git a/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py b/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py index fa808cc76..df9483f82 100644 --- a/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py +++ b/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py @@ -1,7 +1,8 @@ from ariadne import QueryType -from ....authz import authorize +from ....authz import authorize, handle_unauthorized from ....models.definitions.qr_code import QrCode +from ....models.utils import handle_non_existing_resource query = QueryType() @@ -17,6 +18,8 @@ def resolve_qr_exists(*_, qr_code): @query.field("qrCode") -def resolve_qr_code(*_, qr_code): +@handle_unauthorized +@handle_non_existing_resource +def resolve_qr_code(*_, code): authorize(permission="qr:read") - return QrCode.get(QrCode.code == qr_code) + return QrCode.get(QrCode.code == code) diff --git a/back/boxtribute_server/graph_ql/bindables.py b/back/boxtribute_server/graph_ql/bindables.py index da57ae017..3484f265f 100644 --- a/back/boxtribute_server/graph_ql/bindables.py +++ b/back/boxtribute_server/graph_ql/bindables.py @@ -180,6 +180,8 @@ def resolve_location_type(obj, *_): UnionType("MoveBoxesResult", resolve_type_by_class_name), UnionType("AssignTagToBoxesResult", resolve_type_by_class_name), UnionType("UnassignTagFromBoxesResult", resolve_type_by_class_name), + UnionType("QrCodeResult", resolve_type_by_class_name), + UnionType("BoxResult", resolve_type_by_class_name), ) interface_types = ( InterfaceType("Location", resolve_location_type), diff --git a/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql b/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql index 8dca5d779..0ed1e1396 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql +++ b/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql @@ -15,8 +15,8 @@ type Query { box(labelIdentifier: String!): Box " Return page of non-deleted [`Boxes`]({{Types.Box}}) in base with specified ID. Optionally pass filters " boxes(baseId: ID!, paginationInput: PaginationInput, filterInput: FilterBoxInput): BoxPage! - " Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32) " - qrCode(qrCode: String!): QrCode + " Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32), or an error in case of insufficient permission or missing resource. " + qrCode(code: String!): QrCodeResult! qrExists(qrCode: String): Boolean " Return [`ClassicLocation`]({{Types.ClassicLocation}}) with specified ID. Accessible for clients who are members of the location's base " location(id: ID!): ClassicLocation diff --git a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql index 17a6c9453..a13d13791 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql +++ b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql @@ -65,8 +65,8 @@ Representation of a QR code, possibly associated with a [`Box`]({{Types.Box}}). type QrCode { id: ID! code: String! - " [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated) " - box: Box + " [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated), or an error in case of insufficient permission or missing authorization for box's base " + box: BoxResult createdOn: Datetime } @@ -617,8 +617,11 @@ union DeleteProductResult = Product | InsufficientPermissionError | ResourceDoes union EnableStandardProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | StandardProductAlreadyEnabledForBaseError union EditStandardProductInstantiationResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | InvalidPriceError | ProductTypeMismatchError union DisableStandardProductResult = Product | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | BoxesStillAssignedToProductError | ProductTypeMismatchError + union StandardProductResult = StandardProduct | InsufficientPermissionError | ResourceDoesNotExistError union StandardProductsResult = StandardProductPage | InsufficientPermissionError | UnauthorizedForBaseError +union QrCodeResult = QrCode | InsufficientPermissionError | ResourceDoesNotExistError +union BoxResult = Box | InsufficientPermissionError | UnauthorizedForBaseError type Metrics { """ diff --git a/back/test/auth0_integration_tests/test_operations.py b/back/test/auth0_integration_tests/test_operations.py index 5da948e82..a008dc170 100644 --- a/back/test/auth0_integration_tests/test_operations.py +++ b/back/test/auth0_integration_tests/test_operations.py @@ -15,8 +15,8 @@ def _assert_successful_request(*args, **kwargs): return assert_successful_request(*args, **kwargs, endpoint=endpoint) query = """query BoxIdAndItems { - qrCode(qrCode: "093f65e080a295f8076b1c5722a46aa2") { box { id } } - }""" + qrCode(code: "093f65e080a295f8076b1c5722a46aa2") { + ...on QrCode { box { ...on Box { id } } } } }""" queried_box = _assert_successful_request(auth0_client, query)["box"] assert queried_box == {"id": "100000000"} diff --git a/back/test/endpoint_tests/test_app.py b/back/test/endpoint_tests/test_app.py index 298b1ac6c..4ba3d675c 100644 --- a/back/test/endpoint_tests/test_app.py +++ b/back/test/endpoint_tests/test_app.py @@ -113,13 +113,6 @@ def test_query_non_existent_box(read_only_client): assert "SQL" not in response.json["errors"][0]["message"] -def test_query_non_existent_qr_code(read_only_client): - # Test case 8.1.31 - query = """query { qrCode(qrCode: "-1") { id } }""" - response = assert_bad_user_input(read_only_client, query) - assert "SQL" not in response.json["errors"][0]["message"] - - @pytest.mark.parametrize("resource", ["base", "organisation", "user"]) def test_query_non_existent_resource_for_god_user(read_only_client, mocker, resource): # Test case 99.1.3, 10.1.3 @@ -352,6 +345,13 @@ def test_mutate_resource_does_not_exist( @pytest.mark.parametrize( "operation,query_input,field,response", [ + # Test case 8.1.31 + [ + "qrCode", + 'code: "0"', + "...on ResourceDoesNotExistError { id name }", + {"id": None, "name": "QrCode"}, + ], # Test case 8.1.42 [ "standardProduct", diff --git a/back/test/endpoint_tests/test_box.py b/back/test/endpoint_tests/test_box.py index 61d7bf3c7..d5c90ef9f 100644 --- a/back/test/endpoint_tests/test_box.py +++ b/back/test/endpoint_tests/test_box.py @@ -84,11 +84,8 @@ def test_box_query_by_label_identifier( def test_box_query_by_qr_code(read_only_client, default_box, default_qr_code): # Test case 8.1.5 query = f"""query {{ - qrCode(qrCode: "{default_qr_code['code']}") {{ - box {{ - labelIdentifier - }} - }} + qrCode(code: "{default_qr_code['code']}") {{ + ...on QrCode {{ box {{ ...on Box {{ labelIdentifier }} }} }} }} }}""" queried_box = assert_successful_request(read_only_client, query)["box"] assert queried_box["labelIdentifier"] == default_box["label_identifier"] @@ -984,7 +981,8 @@ def _create_query(label_identifier): return f"""query {{ box(labelIdentifier: "{label_identifier}") {{ id }} }}""" def _create_qr_query(qr_code): - return f"""query {{ qrCode(qrCode: "{qr_code['code']}") {{ box {{ id }} }} }}""" + return f"""query {{ qrCode(code: "{qr_code['code']}") {{ + ...on QrCode {{ box {{ ...on Box {{ id }} }} }} }} }}""" queries = { str(in_transit_box["id"]): _create_query(in_transit_box["label_identifier"]), diff --git a/back/test/endpoint_tests/test_cron.py b/back/test/endpoint_tests/test_cron.py index 4b3074d8f..afc056e5e 100644 --- a/back/test/endpoint_tests/test_cron.py +++ b/back/test/endpoint_tests/test_cron.py @@ -13,7 +13,7 @@ NR_OF_CREATED_TAGS_PER_BASE, NR_OF_DELETED_TAGS_PER_BASE, ) -from utils import assert_bad_user_input, assert_successful_request +from utils import assert_successful_request reseed_db_path = f"{CRON_PATH}/reseed-db" headers = [("X-AppEngine-Cron", "true")] @@ -45,14 +45,16 @@ def test_reseed_db(cron_client, monkeypatch, mocker): # Success; perform actual sourcing of seed (takes about 2s) # Create QR code and verify that it is removed after reseeding - mutation = "mutation { createQrCode { code } }" + mutation = "mutation { createQrCode { id code } }" response = assert_successful_request(cron_client, mutation) code = response["code"] response = cron_client.get(reseed_db_path, headers=headers) assert response.status_code == 200 assert response.json == {"message": "reseed-db job executed"} - query = f"""query {{ qrCode(qrCode: "{code}") {{ id }} }}""" - response = assert_bad_user_input(cron_client, query) + query = f"""query {{ qrCode(code: "{code}") {{ + ...on ResourceDoesNotExistError {{ id name }} }} }}""" + response = assert_successful_request(cron_client, query) + assert response == {"id": None, "name": "QrCode"} # Verify generation of fake data query = "query { tags { id } }" diff --git a/back/test/endpoint_tests/test_permissions.py b/back/test/endpoint_tests/test_permissions.py index 92a0baaf4..24798b47d 100644 --- a/back/test/endpoint_tests/test_permissions.py +++ b/back/test/endpoint_tests/test_permissions.py @@ -58,8 +58,6 @@ def operation_name(operation): [ # Test case 8.1.3 """box( labelIdentifier: "12345678") { id }""", - # Test case 8.1.32 - """qrCode( qrCode: "1337beef" ) { id }""", # Test case 8.1.35 """qrExists( qrCode: "1337beef" )""", ], @@ -306,8 +304,12 @@ def test_invalid_permission_for_qr_code_box( # Verify missing stock:read permission mock_user_for_request(mocker, permissions=["qr:read"]) code = default_qr_code["code"] - query = f"""query {{ qrCode(qrCode: "{code}") {{ box {{ id }} }} }}""" - assert_forbidden_request(read_only_client, query, value={"box": None}) + query = f"""query {{ qrCode(code: "{code}") {{ + ...on QrCode {{ + box {{ ...on InsufficientPermissionError {{ name }} }} + }} }} }}""" + response = assert_successful_request(read_only_client, query) + assert response == {"box": {"name": "stock:read"}} # Test case 8.1.11 # Verify missing base-specific stock:read permission (the QR code belongs to a box @@ -325,9 +327,15 @@ def test_invalid_permission_for_qr_code_box( base_ids=[1], ) code = another_qr_code_with_box["code"] # the associated box is in base ID 3 - query = f"""query {{ qrCode(qrCode: "{code}") {{ - box {{ tags {{ taggedResources {{ ...on Beneficiary {{ id }} }} }} }} }} }}""" - assert_forbidden_request(read_only_client, query, value={"box": None}) + query = f"""query {{ qrCode(code: "{code}") {{ + ...on QrCode {{ + box {{ + ...on UnauthorizedForBaseError {{ id name }} + ...on Box {{ + tags {{ taggedResources {{ ...on Beneficiary {{ id }} }} }} + }} }} }} }} }}""" + response = assert_successful_request(read_only_client, query) + assert response == {"box": {"id": "3", "name": ""}} def test_invalid_permission_for_organisation_bases( @@ -648,6 +656,13 @@ def test_mutate_unauthorized_for_base( @pytest.mark.parametrize( "operation,query_input,field,response", [ + # Test case 8.1.32 + [ + "qrCode", + 'code: "1337beef"', + "...on InsufficientPermissionError { name }", + {"name": "qr:read"}, + ], # Test case 8.1.43 [ "standardProduct", diff --git a/back/test/endpoint_tests/test_qr.py b/back/test/endpoint_tests/test_qr.py index 1b87fe563..3993feb1a 100644 --- a/back/test/endpoint_tests/test_qr.py +++ b/back/test/endpoint_tests/test_qr.py @@ -25,12 +25,12 @@ def test_qr_code_query(read_only_client, default_box, default_qr_code): # Test case 8.1.30 code = default_qr_code["code"] query = f"""query {{ - qrCode(qrCode: "{code}") {{ + qrCode(code: "{code}") {{ ...on QrCode {{ id code - box {{ id }} + box {{ ...on Box {{ id }} }} createdOn - }} + }} }} }}""" queried_code = assert_successful_request(read_only_client, query) assert queried_code == { @@ -44,7 +44,8 @@ def test_qr_code_query(read_only_client, default_box, default_qr_code): def test_code_not_associated_with_box(read_only_client, qr_code_without_box): # Test case 8.1.2a code = qr_code_without_box["code"] - query = f"""query {{ qrCode(qrCode: "{code}") {{ box {{ id }} }} }}""" + query = f"""query {{ qrCode(code: "{code}") {{ + ...on QrCode {{ box {{ ...on Box {{ id }} }} }} }} }}""" qr_code = assert_successful_request(read_only_client, query) assert qr_code == {"box": None} @@ -61,10 +62,10 @@ def test_qr_code_mutation(client, box_without_qr_code): createQrCode(boxLabelIdentifier: "{box_without_qr_code['label_identifier']}") {{ id - box {{ + box {{ ...on Box {{ id numberOfItems - }} + }} }} }} }}""" created_qr_code = assert_successful_request(client, mutation) diff --git a/front/src/queries/queries.ts b/front/src/queries/queries.ts index 4c80f0427..2be559eab 100644 --- a/front/src/queries/queries.ts +++ b/front/src/queries/queries.ts @@ -38,10 +38,28 @@ export const BOX_DETAILS_BY_LABEL_IDENTIFIER_QUERY = gql` export const GET_BOX_LABEL_IDENTIFIER_BY_QR_CODE = gql` ${BOX_BASIC_FIELDS_FRAGMENT} query GetBoxLabelIdentifierForQrCode($qrCode: String!) { - qrCode(qrCode: $qrCode) { - code - box { - ...BoxBasicFields + qrCode(code: $qrCode) { + __typename + ... on QrCode { + code + box { + __typename + ...on Box { + ...BoxBasicFields + } + ...on InsufficientPermissionError { + name + } + ...on UnauthorizedForBaseError { + name + } + } + } + ... on InsufficientPermissionError { + name + } + ... on ResourceDoesNotExistError { + name } } } diff --git a/front/src/types/generated/graphql.ts b/front/src/types/generated/graphql.ts index 7beb6e6f8..d02496f9a 100755 --- a/front/src/types/generated/graphql.ts +++ b/front/src/types/generated/graphql.ts @@ -246,12 +246,7 @@ export type BoxPage = { totalCount: Scalars['Int']; }; -/** Utility response type for box bulk-update mutations, containing both updated boxes and invalid boxes (ignored due to e.g. being deleted, in prohibited base, and/or non-existing). */ -export type BoxesResult = { - __typename?: 'BoxesResult'; - invalidBoxLabelIdentifiers: Array; - updatedBoxes: Array; -}; +export type BoxResult = Box | InsufficientPermissionError | UnauthorizedForBaseError; /** Classificators for [`Box`]({{Types.Box}}) state. */ export enum BoxState { @@ -279,6 +274,13 @@ export type BoxUpdateInput = { tagIdsToBeAdded?: InputMaybe>; }; +/** Utility response type for box bulk-update mutations, containing both updated boxes and invalid boxes (ignored due to e.g. being deleted, in prohibited base, and/or non-existing). */ +export type BoxesResult = { + __typename?: 'BoxesResult'; + invalidBoxLabelIdentifiers: Array; + updatedBoxes: Array; +}; + export type BoxesStillAssignedToProductError = { __typename?: 'BoxesStillAssignedToProductError'; labelIdentifiers: Array; @@ -1194,13 +1196,15 @@ export type ProductTypeMismatchError = { /** Representation of a QR code, possibly associated with a [`Box`]({{Types.Box}}). */ export type QrCode = { __typename?: 'QrCode'; - /** [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated) */ - box?: Maybe; + /** [`Box`]({{Types.Box}}) associated with the QR code (`null` if none associated), or an error in case of insufficient permission or missing authorization for box's base */ + box?: Maybe; code: Scalars['String']; createdOn?: Maybe; id: Scalars['ID']; }; +export type QrCodeResult = InsufficientPermissionError | QrCode | ResourceDoesNotExistError; + export type Query = { __typename?: 'Query'; /** Return [`Base`]({{Types.Base}}) with specified ID. Accessible for clients who are members of this base. */ @@ -1241,8 +1245,8 @@ export type Query = { productCategory?: Maybe; /** Return all [`Products`]({{Types.Product}}) (incl. deleted) that the client is authorized to view. */ products: ProductPage; - /** Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32) */ - qrCode?: Maybe; + /** Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32), or an error in case of insufficient permission or missing resource. */ + qrCode: QrCodeResult; qrExists?: Maybe; /** Return [`Shipment`]({{Types.Shipment}}) with specified ID. Clients are authorized to view a shipment if they're member of either the source or the target base */ shipment?: Maybe; @@ -1373,7 +1377,7 @@ export type QueryProductsArgs = { export type QueryQrCodeArgs = { - qrCode: Scalars['String']; + code: Scalars['String']; }; @@ -1945,7 +1949,7 @@ export type GetBoxLabelIdentifierForQrCodeQueryVariables = Exact<{ }>; -export type GetBoxLabelIdentifierForQrCodeQuery = { __typename?: 'Query', qrCode?: { __typename?: 'QrCode', code: string, box?: { __typename?: 'Box', labelIdentifier: string, state: BoxState, comment?: string | null, lastModifiedOn?: any | null, location?: { __typename?: 'ClassicLocation', id: string, base?: { __typename?: 'Base', id: string } | null } | { __typename?: 'DistributionSpot', id: string, base?: { __typename?: 'Base', id: string } | null } | null, shipmentDetail?: { __typename?: 'ShipmentDetail', id: string, shipment: { __typename?: 'Shipment', id: string } } | null } | null } | null }; +export type GetBoxLabelIdentifierForQrCodeQuery = { __typename?: 'Query', qrCode: { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'QrCode', code: string, box?: { __typename: 'Box', labelIdentifier: string, state: BoxState, comment?: string | null, lastModifiedOn?: any | null, location?: { __typename?: 'ClassicLocation', id: string, base?: { __typename?: 'Base', id: string } | null } | { __typename?: 'DistributionSpot', id: string, base?: { __typename?: 'Base', id: string } | null } | null, shipmentDetail?: { __typename?: 'ShipmentDetail', id: string, shipment: { __typename?: 'Shipment', id: string } } | null } | { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'UnauthorizedForBaseError', name: string } | null } | { __typename: 'ResourceDoesNotExistError', name: string } }; export type CheckIfQrExistsInDbQueryVariables = Exact<{ qrCode: Scalars['String']; @@ -2030,7 +2034,7 @@ export type CreateBoxMutationVariables = Exact<{ }>; -export type CreateBoxMutation = { __typename?: 'Mutation', createBox?: { __typename?: 'Box', labelIdentifier: string, qrCode?: { __typename?: 'QrCode', code: string, box?: { __typename?: 'Box', labelIdentifier: string } | null } | null } | null }; +export type CreateBoxMutation = { __typename?: 'Mutation', createBox?: { __typename?: 'Box', labelIdentifier: string, qrCode?: { __typename?: 'QrCode', code: string, box?: { __typename?: 'Box', labelIdentifier: string } | { __typename?: 'InsufficientPermissionError' } | { __typename?: 'UnauthorizedForBaseError' } | null } | null } | null }; export type BoxByLabelIdentifierAndAllProductsWithBaseIdQueryVariables = Exact<{ baseId: Scalars['ID']; diff --git a/front/src/views/BoxCreate/BoxCreateView.tsx b/front/src/views/BoxCreate/BoxCreateView.tsx index ad9d5c7c9..ffe08bdf4 100644 --- a/front/src/views/BoxCreate/BoxCreateView.tsx +++ b/front/src/views/BoxCreate/BoxCreateView.tsx @@ -72,7 +72,9 @@ export const CREATE_BOX_MUTATION = gql` qrCode { code box { - labelIdentifier + ...on Box { + labelIdentifier + } } } } From 7ee08fb4cc045b1b47e9de68ba691e75eb122ef6 Mon Sep 17 00:00:00 2001 From: Philipp Metzner Date: Mon, 9 Sep 2024 16:50:38 +0200 Subject: [PATCH 3/7] Update argument of qrExists query for consistency --- .../business_logic/warehouse/qr_code/queries.py | 4 ++-- .../graph_ql/definitions/protected/queries.graphql | 2 +- back/test/endpoint_tests/test_permissions.py | 2 +- back/test/endpoint_tests/test_qr.py | 4 ++-- front/src/queries/queries.ts | 2 +- front/src/types/generated/graphql.ts | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py b/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py index df9483f82..8d4909ca9 100644 --- a/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py +++ b/back/boxtribute_server/business_logic/warehouse/qr_code/queries.py @@ -8,10 +8,10 @@ @query.field("qrExists") -def resolve_qr_exists(*_, qr_code): +def resolve_qr_exists(*_, code): authorize(permission="qr:read") try: - QrCode.get_id_from_code(qr_code) + QrCode.get_id_from_code(code) except QrCode.DoesNotExist: return False return True diff --git a/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql b/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql index 0ed1e1396..9861065d8 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql +++ b/back/boxtribute_server/graph_ql/definitions/protected/queries.graphql @@ -17,7 +17,7 @@ type Query { boxes(baseId: ID!, paginationInput: PaginationInput, filterInput: FilterBoxInput): BoxPage! " Return [`QrCode`]({{Types.QrCode}}) with specified code (an MD5 hash in hex format of length 32), or an error in case of insufficient permission or missing resource. " qrCode(code: String!): QrCodeResult! - qrExists(qrCode: String): Boolean + qrExists(code: String): Boolean " Return [`ClassicLocation`]({{Types.ClassicLocation}}) with specified ID. Accessible for clients who are members of the location's base " location(id: ID!): ClassicLocation " Return all [`ClassicLocations`]({{Types.ClassicLocation}}) that the client is authorized to view. " diff --git a/back/test/endpoint_tests/test_permissions.py b/back/test/endpoint_tests/test_permissions.py index 24798b47d..60042af94 100644 --- a/back/test/endpoint_tests/test_permissions.py +++ b/back/test/endpoint_tests/test_permissions.py @@ -59,7 +59,7 @@ def operation_name(operation): # Test case 8.1.3 """box( labelIdentifier: "12345678") { id }""", # Test case 8.1.35 - """qrExists( qrCode: "1337beef" )""", + """qrExists( code: "1337beef" )""", ], ids=operation_name, ) diff --git a/back/test/endpoint_tests/test_qr.py b/back/test/endpoint_tests/test_qr.py index 3993feb1a..3890b8fc8 100644 --- a/back/test/endpoint_tests/test_qr.py +++ b/back/test/endpoint_tests/test_qr.py @@ -8,14 +8,14 @@ def test_qr_exists_query(read_only_client, default_qr_code): # Test case 8.1.33 code = default_qr_code["code"] query = f"""query CheckQrExistence {{ - qrExists(qrCode: "{code}") + qrExists(code: "{code}") }}""" qr_exists = assert_successful_request(read_only_client, query) assert qr_exists # Test case 8.1.34 query = """query CheckQrExistence { - qrExists(qrCode: "000") + qrExists(code: "000") }""" qr_exists = assert_successful_request(read_only_client, query) assert not qr_exists diff --git a/front/src/queries/queries.ts b/front/src/queries/queries.ts index 2be559eab..e23e4bb8b 100644 --- a/front/src/queries/queries.ts +++ b/front/src/queries/queries.ts @@ -67,7 +67,7 @@ export const GET_BOX_LABEL_IDENTIFIER_BY_QR_CODE = gql` export const CHECK_IF_QR_EXISTS_IN_DB = gql` query CheckIfQrExistsInDb($qrCode: String!) { - qrExists(qrCode: $qrCode) + qrExists(code: $qrCode) } `; diff --git a/front/src/types/generated/graphql.ts b/front/src/types/generated/graphql.ts index d02496f9a..9f19208a1 100755 --- a/front/src/types/generated/graphql.ts +++ b/front/src/types/generated/graphql.ts @@ -1382,7 +1382,7 @@ export type QueryQrCodeArgs = { export type QueryQrExistsArgs = { - qrCode?: InputMaybe; + code?: InputMaybe; }; From 93beadb5d94a83ccac0bd1f03bdd7f53b71694c6 Mon Sep 17 00:00:00 2001 From: Philipp Metzner Date: Mon, 9 Sep 2024 17:22:59 +0200 Subject: [PATCH 4/7] Add base and organisation names to UnauthorizedForBaseError --- back/boxtribute_server/authz.py | 9 ++++++++- back/boxtribute_server/errors.py | 8 ++++++-- .../definitions/protected/types.graphql | 3 +++ back/test/endpoint_tests/test_app.py | 8 ++++---- back/test/endpoint_tests/test_permissions.py | 17 ++++++++++++++--- front/src/queries/queries.ts | 1 + front/src/types/generated/graphql.ts | 5 ++++- 7 files changed, 40 insertions(+), 11 deletions(-) diff --git a/back/boxtribute_server/authz.py b/back/boxtribute_server/authz.py index 1052eae1d..f4e71c286 100644 --- a/back/boxtribute_server/authz.py +++ b/back/boxtribute_server/authz.py @@ -12,6 +12,7 @@ from .errors import InsufficientPermission, UnauthorizedForBase from .exceptions import Forbidden from .models.definitions.base import Base +from .models.definitions.organisation import Organisation from .models.definitions.shipment import Shipment from .models.definitions.shipment_detail import ShipmentDetail from .models.definitions.transfer_agreement import TransferAgreement @@ -67,7 +68,13 @@ def inner(*args, **kwargs): if e.permission is not None: return InsufficientPermission(name=e.permission) elif e.resource == "base": - return UnauthorizedForBase(id=e.value, name="") + base = ( + Base.select(Base.id, Base.name, Organisation.name) + .join(Organisation) + .where(Base.id == e.value) + .get_or_none() + ) + return UnauthorizedForBase(id=e.value, base=base) raise e return inner diff --git a/back/boxtribute_server/errors.py b/back/boxtribute_server/errors.py index eb3ec3d2e..572d541fe 100644 --- a/back/boxtribute_server/errors.py +++ b/back/boxtribute_server/errors.py @@ -26,9 +26,13 @@ def __init__(self, *, name): class UnauthorizedForBase(UserError): - def __init__(self, *, id, name): + def __init__(self, *, id, base): self.id = id - self.name = name + self.name = "" + self.organisation_name = "" + if base is not None: + self.name = base.name + self.organisation_name = base.organisation.name class BoxesStillAssignedToProduct(UserError): diff --git a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql index a13d13791..fe4716969 100644 --- a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql +++ b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql @@ -579,7 +579,10 @@ type ResourceDoesNotExistError { type UnauthorizedForBaseError { " e.g. 'product:write' present but not for requested base " id: ID! + " Empty string if base does not exist " name: String! + " Empty string if base does not exist " + organisationName: String! } type InvalidPriceError { value: Int! diff --git a/back/test/endpoint_tests/test_app.py b/back/test/endpoint_tests/test_app.py index 4ba3d675c..ddd1d7621 100644 --- a/back/test/endpoint_tests/test_app.py +++ b/back/test/endpoint_tests/test_app.py @@ -222,8 +222,8 @@ def test_update_non_existent_resource( "createCustomProduct", """creationInput: { baseId: 0, name: "a", categoryId: 1, sizeRangeId: 1, gender: none}""", - "...on UnauthorizedForBaseError { id }", - {"id": "0"}, + "...on UnauthorizedForBaseError { id name organisationName }", + {"id": "0", "name": "", "organisationName": ""}, ], # Test case 8.2.37 [ @@ -273,8 +273,8 @@ def test_update_non_existent_resource( [ "enableStandardProduct", """enableInput: { baseId: 0, standardProductId: 2 }""", - "...on UnauthorizedForBaseError { id }", - {"id": "0"}, + "...on UnauthorizedForBaseError { id name organisationName }", + {"id": "0", "name": "", "organisationName": ""}, ], # Test case 8.2.63 [ diff --git a/back/test/endpoint_tests/test_permissions.py b/back/test/endpoint_tests/test_permissions.py index 60042af94..3c087cacf 100644 --- a/back/test/endpoint_tests/test_permissions.py +++ b/back/test/endpoint_tests/test_permissions.py @@ -298,7 +298,12 @@ def test_invalid_permission_for_location_boxes(read_only_client, mocker): def test_invalid_permission_for_qr_code_box( - read_only_client, mocker, default_qr_code, another_qr_code_with_box + read_only_client, + mocker, + default_qr_code, + another_qr_code_with_box, + another_base, + another_organisation, ): # Test case 8.1.10 # Verify missing stock:read permission @@ -330,12 +335,18 @@ def test_invalid_permission_for_qr_code_box( query = f"""query {{ qrCode(code: "{code}") {{ ...on QrCode {{ box {{ - ...on UnauthorizedForBaseError {{ id name }} + ...on UnauthorizedForBaseError {{ id name organisationName }} ...on Box {{ tags {{ taggedResources {{ ...on Beneficiary {{ id }} }} }} }} }} }} }} }}""" response = assert_successful_request(read_only_client, query) - assert response == {"box": {"id": "3", "name": ""}} + assert response == { + "box": { + "id": "3", + "name": another_base["name"], + "organisationName": another_organisation["name"], + } + } def test_invalid_permission_for_organisation_bases( diff --git a/front/src/queries/queries.ts b/front/src/queries/queries.ts index e23e4bb8b..21d9e8a2f 100644 --- a/front/src/queries/queries.ts +++ b/front/src/queries/queries.ts @@ -52,6 +52,7 @@ export const GET_BOX_LABEL_IDENTIFIER_BY_QR_CODE = gql` } ...on UnauthorizedForBaseError { name + organisationName } } } diff --git a/front/src/types/generated/graphql.ts b/front/src/types/generated/graphql.ts index 9f19208a1..839bf6691 100755 --- a/front/src/types/generated/graphql.ts +++ b/front/src/types/generated/graphql.ts @@ -1837,7 +1837,10 @@ export type UnauthorizedForBaseError = { __typename?: 'UnauthorizedForBaseError'; /** e.g. 'product:write' present but not for requested base */ id: Scalars['ID']; + /** Empty string if base does not exist */ name: Scalars['String']; + /** Empty string if base does not exist */ + organisationName: Scalars['String']; }; export type UnboxedItemsCollection = ItemsCollection & { @@ -1949,7 +1952,7 @@ export type GetBoxLabelIdentifierForQrCodeQueryVariables = Exact<{ }>; -export type GetBoxLabelIdentifierForQrCodeQuery = { __typename?: 'Query', qrCode: { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'QrCode', code: string, box?: { __typename: 'Box', labelIdentifier: string, state: BoxState, comment?: string | null, lastModifiedOn?: any | null, location?: { __typename?: 'ClassicLocation', id: string, base?: { __typename?: 'Base', id: string } | null } | { __typename?: 'DistributionSpot', id: string, base?: { __typename?: 'Base', id: string } | null } | null, shipmentDetail?: { __typename?: 'ShipmentDetail', id: string, shipment: { __typename?: 'Shipment', id: string } } | null } | { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'UnauthorizedForBaseError', name: string } | null } | { __typename: 'ResourceDoesNotExistError', name: string } }; +export type GetBoxLabelIdentifierForQrCodeQuery = { __typename?: 'Query', qrCode: { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'QrCode', code: string, box?: { __typename: 'Box', labelIdentifier: string, state: BoxState, comment?: string | null, lastModifiedOn?: any | null, location?: { __typename?: 'ClassicLocation', id: string, base?: { __typename?: 'Base', id: string } | null } | { __typename?: 'DistributionSpot', id: string, base?: { __typename?: 'Base', id: string } | null } | null, shipmentDetail?: { __typename?: 'ShipmentDetail', id: string, shipment: { __typename?: 'Shipment', id: string } } | null } | { __typename: 'InsufficientPermissionError', name: string } | { __typename: 'UnauthorizedForBaseError', name: string, organisationName: string } | null } | { __typename: 'ResourceDoesNotExistError', name: string } }; export type CheckIfQrExistsInDbQueryVariables = Exact<{ qrCode: Scalars['String']; From 7f7c59dd8f390be41a3213c2fcd1facc5f88a815 Mon Sep 17 00:00:00 2001 From: Felipe Henrich Date: Fri, 20 Sep 2024 11:50:44 -0300 Subject: [PATCH 5/7] feat: display an alert message if box is not owned by user base and org --- .../components/QrReader/QrReaderContainer.tsx | 31 ++++++++++++++----- front/src/hooks/useQrResolver.ts | 2 +- front/src/views/BoxCreate/BoxCreateView.tsx | 4 +-- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/front/src/components/QrReader/QrReaderContainer.tsx b/front/src/components/QrReader/QrReaderContainer.tsx index 1fa2bdeca..755cc7dd7 100644 --- a/front/src/components/QrReader/QrReaderContainer.tsx +++ b/front/src/components/QrReader/QrReaderContainer.tsx @@ -34,6 +34,7 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) { const [isMultiBox, setIsMultiBox] = useState(!!qrReaderOverlayState.isMultiBox); const [isProcessingQrCode, setIsProcessingQrCode] = useState(false); const [isCameraNotPermited, setIsCameraNotPermited] = useState(false); + const [boxNotOnwned, setBoxNotOnwned] = useState(""); const setIsProcessingQrCodeDelayed = useCallback( (state: boolean) => { setTimeout(() => { @@ -69,6 +70,7 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) { const onScan = async (qrReaderResultText: string, multiScan: boolean) => { if (!isProcessingQrCode) { setIsProcessingQrCode(true); + setBoxNotOnwned(""); const qrResolvedValue: IQrResolvedValue = await resolveQrCode( qrReaderResultText, multiScan ? "cache-first" : "network-only", @@ -76,16 +78,23 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) { switch (qrResolvedValue.kind) { case IQrResolverResultKind.SUCCESS: { const boxLabelIdentifier = qrResolvedValue.box.labelIdentifier; - if (!multiScan) { - const boxBaseId = qrResolvedValue.box.location.base.id; + if (qrResolvedValue.box.__typename === "UnauthorizedForBaseError") { + setBoxNotOnwned( + `This box it at base ${qrResolvedValue.box.name}, which belongs to organization ${qrResolvedValue.box.organisationName}.`, + ); setIsProcessingQrCode(false); - onSuccess(); - navigate(`/bases/${boxBaseId}/boxes/${boxLabelIdentifier}`); } else { - // Only execute for Multi Box tab - // add box reference to query for list of all scanned boxes - await addBoxToScannedBoxes(qrResolvedValue.box); - setIsProcessingQrCode(false); + if (!multiScan) { + const boxBaseId = qrResolvedValue.box.location.base.id; + setIsProcessingQrCode(false); + onSuccess(); + navigate(`/bases/${boxBaseId}/boxes/${boxLabelIdentifier}`); + } else { + // Only execute for Multi Box tab + // add box reference to query for list of all scanned boxes + await addBoxToScannedBoxes(qrResolvedValue.box); + setIsProcessingQrCode(false); + } } break; } @@ -170,6 +179,12 @@ function QrReaderContainer({ onSuccess }: IQrReaderContainerProps) {
)} + {boxNotOnwned !== "" && ( + <> + +
+ + )} setIsMultiBox(index === 1)} diff --git a/front/src/hooks/useQrResolver.ts b/front/src/hooks/useQrResolver.ts index 842ed0021..edf0228aa 100644 --- a/front/src/hooks/useQrResolver.ts +++ b/front/src/hooks/useQrResolver.ts @@ -52,7 +52,7 @@ export const useQrResolver = () => { }) .then(({ data, errors }) => { if ((errors?.length || 0) > 0) { - const errorCode = errors ? errors[0].extensions.code : undefined; + const errorCode = errors ? errors[0]?.extensions?.code : undefined; if (errorCode === "FORBIDDEN") { triggerError({ message: "You don't have permission to access this box!", diff --git a/front/src/views/BoxCreate/BoxCreateView.tsx b/front/src/views/BoxCreate/BoxCreateView.tsx index ffe08bdf4..fd228143b 100644 --- a/front/src/views/BoxCreate/BoxCreateView.tsx +++ b/front/src/views/BoxCreate/BoxCreateView.tsx @@ -72,7 +72,7 @@ export const CREATE_BOX_MUTATION = gql` qrCode { code box { - ...on Box { + ... on Box { labelIdentifier } } @@ -177,7 +177,7 @@ function BoxCreateView() { }) .then((mutationResult) => { if (mutationResult.errors) { - const errorCode = mutationResult.errors[0].extensions.code; + const errorCode = mutationResult.errors[0]?.extensions?.code; if (errorCode === "BAD_USER_INPUT") { triggerError({ message: "The QR code is already used for another box.", From 57dd97537a87df0804f33abb45ef3f66f01e58ca Mon Sep 17 00:00:00 2001 From: Felipe Henrich Date: Fri, 20 Sep 2024 12:19:35 -0300 Subject: [PATCH 6/7] fix: expected QrCode type to access box property --- front/src/hooks/useQrResolver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/front/src/hooks/useQrResolver.ts b/front/src/hooks/useQrResolver.ts index edf0228aa..18a07997a 100644 --- a/front/src/hooks/useQrResolver.ts +++ b/front/src/hooks/useQrResolver.ts @@ -79,7 +79,7 @@ export const useQrResolver = () => { qrHash: hash, } as IQrResolvedValue; } - if (!data?.qrCode?.box) { + if (data?.qrCode?.__typename === "QrCode" && !data?.qrCode?.box) { return { kind: IQrResolverResultKind.NOT_ASSIGNED_TO_BOX, qrHash: hash, @@ -88,7 +88,7 @@ export const useQrResolver = () => { return { kind: IQrResolverResultKind.SUCCESS, qrHash: hash, - box: data?.qrCode?.box, + box: data?.qrCode?.__typename === "QrCode" && data?.qrCode?.box, } as IQrResolvedValue; }) .catch((err) => { From b63688e7c4642d35cf040cddd47b7833430401fa Mon Sep 17 00:00:00 2001 From: Felipe Henrich Date: Fri, 20 Sep 2024 12:27:32 -0300 Subject: [PATCH 7/7] fix: revert expected QrCode type to access box property --- front/src/hooks/useQrResolver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/front/src/hooks/useQrResolver.ts b/front/src/hooks/useQrResolver.ts index 18a07997a..edf0228aa 100644 --- a/front/src/hooks/useQrResolver.ts +++ b/front/src/hooks/useQrResolver.ts @@ -79,7 +79,7 @@ export const useQrResolver = () => { qrHash: hash, } as IQrResolvedValue; } - if (data?.qrCode?.__typename === "QrCode" && !data?.qrCode?.box) { + if (!data?.qrCode?.box) { return { kind: IQrResolverResultKind.NOT_ASSIGNED_TO_BOX, qrHash: hash, @@ -88,7 +88,7 @@ export const useQrResolver = () => { return { kind: IQrResolverResultKind.SUCCESS, qrHash: hash, - box: data?.qrCode?.__typename === "QrCode" && data?.qrCode?.box, + box: data?.qrCode?.box, } as IQrResolvedValue; }) .catch((err) => {