Skip to content

Conversation

diegolmello
Copy link
Member

@diegolmello diegolmello commented Sep 24, 2025

Proposed changes

Issue(s)

#6661 (comment)

How to test or reproduce

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

  • Bug Fixes

    • Prevented crashes when push notifications have missing or null message content by handling empty messages safely.
  • Improvements

    • Increased reliability of push notification processing so the app remains stable when message data is incomplete.

@diegolmello diegolmello marked this pull request as ready for review September 24, 2025 13:20
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds an early null check in extractMessage of CustomPushNotification to return an empty string when message is null; existing parsing and substring logic remain unchanged and no public signatures were modified.

Changes

Cohort / File(s) Summary of Changes
Null-safety hardening
android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java
Added an early return in extractMessage when message is null; preserved existing ejson.type checks and substring computation. No public API or signature changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • OtavioStasiak

Poem

A hop, a check, a subtle tweak,
I guard the strings so nothing leaks.
Notifications bounce and sing,
Safe and sound—my little spring.
🥕🐇

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 succinctly and accurately describes the primary change: adding a null-check in CustomPushNotification to prevent crashes when the message is null. This matches the code change in extractMessage and is specific and clear enough for reviewers to understand the intent.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 370b8f6 and 2ef7d97.

📒 Files selected for processing (1)
  • android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java (1 hunks)

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java (1)

247-253: Skip/handle empty notification messages (avoid blank notifications)

  • Use extractMessage(...) when setting content text in getNotificationBuilder so the notification body matches MessagingStyle; if the result is empty/null, use a fallback (e.g., title) or omit setContentText.
  • Before calling InboxStyle.addLine(message) and MessagingStyle.addMessage(m, ...), skip adding when message == null or m.trim().isEmpty().
  • Alternatively, have extractMessage return null for “no message” and treat null as “do not display” in the two places above.

File: android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java — methods: getNotificationBuilder, notificationStyle, extractMessage.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6ce0dbc and 370b8f6.

📒 Files selected for processing (1)
  • android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (1)
android/app/src/main/java/chat/rocket/reactnative/notification/CustomPushNotification.java (1)

244-246: LGTM! Critical null safety improvement.

The null check for the message parameter in extractMessage is a critical improvement that will prevent crashes when null messages are encountered in push notifications. The early return of an empty string when message is null is appropriate, as it allows the notification processing to continue gracefully rather than crashing.

Based on the PR objectives, this addresses the referenced crash issue and follows Android development best practices for null safety. The implementation is minimal, focused, and doesn't alter the existing logic flow for non-null cases.

Copy link

Android Build Available

Rocket.Chat Experimental 4.65.0.107405

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTd4bhj_J_nvWjZXAuz0QPUU7KSP4Fqq3GsQEJmW4YSzGIB9NgWXi6i-aKCmznl-kiLWTy4ajDWLkwUuseH

Copy link
Contributor

@OtavioStasiak OtavioStasiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@diegolmello diegolmello force-pushed the fix.android-push-null-message branch from 370b8f6 to 2ef7d97 Compare September 26, 2025 16:56
@diegolmello diegolmello merged commit b76942d into develop Sep 26, 2025
5 of 9 checks passed
@diegolmello diegolmello deleted the fix.android-push-null-message branch September 26, 2025 16:56
@diegolmello diegolmello requested a deployment to official_android_build September 26, 2025 17:00 — with GitHub Actions Waiting
@diegolmello diegolmello requested a deployment to experimental_android_build September 26, 2025 17:00 — with GitHub Actions Waiting
@diegolmello diegolmello requested a deployment to experimental_ios_build September 26, 2025 17:00 — with GitHub Actions Waiting
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.

2 participants