-
Notifications
You must be signed in to change notification settings - Fork 100
11399 phone number validation for sale for cashier #11587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
11399 phone number validation for sale for cashier #11587
Conversation
Signed-off-by: Chinthaka Prasad Wijerathna <[email protected]>
… 11399-phone-number-validation-for-sale-for-cashier
Signed-off-by: Chinthaka Prasad Wijerathna <[email protected]>
Signed-off-by: Chinthaka Prasad Wijerathna <[email protected]>
…@users.noreply.github.com>
…@users.noreply.github.com>
WalkthroughThis pull request modifies the error handling and validation logic in two Java controller classes and updates associated JSF pages. In the Java layer, the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant AC as AmpController
participant Config as ConfigOptionService
U->>AC: Trigger save action
AC->>AC: Check if current AMP is null
alt Current is null
AC->>U: "No AMP is selected" error message
else Current exists
AC->>AC: Verify current.getId() is not null
AC->>Config: Validate edit privileges
alt Insufficient privilege
AC->>U: "You have no privilage to edit AMPs." error message
else
AC->>AC: Check if AMP name is provided
alt Name missing
AC->>U: "Please add a name to AMP" error message
else
AC->>AC: Proceed with saving AMP
end
end
end
sequenceDiagram
participant U as User
participant PSC as PharmacySaleController
participant Config as ConfigOptionService
U->>PSC: Trigger settle pre bill action
PSC->>Config: Check if patient's phone is required
alt Phone requirement enabled
PSC->>PSC: Validate PatientPhoneNumber and PatientMobileNumber
alt Both numbers are missing
PSC->>U: Prompt: "Enter a phone number"
else if patient ID is missing
alt Phone number length < 10
PSC->>U: "Valid phone number must be entered" error message
else
PSC->>PSC: Continue pre bill settlement
end
end
else
PSC->>PSC: Continue pre bill settlement
end
Possibly related PRs
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.31.1)src/main/java/com/divudi/bean/pharmacy/PharmacySaleController.java✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/divudi/bean/pharmacy/PharmacySaleController.java (1)
2070-2083
: Enhance phone number validation for stronger input checking.The validation logic correctly checks for phone number presence when required by configuration, but it has a hardcoded minimum length of 10 digits that could be configurable instead.
Consider these improvements:
- Extract the minimum phone length (10) as a configurable parameter
- Add pattern validation for phone numbers beyond just length
- Apply consistent validation for both new and existing patients
- if (configOptionApplicationController.getBooleanValueByKey("Patient Phone number is mandotary in sale for cashier", true)) { - if (getPatient().getPatientPhoneNumber() == null && getPatient().getPatientMobileNumber() == null) { - JsfUtil.addErrorMessage("Please enter phone number of the patient"); - return; - } else if (getPatient().getId() == null) { - if (getPatient().getPatientPhoneNumber() != null && !(String.valueOf(getPatient().getPatientPhoneNumber()).length() >= 10)) { - JsfUtil.addErrorMessage("Please enter valid phone number with more than 10 digits of the patient"); - return; - } else if (getPatient().getPatientMobileNumber() != null && !(String.valueOf(getPatient().getPatientMobileNumber()).length() >= 10)) { - JsfUtil.addErrorMessage("Please enter valid mobile number with more than 10 digits of the patient"); - return; - } - } - } + if (configOptionApplicationController.getBooleanValueByKey("Patient Phone number is mandotary in sale for cashier", true)) { + // Extract minimum length to a constant or configuration + int minPhoneLength = configOptionApplicationController.getIntValueByKey("Minimum phone number length", 10); + + if (getPatient().getPatientPhoneNumber() == null && getPatient().getPatientMobileNumber() == null) { + JsfUtil.addErrorMessage("Please enter phone number of the patient"); + return; + } + + // Validate phone number format regardless of patient being new or existing + if (getPatient().getPatientPhoneNumber() != null && + !(String.valueOf(getPatient().getPatientPhoneNumber()).length() >= minPhoneLength)) { + JsfUtil.addErrorMessage("Please enter valid phone number with at least " + minPhoneLength + " digits"); + return; + } else if (getPatient().getPatientMobileNumber() != null && + !(String.valueOf(getPatient().getPatientMobileNumber()).length() >= minPhoneLength)) { + JsfUtil.addErrorMessage("Please enter valid mobile number with at least " + minPhoneLength + " digits"); + return; + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/divudi/bean/pharmacy/AmpController.java
(2 hunks)src/main/java/com/divudi/bean/pharmacy/PharmacySaleController.java
(1 hunks)src/main/webapp/pharmacy/admin/amp.xhtml
(1 hunks)src/main/webapp/pharmacy/pharmacy_bill_retail_sale_1.xhtml
(1 hunks)
🔇 Additional comments (5)
src/main/webapp/pharmacy/admin/amp.xhtml (1)
40-40
: Improved security by conditionally enabling the delete AMP buttonThis change ensures that the delete button is only enabled when the application's configuration option for editing/deleting AMPs from Pharmacy Administration is set to true, which aligns with the privilege check in the controller.
src/main/java/com/divudi/bean/pharmacy/AmpController.java (3)
658-658
: Improved error message clarityChanged the error message from a typo "Nothuing selected" to a clearer "No AMP is selected", which improves user experience.
662-667
: Added permission check for AMP editingThis change enforces proper privilege control for editing AMPs, checking if the configuration option "Enable edit and delete AMP from Pharmacy Administration" is enabled before allowing edits to existing AMPs. This is a good security practice that aligns with the disabled attribute added to the delete button in the UI.
670-670
: Improved validation message clarityChanged the error message from "No Name" to a more instructive "Please add a name to AMP", which provides clearer guidance to users.
src/main/webapp/pharmacy/pharmacy_bill_retail_sale_1.xhtml (1)
715-716
: Controller reference and path updates for consistency.The changes standardize the controller references and paths between the top navigation buttons and the bottom preview panel buttons. This ensures consistent behavior between the different sections of the page.
add option to edit and delete AMP from pharmacy administration.
fixed - #11399, #11408 #11255
Summary by CodeRabbit
Bug Fixes
New Features