Skip to content

Conversation

@jesusmmp
Copy link
Contributor

@jesusmmp jesusmmp commented Dec 5, 2025

https://sakaiproject.atlassian.net/browse/SAK-52120

Summary by CodeRabbit

  • Bug Fixes
    • Improved selected day event handling logic to prevent edge cases and enhance reliability in calendar day selection and event tracking.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

Refactored isViewingSelectedDay logic in CalendarBean to handle null selectedDay up-front, returning false immediately when null. When non-null, computes day time, updates selectedDayHasEvents, and returns true only if no selectedEventRef. Removed redundant null checks.

Changes

Cohort / File(s) Summary
Calendar Summary Tool UI Bean
calendar/calendar-summary-tool/tool/src/java/org/sakaiproject/tool/summarycalendar/ui/CalendarBean.java
Reworked isViewingSelectedDay logic to handle null selectedDay up-front, eliminating redundant null checks and improving control flow

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references a Jira issue (SAK-52150) and describes an error related to reading [selectedEvent] in CalendarBean, which aligns with the code change that refactors the isViewingSelectedDay logic to properly handle null selectedDay and selectedEventRef.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c30a21 and 26006dd.

📒 Files selected for processing (1)
  • calendar/calendar-summary-tool/tool/src/java/org/sakaiproject/tool/summarycalendar/ui/CalendarBean.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

📄 CodeRabbit inference engine (.cursor/rules/logging-rule.mdc)

**/*.java: Use SLF4J parameterized logging (logger.info("Value is: {}", value)) instead of string concatenation (logger.info("Value is: " + value))
Log messages and code comments should be in English. Log messages should never be translated.

**/*.java: Java: Never use local variable type inference (var). Always declare explicit types. Yes: Map<String, Integer> counts = new HashMap<>(); No: var counts = new HashMap<String, Integer>();
When proposing Java code, spell out full types in local variable declarations, for loops, and try-with-resources
When editing Java, prefer clarity over brevity; avoid introducing language features that aren't widely used in the repo
Treat any PR or suggestion containing Java var as non-compliant. Recommend replacing with explicit types before merge

**/*.java: Use Java 17 for trunk development (Java 11 was used for Sakai 22 and Sakai 23)
Do not use local variable type inference (var) in Java code. Always declare explicit types (e.g., List<String> names = new ArrayList<>(); not var names = new ArrayList<String>();). Enforced by Checkstyle rule during mvn validate

Files:

  • calendar/calendar-summary-tool/tool/src/java/org/sakaiproject/tool/summarycalendar/ui/CalendarBean.java
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: sakai-deploy
  • GitHub Check: maven-build
  • GitHub Check: maven-build
🔇 Additional comments (1)
calendar/calendar-summary-tool/tool/src/java/org/sakaiproject/tool/summarycalendar/ui/CalendarBean.java (1)

718-729: Null‑safety and state handling in isViewingSelectedDay look correct

The new early return when selectedDay is null cleanly avoids accessing it (and avoids NPEs in downstream usages driven by this flag), and the recomputation of selectedDayHasEvents from getScheduleEventsForDay before returning selectedDayHasEvents && selectedEventRef == null keeps the previous semantics while making the logic clearer. No issues spotted with time zone handling or side effects here; this change reads as a safe and targeted fix.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant