Baf 1097/missing order states for orders #35
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 9 commits
May 21, 2025 10:05
…(or does not have any state)
replace calls to list() and dict() with literals, check empty lists using boolean, not len
WalkthroughThis update primarily refactors code for improved readability and Pythonic style by replacing explicit length and constructor checks (e.g., Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrderController
participant DB
Client->>OrderController: GET /orders/{order_id}
OrderController->>DB: Retrieve order and last state
alt Order or last state not found
OrderController->>Client: 404 Not Found
else Order and last state found
OrderController->>Client: 200 OK with order data
end
sequenceDiagram
participant TestThread1 as Thread 1 (Delete Order)
participant TestThread2 as Thread 2 (Get Orders)
participant API
participant DB
TestThread1->>API: DELETE /orders/{order_id}
API->>DB: Delete order and states
loop Multiple times
TestThread2->>API: GET /orders
API->>DB: Retrieve all orders with last state
API->>TestThread2: Return orders (each with lastState)
end
Suggested reviewers
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
fleet_management_api/api_impl/controllers/platform_hw.py (1)
66-74: Consider removing unnecessary else after return.Following the static analysis suggestion, you can improve readability by removing the else clause and de-indenting the code:
if not hws: return _log_info_and_respond( f"Platform HW with ID={platform_hw_id} was not found.", 404, title=_OBJ_NOT_FOUND, ) -else: - _log_info(f"Found {len(hws)} platform HWs with ID={platform_hw_id}") - return _json_response(hws[0]) +_log_info(f"Found {len(hws)} platform HWs with ID={platform_hw_id}") +return _json_response(hws[0])fleet_management_api/api_impl/api_keys.py (1)
64-67: Consider removing unnecessary else after return.Following the static analysis suggestion, you can improve readability by removing the else clause:
if not _key_db_models: return 401, "Invalid API key used." -else: - return 200, _key_db_models[0] +return 200, _key_db_models[0]fleet_management_api/api_impl/controllers/route_visualization.py (1)
31-40: Consider removing unnecessary else after return.Following the static analysis suggestion, you can improve readability by removing the else clause:
if not rp_db_models: return _error( 404, f"Route visualization (route ID={route_id}) was not found.", title=_OBJ_NOT_FOUND, ) -else: - rp = _obj_to_db.route_visualization_from_db_model(rp_db_models[0]) - _log_info(f"Found route visualization (route ID={route_id}).") - return _json_response(rp) +rp = _obj_to_db.route_visualization_from_db_model(rp_db_models[0]) +_log_info(f"Found route visualization (route ID={route_id}).") +return _json_response(rp)fleet_management_api/api_impl/controllers/route.py (1)
101-107: Consider removing unnecessary else after return.Following the static analysis suggestion, you can improve readability by removing the else clause:
if not routes: return _log_info_and_respond( f"Route with ID={route_id} was not found.", 404, title=_OBJ_NOT_FOUND ) -else: - _log_info(f"Found {len(routes)} route with ID={route_id}") - return _json_response(routes[0]) +_log_info(f"Found {len(routes)} route with ID={route_id}") +return _json_response(routes[0])fleet_management_api/api_impl/controllers/car.py (2)
127-134: Drop the redundantelseafter early return.
Once youreturnin theif not db_cars:branch, theelse:wrapper is unnecessary and costs one indent level.- if not db_cars: - return _log_info_and_respond( - f"Car with ID={car_id} was not found.", 404, title=_OBJ_NOT_FOUND - ) - else: - car = _get_car_with_last_state(request.tenants, db_cars[0]) - _log_info(f"Car with ID={car_id} was found.") - return _json_response(car) + if not db_cars: + return _log_info_and_respond( + f"Car with ID={car_id} was not found.", 404, title=_OBJ_NOT_FOUND + ) + + car = _get_car_with_last_state(request.tenants, db_cars[0]) + _log_info(f"Car with ID={car_id} was found.") + return _json_response(car)
143-151: Same pattern – removeelse, de-indent main path.- cars: list[_models.Car] = [] - if not db_cars: - _log_info("Listing all cars: no cars found.") - else: - for db_car in db_cars: - car = _get_car_with_last_state(request.tenants, db_car) - cars.append(car) - _log_info(f"Listing all cars: {len(cars)} cars found.") + cars: list[_models.Car] = [] + if not db_cars: + _log_info("Listing all cars: no cars found.") + return _json_response(cars) + + for db_car in db_cars: + car = _get_car_with_last_state(request.tenants, db_car) + cars.append(car) + _log_info(f"Listing all cars: {len(cars)} cars found.") return _json_response(cars)This improves readability and eliminates one branch.
fleet_management_api/api_impl/controllers/stop.py (2)
85-90: Unwrap redundantelseafter areturn.- if not stops: - return _log_info_and_respond(f"Stop (ID={stop_id}) not found.", 404, title=_OBJ_NOT_FOUND) - else: - _log_info(f"Found {len(stops)} stop with ID={stop_id}") - return _Response(body=stops[0], status_code=200, content_type="application/json") + if not stops: + return _log_info_and_respond(f"Stop (ID={stop_id}) not found.", 404, title=_OBJ_NOT_FOUND) + + _log_info(f"Found {len(stops)} stop with ID={stop_id}") + return _Response(body=stops[0], status_code=200, content_type="application/json")
134-141: Same refactor applies here.- if route_db_models: - return _log_info_and_respond( - f"Stop with ID={stop_id} cannot be deleted because it is referenced by {len(route_db_models)} route(s).", - 400, - title=_CANNOT_DELETE_REFERENCED, - ) - else: - return _log_info_and_respond(f"Stop (ID={stop_id}) not referenced by any route.") + if route_db_models: + return _log_info_and_respond( + f"Stop with ID={stop_id} cannot be deleted because it is referenced by {len(route_db_models)} route(s).", + 400, + title=_CANNOT_DELETE_REFERENCED, + ) + + return _log_info_and_respond(f"Stop (ID={stop_id}) not referenced by any route.")tests/controllers/order/test_order_controller.py (1)
331-367: Excellent concurrency test design with a minor style improvement needed.This test effectively validates the PR's main objective of ensuring orders without valid states are not returned during concurrent deletion operations. The threading approach and iteration count are well-chosen for catching race conditions.
Apply this diff to improve the dict key check:
- if "lastState" not in order.keys(): + if "lastState" not in order:fleet_management_api/api_impl/controllers/order.py (1)
247-254: Consider removing unnecessary else clauses after return statements.The static analysis correctly identifies opportunities to reduce nesting by removing else clauses that follow return statements.
Apply these diffs to improve code structure:
For lines 247-254:
if response.status_code == 200: msg = f"Order (ID={order_id}) has been deleted." _log_info(msg) remove_order(car_id, order_id=order_id) return _text_response(f"Order (ID={order_id})has been succesfully deleted.") -else: - msg = f"Order (ID={order_id}) could not be deleted. {response.body['detail']}" - return _log_warning_or_error_and_respond(msg, response.status_code, response.body["title"]) + +msg = f"Order (ID={order_id}) could not be deleted. {response.body['detail']}" +return _log_warning_or_error_and_respond(msg, response.status_code, response.body["title"])For lines 271-283:
if not order_db_models: msg = f"Order with ID={order_id} assigned to car with ID={car_id} was not found." _log_info(msg) return _error(404, msg, _OBJ_NOT_FOUND) -else: - db_order = order_db_models[0] - order = _get_order_with_last_state(request.tenants, db_order) - if not order: - msg = f"No valid order found for ID={order_id} for car with ID={car_id}." - _log_info(msg) - return _error(404, msg, _OBJ_NOT_FOUND) - _log_info(f"Found order with ID={order_id} of car with ID={car_id}.") - return _json_response(order) # type: ignore + +db_order = order_db_models[0] +order = _get_order_with_last_state(request.tenants, db_order) +if not order: + msg = f"No valid order found for ID={order_id} for car with ID={car_id}." + _log_info(msg) + return _error(404, msg, _OBJ_NOT_FOUND) +_log_info(f"Found order with ID={order_id} of car with ID={car_id}.") +return _json_response(order) # type: ignoreAlso applies to: 271-283
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
fleet_management_api/api_impl/api_keys.py(1 hunks)fleet_management_api/api_impl/controllers/car.py(2 hunks)fleet_management_api/api_impl/controllers/car_action.py(1 hunks)fleet_management_api/api_impl/controllers/order.py(7 hunks)fleet_management_api/api_impl/controllers/order_state.py(2 hunks)fleet_management_api/api_impl/controllers/platform_hw.py(1 hunks)fleet_management_api/api_impl/controllers/route.py(2 hunks)fleet_management_api/api_impl/controllers/route_visualization.py(1 hunks)fleet_management_api/api_impl/controllers/stop.py(2 hunks)fleet_management_api/database/db_access.py(1 hunks)fleet_management_api/database/wait.py(3 hunks)fleet_management_api/openapi/openapi.yaml(1 hunks)openapi/openapi.yaml(1 hunks)pyproject.toml(1 hunks)tests/_utils/setup_utils.py(1 hunks)tests/controllers/car/test_car_action_controller.py(1 hunks)tests/controllers/order/test_get_order_state.py(2 hunks)tests/controllers/order/test_order_controller.py(2 hunks)tests/controllers/order/test_order_state_controller.py(2 hunks)
🧰 Additional context used
🧠 Learnings (16)
openapi/openapi.yaml (7)
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/payload.py:0-0
Timestamp: 2025-05-12T07:30:23.835Z
Learning: Files autogenerated by OpenAPI Generator in the fleet-protocol-http-api project (indicated by comments like "This class is auto generated by OpenAPI Generator") should not be manually modified, even if they contain linting issues or duplicate imports. Any changes should be made at the specification level and regenerated.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/module.py:0-0
Timestamp: 2025-05-12T07:30:17.701Z
Learning: Files in the server/fleetv2_http_api/models/ directory are auto-generated by OpenAPI Generator and should not be manually modified. Any necessary changes should be made to the OpenAPI specification instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: mikusaq
PR: bringauto/packager#24
File: example/package/fleet-protocol-internal-client/internal_client_debug.json:23-23
Timestamp: 2024-10-09T05:03:50.249Z
Learning: The repository URL for 'fleet-protocol-internal-client' is 'https://github.com/bringauto/internal-client-cpp.git', not 'https://github.com/bringauto/fleet-protocol-internal-client.git'.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
pyproject.toml (9)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#18
File: docker-compose.yaml:12-13
Timestamp: 2024-10-04T06:07:27.658Z
Learning: In the `fleet-management-http-api` project, the logging paths are configured in `config.json`, and the mapped log directory is specified in the Dockerfile, so no additional logging configurations are necessary in the Python code.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#18
File: docker-compose.yaml:12-13
Timestamp: 2024-10-09T05:03:50.249Z
Learning: In the `fleet-management-http-api` project, the logging paths are configured in `config.json`, and the mapped log directory is specified in the Dockerfile, so no additional logging configurations are necessary in the Python code.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/module.py:0-0
Timestamp: 2025-05-12T07:30:17.701Z
Learning: Files in the server/fleetv2_http_api/models/ directory are auto-generated by OpenAPI Generator and should not be manually modified. Any necessary changes should be made to the OpenAPI specification instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: tests_integration/request_validation/test_api_key_and_jwt_simultaneously.py:5-5
Timestamp: 2025-05-12T07:28:16.490Z
Learning: In the fleet-protocol-http-api project, all imports must be absolute and start from the root of the package, with "server" as the first part of the imported module name (e.g., "from server.app import get_test_app"), rather than starting with "fleetv2_http_api".
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/payload.py:0-0
Timestamp: 2025-05-12T07:30:23.835Z
Learning: Files autogenerated by OpenAPI Generator in the fleet-protocol-http-api project (indicated by comments like "This class is auto generated by OpenAPI Generator") should not be manually modified, even if they contain linting issues or duplicate imports. Any changes should be made at the specification level and regenerated.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-19
Timestamp: 2024-10-14T06:44:43.636Z
Learning: In this project, modifying `sys.path` is necessary for including protobuf modules from 'lib/fleet-protocol/protobuf/compiled/python' because they are nested deep inside a non-module folder structure.
fleet_management_api/api_impl/controllers/platform_hw.py (1)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#31
File: fleet_management_api/app.py:141-144
Timestamp: 2025-05-07T11:43:57.384Z
Learning: In fleet_management_api/app.py, an empty string API key is intentionally treated as a present key (different from None). The condition `if self._app.api_key is not None:` should not be changed to a truthiness check, as empty API keys have specific handling in the codebase.
fleet_management_api/api_impl/controllers/route_visualization.py (5)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#31
File: fleet_management_api/app.py:141-144
Timestamp: 2025-05-07T11:43:57.384Z
Learning: In fleet_management_api/app.py, an empty string API key is intentionally treated as a present key (different from None). The condition `if self._app.api_key is not None:` should not be changed to a truthiness check, as empty API keys have specific handling in the codebase.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
fleet_management_api/api_impl/api_keys.py (1)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#31
File: fleet_management_api/app.py:141-144
Timestamp: 2025-05-07T11:43:57.384Z
Learning: In fleet_management_api/app.py, an empty string API key is intentionally treated as a present key (different from None). The condition `if self._app.api_key is not None:` should not be changed to a truthiness check, as empty API keys have specific handling in the codebase.
tests/_utils/setup_utils.py (3)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_db_operations.py:298-298
Timestamp: 2025-01-31T13:44:30.026Z
Learning: Tenant isolation tests are distributed across multiple test files in the codebase, including security/test_tenants.py, database/test_using_tenants.py, and controller-specific test files for cars, routes, and stops.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
fleet_management_api/openapi/openapi.yaml (7)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/payload.py:0-0
Timestamp: 2025-05-12T07:30:23.835Z
Learning: Files autogenerated by OpenAPI Generator in the fleet-protocol-http-api project (indicated by comments like "This class is auto generated by OpenAPI Generator") should not be manually modified, even if they contain linting issues or duplicate imports. Any changes should be made at the specification level and regenerated.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/module.py:0-0
Timestamp: 2025-05-12T07:30:17.701Z
Learning: Files in the server/fleetv2_http_api/models/ directory are auto-generated by OpenAPI Generator and should not be manually modified. Any necessary changes should be made to the OpenAPI specification instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/models/tenant.py:57-67
Timestamp: 2025-01-31T12:11:00.988Z
Learning: Code files auto-generated by OpenAPI Generator (marked with "NOTE: This class is auto generated by OpenAPI Generator") should not be modified directly. Any changes needed should be made to the OpenAPI specification instead.
fleet_management_api/api_impl/controllers/route.py (6)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#31
File: fleet_management_api/app.py:141-144
Timestamp: 2025-05-07T11:43:57.384Z
Learning: In fleet_management_api/app.py, an empty string API key is intentionally treated as a present key (different from None). The condition `if self._app.api_key is not None:` should not be changed to a truthiness check, as empty API keys have specific handling in the codebase.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_db_operations.py:157-166
Timestamp: 2025-02-03T11:29:43.795Z
Learning: The `get_by_id` function in `fleet_management_api.database.db_access` is used only internally within other database access methods that already handle tenant context, so it does not need tenant parameter in its signature.
tests/controllers/car/test_car_action_controller.py (1)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
fleet_management_api/api_impl/controllers/stop.py (2)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#31
File: fleet_management_api/app.py:141-144
Timestamp: 2025-05-07T11:43:57.384Z
Learning: In fleet_management_api/app.py, an empty string API key is intentionally treated as a present key (different from None). The condition `if self._app.api_key is not None:` should not be changed to a truthiness check, as empty API keys have specific handling in the codebase.
Learnt from: MarioIvancik
PR: bringauto/virtual-vehicle#11
File: source/bringauto/communication/fleet_protocol/FleetProtocol.cpp:177-177
Timestamp: 2024-10-17T08:35:13.267Z
Learning: In the C++ file `source/bringauto/communication/fleet_protocol/FleetProtocol.cpp`, the method `status.getNextStop()` always returns an object with default-initialized members, so accessing its members without null checks is safe.
fleet_management_api/api_impl/controllers/car.py (7)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#31
File: fleet_management_api/app.py:141-144
Timestamp: 2025-05-07T11:43:57.384Z
Learning: In fleet_management_api/app.py, an empty string API key is intentionally treated as a present key (different from None). The condition `if self._app.api_key is not None:` should not be changed to a truthiness check, as empty API keys have specific handling in the codebase.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#30
File: fleet_management_api/controllers/tenant_controller.py:11-23
Timestamp: 2025-04-22T12:06:06.987Z
Learning: Files under `fleet_management_api/controllers/` contain auto-generated code with placeholder implementations (like "do some magic!" return statements) and should not be flagged for issues like unused variables or placeholder return values.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
fleet_management_api/database/db_access.py (9)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/order/test_get_order_state.py:411-411
Timestamp: 2025-01-31T13:37:52.479Z
Learning: The `delete` function in `fleet_management_api.database.db_access` is used both within and outside of test suites, so it should not be wrapped in test-specific helper functions that require test app instances.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_db_operations.py:157-166
Timestamp: 2025-02-03T11:29:43.795Z
Learning: The `get_by_id` function in `fleet_management_api.database.db_access` is used only internally within other database access methods that already handle tenant context, so it does not need tenant parameter in its signature.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/api_impl/controllers/tenant.py:0-0
Timestamp: 2025-01-31T11:57:15.656Z
Learning: Transaction management in fleet_management_api is handled by the db_access_method decorator in the _db_access package, which wraps database operations like exists() in transactions.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/api_impl/controllers/tenant.py:0-0
Timestamp: 2025-01-31T11:57:15.656Z
Learning: In fleet_management_api's database operations, transaction management is implemented through multiple layers:
1. The @db_access_method decorator provides top-level transaction handling
2. Session context managers within methods like exists() provide additional transaction safety
3. This pattern is used consistently across the _db_access package
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: docs/entity_manipulations.md:326-338
Timestamp: 2025-01-30T11:13:29.777Z
Learning: The DELETE /tenant endpoint in fleet-management-http-api returns specific error codes: 400 for tenants with associated items, 404 if tenant doesn't exist, and 405 for incorrect HTTP method.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: docs/entity_manipulations.md:326-338
Timestamp: 2025-01-30T11:13:29.777Z
Learning: The tenant entity in fleet-management-http-api only supports GET and DELETE operations. Tenants are created automatically when first used for data operations, and there is no explicit POST or PUT endpoint.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_database_cleanup.py:53-54
Timestamp: 2025-01-31T13:43:48.677Z
Learning: In fleet-management-http-api tests, the database is completely recreated for each test using docker compose, making explicit cleanup of individual records unnecessary.
tests/controllers/order/test_order_controller.py (2)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
tests/controllers/order/test_get_order_state.py (1)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
fleet_management_api/api_impl/controllers/order.py (5)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/order/test_get_order_state.py:411-411
Timestamp: 2025-01-31T13:37:52.479Z
Learning: The `delete` function in `fleet_management_api.database.db_access` is used both within and outside of test suites, so it should not be wrapped in test-specific helper functions that require test app instances.
tests/controllers/order/test_order_state_controller.py (1)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
🧬 Code Graph Analysis (5)
fleet_management_api/api_impl/controllers/car_action.py (1)
fleet_management_api/models/car_action_status.py (1)
CarActionStatus(9-39)
tests/_utils/setup_utils.py (2)
fleet_management_api/api_impl/tenants.py (2)
all(102-104)all(145-146)fleet_management_api/database/db_models.py (1)
all(60-60)
fleet_management_api/api_impl/controllers/route.py (1)
fleet_management_api/database/db_access.py (1)
get(384-429)
fleet_management_api/api_impl/controllers/car.py (1)
fleet_management_api/models/car.py (1)
Car(13-227)
fleet_management_api/api_impl/controllers/order.py (1)
fleet_management_api/models/order.py (7)
car_id(163-170)car_id(173-183)Order(15-313)last_state(274-281)last_state(284-292)id(94-101)id(104-112)
🪛 Pylint (3.3.7)
fleet_management_api/api_impl/controllers/platform_hw.py
[refactor] 66-74: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
fleet_management_api/api_impl/controllers/route_visualization.py
[refactor] 31-40: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
fleet_management_api/api_impl/api_keys.py
[refactor] 64-67: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
fleet_management_api/api_impl/controllers/route.py
[refactor] 101-107: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
fleet_management_api/api_impl/controllers/stop.py
[refactor] 85-89: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 134-141: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
fleet_management_api/api_impl/controllers/car.py
[refactor] 127-134: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
fleet_management_api/api_impl/controllers/order.py
[refactor] 247-254: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 271-283: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🪛 Ruff (0.11.9)
tests/controllers/order/test_order_controller.py
346-346: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
🔇 Additional comments (24)
fleet_management_api/api_impl/controllers/platform_hw.py (1)
66-66: LGTM! Pythonic improvement applied.The change from
len(hws) == 0tonot hwsis a good Pythonic improvement that maintains the same logic while being more readable and idiomatic.fleet_management_api/api_impl/api_keys.py (1)
64-64: LGTM! Pythonic improvement applied.The change from
len(_key_db_models) == 0tonot _key_db_modelsis a good Pythonic improvement that maintains the same logic while being more concise and readable.fleet_management_api/api_impl/controllers/route_visualization.py (1)
31-31: LGTM! Pythonic improvement applied.The change from
len(rp_db_models) == 0tonot rp_db_modelsis a good Pythonic improvement that maintains the same logic while being more readable and concise.tests/_utils/setup_utils.py (1)
18-18: LGTM! Pythonic improvement applied.The change from
len(self.all) == 0tonot self.allis a good Pythonic improvement. The logic correctly handles unrestricted access (when no tenants are specified) and restricted access (when specific tenants are allowed).fleet_management_api/api_impl/controllers/route.py (2)
101-101: LGTM! Pythonic improvement applied.The change from
len(routes) == 0tonot routesis a good Pythonic improvement that maintains the same logic while being more readable and concise.
186-186: LGTM! Excellent set comprehension improvement.The change from
set([stop_id.id for stop_id in _db_access.get(tenants, _StopDB)])to{stop_id.id for stop_id in _db_access.get(tenants, _StopDB)}is a great improvement. Using set comprehension syntax{...}is more efficient and idiomatic than constructing a list first and then converting it to a set.fleet_management_api/database/wait.py (3)
22-22: Literal initialization is fine – good stylistic tweak.
{}is faster and more idiomatic thandict(). No further issues spotted.
69-70: Consistent use of[]literal – looks good.
Keeps the codebase uniform and marginally faster.
107-108: 👍 Consistent empty-list initialization.
No behavioural change introduced.openapi/openapi.yaml (1)
5-5: Version bump acknowledged.
4.1.3matches the library version; no spec inconsistencies detected.pyproject.toml (1)
3-3: Project version aligned with OpenAPI spec – looks good.
No further action necessary.fleet_management_api/openapi/openapi.yaml (1)
12-12: Version bump to 4.1.3 looks goodNo other spec changes detected; the update is consistent with the PR objective.
fleet_management_api/api_impl/controllers/car_action.py (1)
172-173: LGTM! Pythonic dictionary initialization.The change from
dict()to{}literal syntax follows Python best practices and improves readability while maintaining identical functionality.fleet_management_api/api_impl/controllers/order_state.py (2)
215-215: LGTM! Pythonic dictionary initialization.The change from
dict()to{}literal syntax follows Python best practices and improves readability.
270-270: LGTM! Consistent with codebase style improvements.The change from
dict()to{}literal syntax aligns with the stylistic improvements being made across the codebase.tests/controllers/car/test_car_action_controller.py (1)
119-119: LGTM! Test assertion updated consistently with code changes.The change from
dict()to{}in the assertion aligns with the stylistic improvements made in the production code while maintaining identical test behavior.fleet_management_api/database/db_access.py (1)
266-270: LGTM! Valuable documentation clarification.The updated docstring clearly explains that the deletion of the entity and its dependent entities occurs within a single transaction, which is important for understanding the deletion behavior mentioned in the PR objectives.
tests/controllers/order/test_get_order_state.py (2)
402-402: LGTM! Test method name reflects the updated behavior.The renamed test method better describes the current behavior where orders are properly handled when they lack states, aligning with the PR objectives.
416-416: LGTM! Behavioral change aligns with PR objectives.The change from expecting a 200 status code to 404 correctly reflects the new behavior where orders without valid states return a 404 error. This addresses the core issue mentioned in the PR objectives about orders briefly existing without associated states during deletion.
tests/controllers/order/test_order_controller.py (1)
4-4: LGTM!Clean import addition for the new concurrency test functionality.
fleet_management_api/api_impl/controllers/order.py (2)
66-66: Great Pythonic style improvements!The changes from
list()to[]literals and using truthiness checks instead of explicit length comparisons improve code readability and follow Python best practices.Also applies to: 83-83, 271-271, 299-299, 315-315
254-254: Good refactoring to consolidate error handling.The change to use
_log_warning_or_error_and_respondreduces code duplication and follows the DRY principle while maintaining the same functionality.tests/controllers/order/test_order_state_controller.py (2)
396-396: Good grammatical correction in test method name.The change from "yield_403_code" to "yields_403_code" improves the grammatical correctness of the test method name.
439-472: Excellent test coverage for final state edge case behavior.This test effectively validates that attempting to add states after final states doesn't trigger autodeletion of existing states. The test design with limited max states (3) and clear phase documentation makes it easy to understand and maintain.
MarioIvancik
reviewed
Jul 2, 2025
MarioIvancik
approved these changes
Jul 2, 2025
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fleet_management_api/api_impl/controllers/order.py (1)
247-254: Consider refactoring unnecessary else blocks.Static analysis suggests removing unnecessary
elseblocks afterreturnstatements for cleaner code structure.For the delete_order function:
if response.status_code == 200: msg = f"Order (ID={order_id}) has been deleted." _log_info(msg) remove_order(car_id, order_id=order_id) return _text_response(f"Order (ID={order_id})has been succesfully deleted.") -else: - msg = f"Order (ID={order_id}) could not be deleted. {response.body['detail']}" - return _log_warning_or_error_and_respond(msg, response.status_code, response.body["title"]) + +msg = f"Order (ID={order_id}) could not be deleted. {response.body['detail']}" +return _log_warning_or_error_and_respond(msg, response.status_code, response.body["title"])Similar refactoring applies to the get_order function.
Also applies to: 271-283
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fleet_management_api/api_impl/controllers/order.py(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
fleet_management_api/api_impl/controllers/order.py (7)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#30
File: fleet_management_api/controllers/tenant_controller.py:11-23
Timestamp: 2025-04-22T12:06:06.987Z
Learning: Files under `fleet_management_api/controllers/` contain auto-generated code with placeholder implementations (like "do some magic!" return statements) and should not be flagged for issues like unused variables or placeholder return values.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/module.py:0-0
Timestamp: 2025-05-12T07:30:17.701Z
Learning: Files in the server/fleetv2_http_api/models/ directory are auto-generated by OpenAPI Generator and should not be manually modified. Any necessary changes should be made to the OpenAPI specification instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
🧬 Code Graph Analysis (1)
fleet_management_api/api_impl/controllers/order.py (1)
fleet_management_api/models/order.py (7)
car_id(163-170)car_id(173-183)Order(15-313)last_state(274-281)last_state(284-292)id(94-101)id(104-112)
🪛 Pylint (3.3.7)
fleet_management_api/api_impl/controllers/order.py
[refactor] 247-254: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 271-283: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🔇 Additional comments (6)
fleet_management_api/api_impl/controllers/order.py (6)
66-66: LGTM: Improved list initialization syntax.Using
[]instead oflist()is more Pythonic and slightly more efficient for creating empty lists.Also applies to: 83-83
254-254: LGTM: Consolidated error handling.The refactoring to use
_log_warning_or_error_and_respondsimplifies the code by combining logging and response generation into a single call.
271-271: LGTM: More Pythonic truthiness check.Using
not order_db_modelsinstead oflen(order_db_models) == 0is cleaner and more idiomatic Python.
278-281: LGTM: Core feature implementation is correct.The validation for orders without valid last states correctly implements the PR requirements. Returning 404 responses for orders without states ensures they are properly ignored during the deletion process, which aligns with the stated objective of handling orders that briefly exist without associated states.
299-299: LGTM: Consistent empty list initialization.Using
[]instead oflist()maintains consistency with the other changes in the file and follows Python best practices.Also applies to: 315-315
332-334: LGTM: Robust state validation.The helper function properly handles missing states by logging and returning
None, which prevents downstream errors when constructing Order objects. This is the core implementation that enables ignoring orders without valid states.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue was that the orders briefly existed without any orders states during their deletion.
The SQLAlchemy first deletes the child items, then is deletes the parent (the Order in this case).
Attempts to fix this on the side of the DB models definitions were not succesfull (there is an issue with the sqlite dialect used when testing, that ignores settings for deleting the children first).
The problem is fixed by ignoring orders without states. When the server is running and the orders are manipulated via the API, there are only two situation when the order in DB does not have any states - during its creation and its deletion. There is no possibility, that for example, the order would be ignored in the middle of its existence and the become available again. This solution is thus considered safe enough to be used.
Summary by CodeRabbit
Bug Fixes
Style
Documentation
Tests
Chores