Add null check to FlowSheetCustomizationService#2429
Conversation
Reviewer's GuideAdds a null-safe scope resolution in FlowSheetCustomizationService.getScopeLevel while applying minor documentation and formatting cleanups across the service. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
sebastian-j-ibanez
added
the
type: maintenance
Review Summary by QodoAdd null check and improve code formatting in FlowSheetCustomizationService
WalkthroughsDescription• Add null check to getDemographicNo() in getScopeLevel() method • Rename parameter cust to custom for consistency across methods • Format JavaDoc comments for improved readability • Align parameter documentation with consistent spacing Diagramflowchart LR
A["getScopeLevel method"] -->|"Add null check"| B["Prevent NullPointerException"]
C["Parameter naming"] -->|"Rename cust to custom"| D["Improved consistency"]
E["JavaDoc formatting"] -->|"Align spacing"| F["Better readability"]
File Changes1. src/main/java/ca/openosp/openo/commn/service/FlowSheetCustomizationService.java
|
Code Review by Qodo
1. JavaDoc @param missing types
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
getScopeLevel, you now guard against a nulldemographicNobut still call"".equals(custom.getProviderNo())without a null check; consider handling a nullproviderNoas well for consistency and to avoid potential NPEs. - If
demographicNocan be null onFlowSheetCustomization, review other call sites that interpret scope (e.g.,resolveCurrentScopeLeveland any DAO queries) to ensure they either never receive null or handle it consistently with the new logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `getScopeLevel`, you now guard against a null `demographicNo` but still call `"".equals(custom.getProviderNo())` without a null check; consider handling a null `providerNo` as well for consistency and to avoid potential NPEs.
- If `demographicNo` can be null on `FlowSheetCustomization`, review other call sites that interpret scope (e.g., `resolveCurrentScopeLevel` and any DAO queries) to ensure they either never receive null or handle it consistently with the new logic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the FlowSheetCustomizationService by renaming parameters for clarity, updating Javadoc formatting, and introducing a null check for demographic numbers. Feedback was provided regarding the getScopeLevel method, which remains vulnerable to a NullPointerException if the custom object is null; a guard clause was suggested to improve robustness.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
fix: add provider number null check fix: add null check for flowsheet customization argument fix: add more null checks
sebastian-j-ibanez
force-pushed
the
fix-1867-flowsheet-customization
branch
from
71220b5 to
e24503b
Compare
|
Just squashed the commits to make the git history cleaner. |
2 participants
Changes made
FlowSheetCustomizationService.getScopeLevel().Summary by Sourcery
Add a null-safe scope resolution for flowsheet customizations and tidy related service documentation and formatting.
Bug Fixes:
Enhancements:
Summary by cubic
Adds null-safety to scope resolution and archiving checks in
FlowSheetCustomizationServiceto prevent NPEs. Ensures correct clinic/provider/patient detection even when fields are missing.Bug Fixes
getScopeLevel()and handle null levels ingetScopeLevelName()andcheckCanArchive()(returns allowed when customization is null).providerNois empty anddemographicNois "0"; patient whendemographicNo!= "0".Refactors
cust->custom) and clean up Javadoc and formatting.Written for commit e24503b. Summary will update on new commits. Review in cubic