Skip to content

Updated Integration Guide - #440

Open
raj-cometchat wants to merge 16 commits into
mainfrom
docs/new-flutter-push-notifications
Open

Updated Integration Guide#440
raj-cometchat wants to merge 16 commits into
mainfrom
docs/new-flutter-push-notifications

Conversation

@raj-cometchat

@raj-cometchat raj-cometchat commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

Related Issue(s)

Type of Change

  • Documentation correction/update
  • New documentation
  • Improvement to existing documentation
  • Typo fix
  • Other (please specify)

Checklist

  • I have read the CONTRIBUTING document
  • My branch name follows the naming convention
  • My changes follow the documentation style guide
  • I have checked for spelling and grammar errors
  • All links in my changes are valid and working
  • My changes are accurately described in this pull request

Additional Information

Screenshots (if applicable)

@mintlify

mintlify Bot commented Jul 27, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
cometchat 🟢 Ready View Preview Jul 27, 2026, 5:37 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@mintlify

mintlify Bot commented Jul 27, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
cometchat 🟡 Building Jul 27, 2026, 5:32 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@ashfaqcometchat ashfaqcometchat changed the title Updated Migration Guide Updated Integration Guide Jul 28, 2026
raj-dubey1
raj-dubey1 previously approved these changes Jul 28, 2026
ashfaqcometchat and others added 2 commits July 28, 2026 15:37
Replace the copy-the-UI-Kit-sample approach with the released
com.cometchat:push-notifications-android:1.0.0 drop-in SDK, mirroring the
new Flutter guide's structure. Covers dependency setup, Application init
via PNConfiguration.Builder, forwarding FCM payloads to
handlePushNotification, token registration/refresh, notification-tap and
call-event listeners, badge count, testing, and troubleshooting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s-sdk

Rewrite Android push guide for drop-in push-notifications-android SDK
Consolidates the two legacy iOS pages (APNs + FCM copy-files patterns)
into a single notifications/ios-push-notifications.mdx that documents
the CometChatPushNotifications drop-in SDK: SPM/CocoaPods install,
AppDelegate + SceneDelegate wiring, CometChatPushNotificationsDelegate
for taps and calls, foreground suppression, badge, and troubleshooting.

Updates docs.json nav + redirects, the notifications and getting-started
landing pages (single "iOS" card, no FCM-iOS Firebase tab), and stray
cross-references from Flutter VoIP and legacy iOS extensions pages.
The legacy 2.0 extensions page is out of scope for the push SDK
rewrite; the redirect on the old slug handles the old link.
@jitvarpatil

Copy link
Copy Markdown
Contributor

Docs review — 🟠 Request changes

A large push-notifications restructure (18 files, +1,104/−2,831) consolidating the split Flutter (android+ios) and iOS (APNs+FCM) push pages into single flutter-push-notifications / ios-push-notifications pages. Content and redirect intent are good — but there's an execution bug that will leave blank pages and silently defeat the redirects.

🟠 P1 — 3 old pages emptied instead of deleted (breaks their own redirects)

git diff --name-status shows these as M (modified to 0 bytes), still tracked (empty blob e69de29b), not deleted:

  • notifications/flutter-push-notifications-android.mdx → 0 bytes
  • notifications/flutter-push-notifications-ios.mdx → 0 bytes
  • notifications/ios-apns-push-notifications.mdx → 0 bytes

Only ios-fcm-push-notifications.mdx was correctly git rm'd.

Why it matters: the PR does add redirects for all four old URLs (e.g. /notifications/flutter-push-notifications-android → /notifications/flutter-push-notifications) — which is exactly right — but a redirect only fires when no page exists at that path. Since these 3 files still exist (empty), Mintlify serves a blank page instead of applying the redirect. So the consolidation half-works: ios-fcm redirects correctly (properly deleted); the other 3 render blank and their redirects are dead-on-arrival.

Fix: actually delete them —

git rm notifications/flutter-push-notifications-android.mdx \
       notifications/flutter-push-notifications-ios.mdx \
       notifications/ios-apns-push-notifications.mdx

— then the already-present redirects take effect.

🟠 P1 — in-content link to a deleted page

notifications/push-overview.mdx still links to /notifications/ios-fcm-push-notifications (now deleted). A redirect exists so it won't hard-404, but an in-content link shouldn't rely on a redirect — point it directly at /notifications/ios-push-notifications.

✅ What passed

  • Redirects added for all four consolidated old URLs + push-integration → push-overview, with correct destinations. Good coverage — just undermined by the empty files above.
  • Merged content is clean: both new pages have proper frontmatter (title: "Flutter" / "iOS"), a single consistent version pin (cometchat_push_notifications: ^1.0.1 — no version drift), no placeholders/TODOs, and no broken "see above/below" cross-refs from the merge.
  • Build/nav: 0 unresolved nav refs. (The /notifications/overview orphan the analyzer flagged is pre-existing — not in this PR.)

