fix: Pr fix request for update and create order - #77
Open
Olatomiw wants to merge 1 commit into
Open
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
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.
Description
This PR addresses critical issues in the order management service identified in #69, improving validation, stock management, and financial accuracy in order creation and update operations.
What I Did
Order Creation (
createOrder)InsufficientStockExceptionfor better error handling in controller layerBigDecimalscale and rounding (setScale(2, RoundingMode.HALF_UP)) for financial calculationsunitPriceandtotalPricefor historical price trackingOrder Update (
update)unitPriceunitPrice(no price updates)BigDecimalscale and rounding for financial accuracyException Handling
InsufficientStockExceptionwith detailed stock availability informationInvalidOrderRequestExceptionfor validation errorsOrderNotEditableExceptionfor status-based restrictionsWhy I Did It
1. Input Validation
Without null/empty checks, the service would throw generic
NullPointerExceptionerrors that are difficult to debug and provide poor user experience. Explicit validation provides clear, actionable error messages.2. Specific Exceptions
Generic
RuntimeExceptionmakes it difficult for the controller layer to differentiate between error types and return appropriate HTTP status codes. Specific exceptions enable:InsufficientProductStockException→ HTTP 409 CONFLICTInvalidRequestException→ HTTP 400 BAD REQUESTProductNotFoundException→ HTTP 404 NOT FOUND3. Stock Management
The original implementation validated stock but never reduced it, leading to overselling scenarios. The new implementation:
4. Financial Accuracy
Using
BigDecimalwithout scale/rounding can lead to precision issues in financial calculations. Setting scale to 2 decimal places withHALF_UProunding ensures:5. Price Preservation
In e-commerce systems, historical price integrity is critical for:
The new implementation stores the price at the time of order creation and preserves it during updates, preventing discrepancies.
6. Duplicate Prevention
Without duplicate detection, submitting the same product multiple times in an update request could create multiple order items for the same product, leading to data inconsistency and incorrect total calculations.
7. Order Status Protection
Allowing updates to shipped or completed orders would create serious business logic violations. Restricting updates to PENDING status ensures data integrity and matches real-world e-commerce workflows.
Testing Recommendations
Related Issues
Fixes #69
Breaking Changes
None. This is a backward-compatible bug fix and enhancement.