perf(pharmacy): optimize transfer request item add — DTOs, single rate query, AJAX#21089
perf(pharmacy): optimize transfer request item add — DTOs, single rate query, AJAX#21089buddhika75 wants to merge 1 commit into
Conversation
…e query, AJAX add Replace full Item entity loads in pharmacy_transfer_request.xhtml with lightweight DTOs and a single-query rate lookup, eliminating the main sources of delay when adding items to a transfer request. Changes: - ItemDTO (search): add dblValue, itemTypeName, rateItemId fields + new constructor for autocomplete DTO queries - ItemRatesDTO (new): holds purchase/retail/cost rates from one query - ItemDtoConverter (new): CDI converter encodes DTO as id:type:dblValue:rateItemId - ItemController: add completeAmpAmppVmpVmppItemDtosForRequestingDepartment returning List<ItemDTO> via 4 typed constructor queries (replaces full entity load per keystroke) - PharmacyBean: add getLastRatesForItem replacing 3 separate entity-loading rate calls with a single BillItemFinanceDetails scalar query - TransferRequestController: autocomplete now uses ItemDTO + typed proxy; populateRatesOnItemSelect reduced from 3 entity loads to 1 entity load + 1 scalar query; addItem reuses cached rates; fetchBillItems adds JOIN FETCH bi.item and LEFT JOIN FETCH bi.billItemFinanceDetails to eliminate N+1 on edit path; recentToDepartments query fetches institution eagerly; three display type list getters are now lazily cached - XHTML: autocomplete bound to currentItemDto with itemDtoConverter; Add Item button changed from ajax=false (full page reload) to AJAX with targeted updates; pack-size column uses class.simpleName instead of brittle full class string comparison (also fixes Vmpp display) Closes #21062 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughThis PR refactors transfer request item selection to use Data Transfer Objects (DTOs) for autocomplete and rate lookups. It extends ItemDTO with pricing metadata, introduces ItemRatesDTO for rates, adds a JSF converter, provides backend DTO-based queries, refactors TransferRequestController to integrate these DTOs with rate population, and updates the UI to use DTO serialization with optimized AJAX form submission. ChangesTransfer Request DTO-driven Item Selection and Rates
Sequence Diagram(s)Included in layer diagram above. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.3)src/main/java/com/divudi/bean/common/ItemController.javaThanks 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47c7acbb60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| action="#{transferRequestController.addItem}" | ||
| ajax="false"/> | ||
| process="@form" | ||
| update="itemAutoComplete txtQty txtLineGrossRate txtLineNetValue itemList requestTotals focusItem"/> |
There was a problem hiding this comment.
When the rate/value columns are disabled (the config defaults to false, or the user lacks StockRequestViewRates), txtLineGrossRate and txtLineNetValue are inside rendered panel groups and are not present in the JSF component tree. This new AJAX add action always updates those IDs, so clicking Add Item in that common mode will fail component resolution instead of adding the item; update a stable wrapper or only target these inputs when they are rendered.
Useful? React with 👍 / 👎.
![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 (2)
src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml (2)
176-178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
txtLineCostRatefrom the AJAX update targets (src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml:176-178)
txtLineCostRateis referenced in thep:ajaxupdatelist, but no component withid="txtLineCostRate"exists in this view, so the AJAX update target can’t be resolved.Suggested fix
<p:ajax event="itemSelect" listener="#{transferRequestController.populateRatesOnItemSelect}" - update="txtLineCostRate txtLineGrossRate txtLineNetValue requestTotals focusQty selDepartmentType"/> + update="txtLineGrossRate txtLineNetValue requestTotals focusQty selDepartmentType"/>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml` around lines 176 - 178, Remove the non-existent AJAX update target by editing the p:ajax in pharmacy_transfer_request.xhtml: locate the p:ajax with listener "#{transferRequestController.populateRatesOnItemSelect}" and remove "txtLineCostRate" from its update list so the remaining update targets (e.g., txtLineGrossRate, txtLineNetValue, requestTotals, focusQty, selDepartmentType) are valid.
211-217:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNarrow the AJAX
processscope for “Add Item”. (src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml:211-217)
process="@Form"causes the whole<h:form>(including the editablep:dataTable id="itemList"rows) to be decoded/validated on every add, so latency will scale with row count.<p:commandButton value="Add Item" icon="fas fa-plus" class="ui-button-success w-100" action="#{transferRequestController.addItem}" process="`@form`" update="itemAutoComplete txtQty txtLineGrossRate txtLineNetValue itemList requestTotals focusItem"/>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml` around lines 211 - 217, The Add Item button currently uses process="`@form`" which decodes/validates the entire h:form (including the editable p:dataTable id="itemList"); change the p:commandButton's process attribute to only submit the specific input components needed for addItem (for example use "`@this` itemAutoComplete txtQty txtLineGrossRate txtLineNetValue" or similar) and remove "`@form`" so the controller method addItem and validations only run for those inputs while leaving update="itemList requestTotals focusItem" unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml`:
- Around line 171-174: The Pack Size column is showing vt.dblValue for any
positive number; update the EL in the h:outputText so dblValue is used only when
the item type is Ampp or Vmpp. Replace the current condition "(vt.dblValue ne
null and vt.dblValue gt 0) ? vt.dblValue : 1" with a compound check that also
ensures the DTO's item-type field (e.g., vt.itemType or the actual property
holding the item type) is 'Ampp' or 'Vmpp' before using vt.dblValue, otherwise
default to 1; keep the <f:convertNumber> formatting unchanged.
---
Outside diff comments:
In `@src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml`:
- Around line 176-178: Remove the non-existent AJAX update target by editing the
p:ajax in pharmacy_transfer_request.xhtml: locate the p:ajax with listener
"#{transferRequestController.populateRatesOnItemSelect}" and remove
"txtLineCostRate" from its update list so the remaining update targets (e.g.,
txtLineGrossRate, txtLineNetValue, requestTotals, focusQty, selDepartmentType)
are valid.
- Around line 211-217: The Add Item button currently uses process="`@form`" which
decodes/validates the entire h:form (including the editable p:dataTable
id="itemList"); change the p:commandButton's process attribute to only submit
the specific input components needed for addItem (for example use "`@this`
itemAutoComplete txtQty txtLineGrossRate txtLineNetValue" or similar) and remove
"`@form`" so the controller method addItem and validations only run for those
inputs while leaving update="itemList requestTotals focusItem" unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b552135-72c7-4aca-8a5c-90e31c302b8d
📒 Files selected for processing (7)
src/main/java/com/divudi/bean/common/ItemController.javasrc/main/java/com/divudi/bean/pharmacy/TransferRequestController.javasrc/main/java/com/divudi/core/converter/ItemDtoConverter.javasrc/main/java/com/divudi/core/data/dto/ItemRatesDTO.javasrc/main/java/com/divudi/core/data/dto/search/ItemDTO.javasrc/main/java/com/divudi/ejb/PharmacyBean.javasrc/main/webapp/pharmacy/pharmacy_transfer_request.xhtml
| <p:column headerText="Pack Size" style="padding: 6px;"> | ||
| <h:outputText value="#{(vt.dblValue ne null and vt.dblValue gt 0) ? vt.dblValue : 1}"> | ||
| <f:convertNumber pattern="0.#" /> | ||
| </h:outputText> |
There was a problem hiding this comment.
Gate the autocomplete pack-size column by item type.
This now shows vt.dblValue for any DTO with a positive numeric value, so non-pack rows can render a misleading “Pack Size”. Match the bill-items table behavior and only use dblValue for Ampp/Vmpp, otherwise default to 1.
Suggested fix
- <p:column headerText="Pack Size" style="padding: 6px;">
- <h:outputText value="#{(vt.dblValue ne null and vt.dblValue gt 0) ? vt.dblValue : 1}">
+ <p:column headerText="Pack Size" style="padding: 6px;">
+ <h:outputText value="#{((vt.itemTypeName eq 'Ampp' or vt.itemTypeName eq 'Vmpp') and vt.dblValue ne null and vt.dblValue gt 0) ? vt.dblValue : 1}">
<f:convertNumber pattern="0.#" />
</h:outputText>
</p:column>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p:column headerText="Pack Size" style="padding: 6px;"> | |
| <h:outputText value="#{(vt.dblValue ne null and vt.dblValue gt 0) ? vt.dblValue : 1}"> | |
| <f:convertNumber pattern="0.#" /> | |
| </h:outputText> | |
| <p:column headerText="Pack Size" style="padding: 6px;"> | |
| <h:outputText value="#{((vt.itemTypeName eq 'Ampp' or vt.itemTypeName eq 'Vmpp') and vt.dblValue ne null and vt.dblValue gt 0) ? vt.dblValue : 1}"> | |
| <f:convertNumber pattern="0.#" /> | |
| </h:outputText> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/webapp/pharmacy/pharmacy_transfer_request.xhtml` around lines 171 -
174, The Pack Size column is showing vt.dblValue for any positive number; update
the EL in the h:outputText so dblValue is used only when the item type is Ampp
or Vmpp. Replace the current condition "(vt.dblValue ne null and vt.dblValue gt
0) ? vt.dblValue : 1" with a compound check that also ensures the DTO's
item-type field (e.g., vt.itemType or the actual property holding the item type)
is 'Ampp' or 'Vmpp' before using vt.dblValue, otherwise default to 1; keep the
<f:convertNumber> formatting unchanged.
1 participant
Summary
Itementity loads in the transfer request autocomplete with lightweightItemDTOprojections (4 typed constructor queries per keystroke instead of 1 full entity query)BillItemFinanceDetailsscalar query via newItemRatesDTOajax=false(full page reload) to AJAX with targeted component updatesJOIN FETCH bi.itemandLEFT JOIN FETCH bi.billItemFinanceDetailstofetchBillItemsbi.item.class eq 'class com.divudi...'comparison that silently broke for Hibernate proxies in edit mode (now usesclass.simpleName, and correctly handles Vmpp which was missing)institutioneagerly inrecentToDepartmentsquery to prevent lazy load inprocessTransferRequestNew files
core/data/dto/ItemRatesDTO.javacore/converter/ItemDtoConverter.javaDB calls reduced per item-add cycle
BillItementity loads for ratesitemFacade.find+ 1 × scalar rates queryitemList+requestTotalsonlyNavigation path for QA testing
fetchBillItemsJOIN FETCH path)Related issues created
ph:historycomposite component N+1 (pendingGrns— out of scope for this PR)PharmacyDepartmentStockDTOcarries fullDepartmententity (out of scope for this PR)Closes #21062
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Summary by CodeRabbit
New Features
Improvements