Well-intentioned consolidation with correct redirects and clean merged content — it just needs the 3 pages deleted rather than emptied, plus the one in-content link updated. Happy to re-check once that's done.

🤖 Automated docs review (Mintlify link/redirect/nav/content checks).

Both 'iOS - FCM' and 'iOS - APNs' sample cards pointed at
.../SampleAppPushNotificationAPNs/Push Notification + VoIP, which
404s (that subfolder no longer exists in the v5 branch). Collapse
to a single 'iOS' card pointing at the SampleAppPushNotificationAPNs
folder itself, which is where the sample now lives.
The 'resolved conflicts' merge on this branch resurrected
notifications/ios-apns-push-notifications.mdx as an empty file and
reintroduced the split 'iOS (APNs)' / 'iOS (FCM)' cards on
notifications/push-overview.mdx pointing at deleted slugs. A blank
file at the old path silently defeats the redirect, so:

- git rm the empty ios-apns page so the redirect fires again.
- Collapse the two iOS cards on push-overview into a single 'iOS'
  card pointing at /notifications/ios-push-notifications.
Same merge-conflict-resolution regression as the iOS side: the
'resolved conflicts' merge on this branch resurrected
notifications/flutter-push-notifications-android.mdx and
notifications/flutter-push-notifications-ios.mdx as empty (0-byte)
files, which silently defeats their /notifications/flutter-push-
notifications redirects, and reintroduced the split
'Flutter (Android)' / 'Flutter (iOS)' cards on push-overview
pointing at the deleted slugs. Also the two Flutter sample-app
cards on the notifications landing page pointed at the same
sample_app_push_notifications URL.

- git rm both empty Flutter pages so the redirects fire again.
- Collapse the two Flutter cards on push-overview into a single
  'Flutter' card pointing at /notifications/flutter-push-notifications.
- Collapse the two Flutter sample-app cards on notifications.mdx
  into one 'Flutter' card at the same working URL.
@adityakotasthane07
adityakotasthane07 self-requested a review August 10, 2026 09:17
@jitvarpatil

Copy link
Copy Markdown
Contributor

Re-review — ✅ Approve

Both P1 findings from my earlier review are fully resolved (the new commits — "Re-delete empty Flutter push pages…", "Re-delete ios-apns page and fix stale iOS links…" — address them directly). Re-verified on head 0df4bdd.

✅ Fixed

  • All 4 old pages now properly deleted — analyzer reports removed: 4 (was removed: 1 with 3 emptied to 0 bytes). No 0-byte .mdx files remain in notifications/.
  • All 4 deleted URLs have working redirects ([2a] 0 of 4 unredirected) — and now that the files are genuinely gone, the redirects will actually fire:
    • flutter-push-notifications-android / -iosflutter-push-notifications
    • ios-apns-push-notifications / ios-fcm-push-notificationsios-push-notifications
  • Broken in-content link fixed[4] 0 broken links (was 1); push-overview.mdx now links directly to /notifications/ios-push-notifications.
  • 0 nav breaks. (The /notifications/overview orphan is pre-existing — not this PR.)

Combined with the already-verified merged content (proper frontmatter, single consistent version pin cometchat_push_notifications: ^1.0.1, no placeholders/contradictions), the consolidation is now correct end-to-end.

Nicely done on the redirect coverage. Ready to merge. 🚀

🤖 Automated docs review — re-check after fixes.

@jitvarpatil jitvarpatil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-review — ✅ Approve

Both P1 findings from my earlier review are fully resolved (the new commits — "Re-delete empty Flutter push pages…", "Re-delete ios-apns page and fix stale iOS links…" — address them directly). Re-verified on head 0df4bdd.

✅ Fixed

  • All 4 old pages now properly deleted — analyzer reports removed: 4 (was removed: 1 with 3 emptied to 0 bytes). No 0-byte .mdx files remain in notifications/.
  • All 4 deleted URLs have working redirects ([2a] 0 of 4 unredirected) — and now that the files are genuinely gone, the redirects will actually fire:
    • flutter-push-notifications-android / -iosflutter-push-notifications
    • ios-apns-push-notifications / ios-fcm-push-notificationsios-push-notifications
  • Broken in-content link fixed[4] 0 broken links (was 1); push-overview.mdx now links directly to /notifications/ios-push-notifications.
  • 0 nav breaks. (The /notifications/overview orphan is pre-existing — not this PR.)

Combined with the already-verified merged content (proper frontmatter, single consistent version pin cometchat_push_notifications: ^1.0.1, no placeholders/contradictions), the consolidation is now correct end-to-end.

Nicely done on the redirect coverage. Ready to merge. 🚀

🤖 Automated docs review — re-check after fixes.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

7 participants