Issue in Yearly Report Download#116
Conversation
📝 WalkthroughWalkthroughSingle file refactoring in the CRM report service that replaces direct type casts with null-safe parsing methods, adjusts field extraction indices to align with revised column positions, and converts primitive operations to use valueOf/toString for improved robustness. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java (1)
517-518:⚠️ Potential issue | 🔴 CriticalCritical: Yearly report uses different column indices than daily/monthly despite all three calling the same stored procedure.
getDailyStockDetailsReport(line 204),getMonthlyReport(line 402), andgetYearlyReport(line 517) all callitemStockReportRepo.getDailyStockDetailReportByFacilityID(...), which executes the same stored procedure. However, they parse results using different column indices:
- Daily & Monthly:
objects[11]→adjustedQuantity_FromDate,objects[12]→quantityDispanced,objects[13]→itemName, ...,objects[19]→ClosingStock- Yearly:
objects[11]→quantityDispanced,objects[12]→itemName, ...,objects[15]→adjustedQuantity_FromDate, ...,objects[18]→ClosingStockAdditionally, both
adjustedQuantity_FromDate(line 538) andadjustedQuantity_ToDate(line 549) read fromobjects[15], causing them to always have the same value.Since the same stored procedure returns the same columns regardless of date range, either all three methods are using correct indices or all are wrong. Verify the stored procedure schema and ensure consistent index mappings across all three methods.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java` around lines 517 - 518, The three methods getDailyStockDetailsReport, getMonthlyReport, and getYearlyReport all call itemStockReportRepo.getDailyStockDetailReportByFacilityID but parse the returned Object[] with inconsistent indices (e.g., adjustedQuantity_FromDate, adjustedQuantity_ToDate, quantityDispanced, itemName, ClosingStock) and Yearly currently reads adjustedQuantity_FromDate and adjustedQuantity_ToDate from the same objects[15]; verify the stored procedure's result column order, create a single canonical index-to-field mapping, and update the parsing in getDailyStockDetailsReport, getMonthlyReport, and getYearlyReport to use the same indices (fix the duplicate objects[15] read so adjustedQuantity_FromDate and adjustedQuantity_ToDate use their correct, distinct indices).
🧹 Nitpick comments (2)
src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java (2)
564-566: Dead variables:adjustedQuantity_FromDateandadjustedQuantity_ToDateare assigned but never used.Both
adjustedQuantity_FromDate(line 537) andadjustedQuantity_ToDate(line 548) are computed but never referenced in the output construction (lines 568–583). The commented-out logic that previously used them is disabled. Consider either removing these dead variables or, if they are expected to be used in future, adding a TODO comment explaining intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java` around lines 564 - 566, Dead local variables adjustedQuantity_FromDate and adjustedQuantity_ToDate in CRMReportServiceImpl are assigned but never used; remove their assignments (or the variables entirely) from the method to eliminate dead code, or if they are intended for future use, add a clear TODO comment above the assignments referencing adjustedQuantity_FromDate and adjustedQuantity_ToDate and explain how they should be incorporated into the output construction (where actualOpening, actualDispensed, and actualClosing are set) so the intent is preserved; locate the assignments in CRMReportServiceImpl and either delete them or replace them with a TODO stub and keep the rest of the output construction unchanged.
523-563: Prefer((Number) obj).longValue()overLong.valueOf(obj.toString())for numeric database results.The daily and monthly methods use
((Number) objects[x]).longValue(), which is safe for any numeric SQL type (Integer, Long, BigDecimal, BigInteger, etc.). The yearly method now usesLong.valueOf(objects[x].toString()), which introduces aNumberFormatExceptionrisk if the database returns a numeric type whosetoString()produces a non-parseable representation (e.g., decimal values like"123.0"from aDouble/BigDecimalwill failLong.valueOf).If the PR's goal is null-safety, the existing
if (objects[x] != null)guard already handles that. The((Number) obj).longValue()pattern is both safer and consistent with the rest of the file.Example fix for one field (apply same pattern to all)
if (objects[4] != null) { - totalQuantityReceived = Long.valueOf(objects[4].toString()); + totalQuantityReceived = ((Number) objects[4]).longValue(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java` around lines 523 - 563, Replace numeric parsing that uses Long.valueOf(obj.toString()) and Double.valueOf(obj.toString()) with safe numeric casts using the Number interface while keeping the null checks; e.g., where totalQuantityReceived, openingStock, adjustedQuantity_FromDate, quantityDispanced, adjustedQuantity_ToDate, adjustedQuantity_ToDate_Receipt, adjustedQuantity_ToDate_Issue, ClosingStock are set using Long.valueOf(objects[i].toString()), change those assignments to ((Number) objects[i]).longValue(), and change unitCostPrice (currently Double.valueOf(objects[5].toString())) to ((Number) objects[5]).doubleValue(); keep the existing if (objects[i] != null) guards to avoid NPEs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java`:
- Around line 538-550: In CRMReportServiceImpl update the incorrect array index
usages: change the assignment of adjustedQuantity_FromDate to read from
objects[11] instead of objects[15], and change adjustedQuantity_ToDate to read
from objects[16] instead of objects[15]; locate the assignments to the variables
adjustedQuantity_FromDate and adjustedQuantity_ToDate in the block that also
sets quantityDispanced, itemName, facilityName and itemCategoryName and adjust
the indices accordingly, then run the related report/unit tests to verify
mappings against the PR_StockDetail stored procedure.
---
Outside diff comments:
In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java`:
- Around line 517-518: The three methods getDailyStockDetailsReport,
getMonthlyReport, and getYearlyReport all call
itemStockReportRepo.getDailyStockDetailReportByFacilityID but parse the returned
Object[] with inconsistent indices (e.g., adjustedQuantity_FromDate,
adjustedQuantity_ToDate, quantityDispanced, itemName, ClosingStock) and Yearly
currently reads adjustedQuantity_FromDate and adjustedQuantity_ToDate from the
same objects[15]; verify the stored procedure's result column order, create a
single canonical index-to-field mapping, and update the parsing in
getDailyStockDetailsReport, getMonthlyReport, and getYearlyReport to use the
same indices (fix the duplicate objects[15] read so adjustedQuantity_FromDate
and adjustedQuantity_ToDate use their correct, distinct indices).
---
Nitpick comments:
In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java`:
- Around line 564-566: Dead local variables adjustedQuantity_FromDate and
adjustedQuantity_ToDate in CRMReportServiceImpl are assigned but never used;
remove their assignments (or the variables entirely) from the method to
eliminate dead code, or if they are intended for future use, add a clear TODO
comment above the assignments referencing adjustedQuantity_FromDate and
adjustedQuantity_ToDate and explain how they should be incorporated into the
output construction (where actualOpening, actualDispensed, and actualClosing are
set) so the intent is preserved; locate the assignments in CRMReportServiceImpl
and either delete them or replace them with a TODO stub and keep the rest of the
output construction unchanged.
- Around line 523-563: Replace numeric parsing that uses
Long.valueOf(obj.toString()) and Double.valueOf(obj.toString()) with safe
numeric casts using the Number interface while keeping the null checks; e.g.,
where totalQuantityReceived, openingStock, adjustedQuantity_FromDate,
quantityDispanced, adjustedQuantity_ToDate, adjustedQuantity_ToDate_Receipt,
adjustedQuantity_ToDate_Issue, ClosingStock are set using
Long.valueOf(objects[i].toString()), change those assignments to ((Number)
objects[i]).longValue(), and change unitCostPrice (currently
Double.valueOf(objects[5].toString())) to ((Number) objects[5]).doubleValue();
keep the existing if (objects[i] != null) guards to avoid NPEs.
| if (objects[15] != null) { | ||
| adjustedQuantity_FromDate = Long.valueOf(objects[15].toString()); | ||
| } | ||
| Long quantityDispanced = 0L; | ||
| if (objects[12] != null) { | ||
| quantityDispanced = ((Number) objects[12]).longValue(); | ||
| if (objects[11] != null) { | ||
| quantityDispanced = Long.valueOf(objects[11].toString()); | ||
| } | ||
| String itemName = (String) objects[13]; | ||
| String facilityName = (String) objects[14]; | ||
| String itemCategoryName = (String) objects[15]; | ||
| String itemName = objects[12] != null ? objects[12].toString() : null; | ||
| String facilityName = objects[13] != null ? objects[13].toString() : null; | ||
| String itemCategoryName = objects[14] != null ? objects[14].toString() : null; | ||
| Long adjustedQuantity_ToDate = 0L; | ||
| if (objects[16] != null) { | ||
| adjustedQuantity_ToDate = ((Number) objects[16]).longValue(); | ||
| if (objects[15] != null) { | ||
| adjustedQuantity_ToDate = Long.valueOf(objects[15].toString()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file around lines 538-550
head -560 src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java | tail -30Repository: PSMRI/Inventory-API
Length of output: 1242
🏁 Script executed:
# Also get more context around this section to understand the field mappings
sed -n '520,560p' src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.javaRepository: PSMRI/Inventory-API
Length of output: 1661
🏁 Script executed:
# Find the SQL query or stored procedure that generates these objects
# Look for the method that calls this code
rg -B 30 "adjustedQuantity_FromDate" src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java | head -80Repository: PSMRI/Inventory-API
Length of output: 2580
🏁 Script executed:
# Search for stored procedures or SQL queries related to daily stock
fd -e sql -o -e SQL | xargs grep -l "dailystock\|DailyStock" 2>/dev/null | head -5Repository: PSMRI/Inventory-API
Length of output: 179
🏁 Script executed:
# Find the method name and SQL query for lines 538-550
sed -n '490,560p' src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java | grep -B 50 "adjustedQuantity_FromDate"Repository: PSMRI/Inventory-API
Length of output: 1590
🏁 Script executed:
# Search for SQL queries related to daily stock summary
fd -e sql -o -e SQL | xargs grep -l "DailyStock\|dailystock" 2>/dev/nullRepository: PSMRI/Inventory-API
Length of output: 181
🏁 Script executed:
# Find nativeQuery or `@Query` annotations in the repository
rg -A 15 "@Query|nativeQuery" src/main/java/com/iemr/inventory --type java -g '*Repo*' | grep -A 15 "dailystock\|summary" -iRepository: PSMRI/Inventory-API
Length of output: 3986
🏁 Script executed:
# Find the PR_StockDetail stored procedure definition
fd -e sql -o -e SQL | xargs grep -l "PR_StockDetail" 2>/dev/nullRepository: PSMRI/Inventory-API
Length of output: 181
🏁 Script executed:
# Search in database folder for the procedure
find . -type f \( -name "*.sql" -o -name "*.SQL" \) | xargs grep -l "StockDetail" 2>/dev/nullRepository: PSMRI/Inventory-API
Length of output: 47
🏁 Script executed:
# Look for stored procedure files in common locations
fd -path "*/db/*" -o -path "*/database/*" -o -path "*/schema/*" | grep -i sqlRepository: PSMRI/Inventory-API
Length of output: 231
🏁 Script executed:
# Search more broadly for SQL files
find . -name "*.sql" 2>/dev/null | head -20Repository: PSMRI/Inventory-API
Length of output: 45
🏁 Script executed:
# Look for stored procedure definitions in resources or database files
rg "CREATE.*PROCEDURE.*PR_StockDetail\|CREATE.*PROCEDURE.*PR_StockSummary" -iRepository: PSMRI/Inventory-API
Length of output: 45
🏁 Script executed:
# Check if there are any SQL files at all in the repository
git ls-files | grep -i "\\.sql$"Repository: PSMRI/Inventory-API
Length of output: 45
🏁 Script executed:
# Let me compare the two methods that use the same stored procedure to identify the correct indices
echo "=== getYearlyReport method indices ==="
sed -n '530,570p' src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java
echo ""
echo "=== getDailyStockDetailsReport method indices (corrected version) ==="
sed -n '450,500p' src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.javaRepository: PSMRI/Inventory-API
Length of output: 3578
🏁 Script executed:
# Check what repository method is called by getYearlyReport
sed -n '505,535p' src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.javaRepository: PSMRI/Inventory-API
Length of output: 1048
Critical bug: adjustedQuantity_FromDate and adjustedQuantity_ToDate both incorrectly read from objects[15].
Both fields are assigned the same array index. The correct indices should be objects[11] for adjustedQuantity_FromDate and objects[16] for adjustedQuantity_ToDate, as confirmed by the correctly-implemented getDailyStockDetailsReport() method which uses the same PR_StockDetail stored procedure.
Suggested fix
Long adjustedQuantity_FromDate = 0L;
- if (objects[15] != null) {
- adjustedQuantity_FromDate = Long.valueOf(objects[15].toString());
+ if (objects[11] != null) {
+ adjustedQuantity_FromDate = Long.valueOf(objects[11].toString());
}
Long quantityDispanced = 0L;
if (objects[11] != null) {
quantityDispanced = Long.valueOf(objects[11].toString());
}
String itemName = objects[12] != null ? objects[12].toString() : null;
String facilityName = objects[13] != null ? objects[13].toString() : null;
String itemCategoryName = objects[14] != null ? objects[14].toString() : null;
Long adjustedQuantity_ToDate = 0L;
- if (objects[15] != null) {
- adjustedQuantity_ToDate = Long.valueOf(objects[15].toString());
+ if (objects[16] != null) {
+ adjustedQuantity_ToDate = Long.valueOf(objects[16].toString());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/inventory/service/report/CRMReportServiceImpl.java`
around lines 538 - 550, In CRMReportServiceImpl update the incorrect array index
usages: change the assignment of adjustedQuantity_FromDate to read from
objects[11] instead of objects[15], and change adjustedQuantity_ToDate to read
from objects[16] instead of objects[15]; locate the assignments to the variables
adjustedQuantity_FromDate and adjustedQuantity_ToDate in the block that also
sets quantityDispanced, itemName, facilityName and itemCategoryName and adjust
the indices accordingly, then run the related report/unit tests to verify
mappings against the PR_StockDetail stored procedure.
|
2 participants
📋 Description
JIRA ID:
AMM-1766
AMM-2017
Some column mismatch and datatype mismatch is there between the stored procedure and the backend logic
✅ Type of Change
Summary by CodeRabbit