boxtribute icon indicating copy to clipboard operation
boxtribute copied to clipboard

Update tag management mutations to return union types for enhanced error handling

Open Copilot opened this issue 4 months ago β€’ 3 comments

Task: Update ManageTags back-end implementation for enhanced error handling

Changes completed:

GraphQL Schema Changes

  • [x] Updated createTag, updateTag, and deleteTag mutations to return union types
  • [x] Added InvalidColorError type definition
  • [x] Defined union types: CreateTagResult, UpdateTagResult, DeleteTagResult

Backend Implementation

  • [x] Added InvalidColor error class to errors.py
  • [x] Updated tag/mutations.py with @handle_unauthorized decorator and validation
  • [x] Updated tag/crud.py with validations and decorators
  • [x] Added union type bindings in bindables.py

Test Updates (Original)

  • [x] Updated all tag mutation tests to handle union return types
  • [x] Added validation tests for invalid color, empty name, deleted tags
  • [x] Updated test_app.py and test_permissions.py
  • [x] All 409 endpoint tests pass

Test Updates (PR Review Feedback)

  • [x] Removed special handling for deleteTag from test_mutation_non_existent_resource
  • [x] Removed special handling for updateTag from test_update_non_existent_resource
  • [x] Added updateTag and deleteTag test cases to test_mutate_resource_does_not_exist
  • [x] Added test cases for createTag, updateTag, deleteTag to test_mutate_unauthorized_for_base using tag ID 4
  • [x] Removed __typename checks from tag management tests where result is Tag
  • [x] Removed duplicate tests (test_update_non_existing_tag, test_delete_non_existing_tag)
  • [x] Regenerated GraphQL TypeScript types with pnpm graphql-gen

Testing

  • βœ… All linting passes
  • βœ… All endpoint tests pass
  • βœ… GraphQL types regenerated successfully
Original prompt

This section details on the original issue you should resolve

<issue_title>Update ManageTags back-end implementation</issue_title> <issue_description>## Goal

The tag-management operations need an update for enhanced error handling in the GraphQL API.

Task

Update the schema acc. to the diff:

diff --git a/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql b/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql
index a6086d074..c9fba916f 100644
--- a/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql
+++ b/back/boxtribute_server/graph_ql/definitions/protected/mutations.graphql
@@ -28,11 +28,11 @@ type Mutation {
   createBeneficiaries(creationInput: BeneficiariesCreationInput): CreateBeneficiariesResult
 
   " Create a new tag for a base, described by name, color and type. "
-  createTag(creationInput: TagCreationInput): Tag
+  createTag(creationInput: TagCreationInput): CreateTagResult
   " Update one or more properties of a tag with specified ID. "
-  updateTag(updateInput: TagUpdateInput): Tag
+  updateTag(updateInput: TagUpdateInput): UpdateTagResult
   " Soft-delete tag with specified ID. "
-  deleteTag(id: ID!): Tag
+  deleteTag(id: ID!): DeleteTagResult
   " Assign a tag to a resource (box or beneficiary). If the resource already has this tag assigned, do nothing "
   assignTag(assignmentInput: TagOperationInput): TaggableResource
   " Remove a tag from a resource (box or beneficiary). If the resource does not have this tag assigned, do nothing "
diff --git a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql
index 42bee9241..a161955ac 100644
--- a/back/boxtribute_server/graph_ql/definitions/protected/types.graphql
+++ b/back/boxtribute_server/graph_ql/definitions/protected/types.graphql
@@ -700,9 +700,15 @@ type DeletedBaseError {
 type InvalidDateError {
   date: Datetime!
 }
+type InvalidColorError {
+  color: String!
+}
 
 union MoveBoxesResult = BoxesResult | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | DeletedLocationError
 union DeleteBoxesResult = BoxesResult | InsufficientPermissionError
+union CreateTagResult = Tag | InsufficientPermissionError | UnauthorizedForBaseError | InvalidColorError
+union UpdateTagResult = Tag | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError | EmptyNameError | DeletedTagError | InvalidColorError
+union DeleteTagResult = Tag | InsufficientPermissionError | ResourceDoesNotExistError | UnauthorizedForBaseError
 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

This requires changes in the .../tag/mutations.py and .../tag/crud.py files, along with the corresponding tests.

  • in general, follow the structure used for the createCustomProduct/editCustomProduct/deleteProduct mutations, especially
    • use of handle_unauthorized() decorator for resolver
    • validation for ResourceDoesNotExist
    • use of handle_non_existing_resource() decorator for crud functions
  • when updating a tag, run more validation
    • if requested name is empty, return EmptyNameError
    • if tag deleted, return DeletedTagError
  • validate if the requested color is a color hex code when creating/updating a tag
  • ignore deleting a deleted tag
  • update the test cases in test_tag.py (general functionality and validation issues), in test_app.py (non-existing tag), and in test_permissions.py
  • do not change the implementation of the assignTag/unassignTag mutations
  • note that errors have the Error suffix in the GraphQL schema but the corresponding Python object lack it (logic in bindables.py)

Testing

  • make sure that git ls-files . | xargs pre-commit run --files passes
  • make sure that pytest -s -vv back/test/endpoint_tests/ passes </issue_description>

Comments on the Issue (you are @copilot in this section)

  • Fixes boxwise/boxtribute#2463

πŸ’‘ You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot avatar Nov 18 '25 09:11 Copilot

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 77.61%. Comparing base (1a9f83d) to head (9f41ef0).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2464      +/-   ##
==========================================
+ Coverage   77.58%   77.61%   +0.03%     
==========================================
  Files         277      277              
  Lines       19500    19545      +45     
  Branches     1924     1937      +13     
==========================================
+ Hits        15129    15170      +41     
- Misses       4327     4329       +2     
- Partials       44       46       +2     
Flag Coverage Ξ”
backend 99.54% <ΓΈ> (-0.08%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sentry[bot] avatar Nov 18 '25 09:11 sentry[bot]

some changes in the tests required @copilot

  • add test cases to test_mutate_unauthorized_for_base for the three tag management operations, using tag ID 4 (from unautzed base)
  • please also run `pnpm ...

Done in commit 9953b0b. Added test cases for createTag, updateTag, and deleteTag to test_mutate_unauthorized_for_base using tag ID 4 (which is in base 2, an unauthorized base). Also ran pnpm graphql-gen and committed the generated TypeScript type files.

Copilot avatar Nov 18 '25 10:11 Copilot

https://trello.com/c/TH4HxQEk

pylipp avatar Nov 18 '25 12:11 pylipp