make PO Number and GRN Number clickable in GRN Bill Item Report#18756
Merged
ChinthakaPrasad merged 1 commit intodevelopmentfrom Feb 26, 2026
Merged
Conversation
Signed-off-by: ishira-web <ishira.pahasara@icloud.com>
Contributor
WalkthroughAdded GRN and PO bill ID fields to PharmacyGrnBillItemDTO. Updated the data query to fetch these IDs. Created two navigation methods in CommonReport to view associated bills. Modified the report view to render PO and GRN numbers as conditional clickable links for bill navigation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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.40.5)src/main/java/com/divudi/bean/report/CommonReport.javasrc/main/java/com/divudi/bean/report/PharmacySaleReportController.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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/divudi/bean/report/PharmacySaleReportController.java (1)
439-451:⚠️ Potential issue | 🟠 MajorAvoid implicit inner-join filtering on referenceBill (rows lost).
Using
bi.bill.referenceBill.*in the select introduces an implicit inner join, so GRNs without a linked PO are filtered out and the UI fallback for null IDs never triggers. Use a LEFT JOIN alias and select from that alias instead.🛠️ Suggested fix (LEFT JOIN referenceBill)
- sql = "select new com.divudi.core.data.dto.PharmacyGrnBillItemDTO( " - + " bi.bill.deptId, " - + " bi.bill.department.name, " - + " bi.bill.referenceBill.deptId, " - + " bi.bill.fromInstitution.name, " - + " bi.item.name, " - + " bi.pharmaceuticalBillItem.qty, " - + " bi.pharmaceuticalBillItem.freeQty, " - + " bi.pharmaceuticalBillItem.purchaseRate, " - + " bi.bill.referenceBill.saleValue, " - + " bi.bill.referenceBill.netTotal, " - + " bi.bill.id, " - + " bi.bill.referenceBill.id ) " - + " from BillItem bi " + sql = "select new com.divudi.core.data.dto.PharmacyGrnBillItemDTO( " + + " bi.bill.deptId, " + + " bi.bill.department.name, " + + " ref.deptId, " + + " bi.bill.fromInstitution.name, " + + " bi.item.name, " + + " bi.pharmaceuticalBillItem.qty, " + + " bi.pharmaceuticalBillItem.freeQty, " + + " bi.pharmaceuticalBillItem.purchaseRate, " + + " ref.saleValue, " + + " ref.netTotal, " + + " bi.bill.id, " + + " ref.id ) " + + " from BillItem bi " + + " left join bi.bill.referenceBill ref "🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/divudi/bean/report/PharmacySaleReportController.java` around lines 439 - 451, The JPQL uses bi.bill.referenceBill.* which forces an implicit inner join and drops GRNs with no referenceBill; modify the query in PharmacySaleReportController where the sql string is built so you LEFT JOIN bi.bill.referenceBill AS rb (or similar alias) and then select rb.deptId, rb.saleValue, rb.netTotal, rb.id (or null-safe aliases) instead of bi.bill.referenceBill.*; ensure the selected fields still match the PharmacyGrnBillItemDTO constructor parameter order and update the SELECT expressions to use the new alias.
🧹 Nitpick comments (2)
src/main/webapp/pharmacy/pharmacy_report_grn_billitem.xhtml (2)
88-88:h:outputTextfallback is inconsistent withh:outputLabelused elsewhere in the table.The rest of the table cells use
h:outputLabel; the null-fallback renders useh:outputText. Either is functionally fine (h:outputTextis arguably more semantically appropriate for display-only values), but aligning with the surrounding pattern would improve consistency.♻️ Align fallback with surrounding pattern (optional)
-<h:outputText value="#{i.poNo}" rendered="#{i.poBillId eq null}"/> +<h:outputLabel value="#{i.poNo}" rendered="#{i.poBillId eq null}"/>-<h:outputText value="#{i.grnNo}" rendered="#{i.grnBillId eq null}"/> +<h:outputLabel value="#{i.grnNo}" rendered="#{i.grnBillId eq null}"/>Also applies to: 99-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/pharmacy/pharmacy_report_grn_billitem.xhtml` at line 88, Replace the inconsistent h:outputText used as the null-fallback with the same component used elsewhere for table cells (h:outputLabel) so the cell rendering pattern is uniform: change the element rendering "#{i.poBillId eq null}" that currently uses <h:outputText value="#{i.poNo}" ...> to use <h:outputLabel> with the same value and rendered expression (refer to the occurrences of value="#{i.poNo}" and rendered="#{i.poBillId eq null}" to locate the spot). Ensure attributes (value and rendered) are preserved when swapping h:outputText to h:outputLabel.
83-88: Consider privilege-guarding the navigationp:commandLinkelements.The links navigate to
pharmacy_reprint_po_billandpharmacy_grn_bill— sensitive bill detail pages. Neither link has arendered="#{webUserController.hasPrivilege(...)}"guard. The target pages presumably enforce their own access control, but adding guards here would be consistent with the coding guideline: "UsewebUserController.hasPrivilege()for UI privilege integration."Note: the existing Process/Download/Print buttons in this file are also ungarded, so this is a pre-existing systemic gap — applying it here when the rest of the form is addressed would be ideal.
Also applies to: 94-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/pharmacy/pharmacy_report_grn_billitem.xhtml` around lines 83 - 88, Wrap the navigation commandLinks with UI privilege checks by adding rendered attributes that call webUserController.hasPrivilege(...) so links are only shown to authorized users; specifically, add rendered="#{webUserController.hasPrivilege('pharmacy_reprint_po_bill')}" to the p:commandLink that invokes commonReport.navigateToViewPOBillFromGRNBillItemReport(i.poBillId) and add the corresponding rendered check (e.g. webUserController.hasPrivilege('pharmacy_grn_bill')) to the other commandLink(s) in this fragment (the ones that navigate to the GRN/bill detail pages); apply the same pattern to the other similar links in the same block.
🤖 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/divudi/bean/report/CommonReport.java`:
- Around line 297-307: Add privilege checks to the two navigation actions so
unauthorized users cannot load bills directly: in
navigateToViewGrnBillFromGRNBillItemReport and
navigateToViewPOBillFromGRNBillItemReport call
webUserController.hasPrivilege(...) with the appropriate privilege key(s) before
calling billFacade.find(...); if the check fails, do not load the bill and
return null or an access-denied navigation string (and optionally log or set a
Faces message). Ensure you use the same privilege keys used elsewhere in the UI
for viewing GRN/PO bills so the behavior matches existing gating.
In `@src/main/webapp/pharmacy/pharmacy_report_grn_billitem.xhtml`:
- Around line 83-88: The p:commandLink lacking a title attribute reduces
accessibility; update the p:commandLink(s) that call
commonReport.navigateToViewPOBillFromGRNBillItemReport(i.poBillId) (and the
sibling link at lines 94-99) to include a descriptive title (for example using
title="#{msgs.viewPO}: #{i.poNo}" or a similar i18n string) so screen readers
and hover users get context; ensure the title references the PO identifier
(i.poNo or i.poBillId) and add matching title text for any other p:commandLink
instances in this fragment.
---
Outside diff comments:
In `@src/main/java/com/divudi/bean/report/PharmacySaleReportController.java`:
- Around line 439-451: The JPQL uses bi.bill.referenceBill.* which forces an
implicit inner join and drops GRNs with no referenceBill; modify the query in
PharmacySaleReportController where the sql string is built so you LEFT JOIN
bi.bill.referenceBill AS rb (or similar alias) and then select rb.deptId,
rb.saleValue, rb.netTotal, rb.id (or null-safe aliases) instead of
bi.bill.referenceBill.*; ensure the selected fields still match the
PharmacyGrnBillItemDTO constructor parameter order and update the SELECT
expressions to use the new alias.
---
Nitpick comments:
In `@src/main/webapp/pharmacy/pharmacy_report_grn_billitem.xhtml`:
- Line 88: Replace the inconsistent h:outputText used as the null-fallback with
the same component used elsewhere for table cells (h:outputLabel) so the cell
rendering pattern is uniform: change the element rendering "#{i.poBillId eq
null}" that currently uses <h:outputText value="#{i.poNo}" ...> to use
<h:outputLabel> with the same value and rendered expression (refer to the
occurrences of value="#{i.poNo}" and rendered="#{i.poBillId eq null}" to locate
the spot). Ensure attributes (value and rendered) are preserved when swapping
h:outputText to h:outputLabel.
- Around line 83-88: Wrap the navigation commandLinks with UI privilege checks
by adding rendered attributes that call webUserController.hasPrivilege(...) so
links are only shown to authorized users; specifically, add
rendered="#{webUserController.hasPrivilege('pharmacy_reprint_po_bill')}" to the
p:commandLink that invokes
commonReport.navigateToViewPOBillFromGRNBillItemReport(i.poBillId) and add the
corresponding rendered check (e.g.
webUserController.hasPrivilege('pharmacy_grn_bill')) to the other commandLink(s)
in this fragment (the ones that navigate to the GRN/bill detail pages); apply
the same pattern to the other similar links in the same block.
ChinthakaPrasad
deleted the
#18578-make-bill-numbers-clickable-in-grn-bill-item-report
branch
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
2 participants
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.
PO Number and GRN Number in the GRN Bill Item Report are now clickable links that navigate directly to the corresponding bill detail pages (pharmacy_reprint_po_bill and pharmacy_grn_bill).
Clicking a GRN Number opens the GRN bill view; clicking a PO Number opens the Purchase Order reprint view matching the same navigation pattern used in the GRN Summary report.
Changes
Summary by CodeRabbit