Skip to content

Conversation

@coodos
Copy link
Contributor

@coodos coodos commented Dec 8, 2025

Description of change

Issue Number

Type of change

  • Breaking (any change that would cause existing functionality to not work as expected)
  • New (a change which implements a new feature)
  • Update (a change which updates existing functionality)
  • Fix (a change which fixes an issue)
  • Docs (changes to the documentation)
  • Chore (refactoring, build scripts or anything else that isn't user-facing)

How the change has been tested

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • Bug Fixes

    • Resolved duplicate direct message creation when processing webhooks from external sources.
    • Improved chat deduplication logic to reuse existing chats where applicable.
    • Enhanced direct message vs. group chat classification handling.
  • Performance Improvements

    • Made notification dispatch non-blocking, preventing notification failures from impacting transaction flows.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The PR introduces DM deduplication safeguards across multiple services by adding pre-checks to skip webhook processing when descriptions contain "DM ID:". Additionally, it converts synchronous notification dispatch to non-blocking fire-and-forget in LedgerService, and refactors Pictique's chat handling to reuse existing chats instead of creating duplicates.

Changes

Cohort / File(s) Summary
DM Deduplication Guards
platforms/blabsy-w3ds-auth-api/src/controllers/WebhookController.ts, platforms/eCurrency-api/src/web3adapter/watchers/subscriber.ts
Added pre-checks to skip webhook processing when data description starts with "DM ID:", preventing duplicate DM/chat creation with early-return logic and debug logging
Notification Optimization
platforms/eCurrency-api/src/services/LedgerService.ts
Converted synchronous notification dispatch to non-blocking fire-and-forget pattern via setImmediate with error logging, removing blocking await and try/catch
Chat Reuse Logic
platforms/pictique-api/src/controllers/WebhookController.ts
Refactored chat handling to reuse existing chats: loads and updates fields (name, ename, participants, admins) when local context exists or mapping found; implements branching logic for DMs (participant-based) and group chats (name-based eCurrency matching); skips processing for DM ID-marked descriptions

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pictique WebhookController: Significant refactoring with multiple branching paths for chat reuse, DM detection, and field updates—requires careful verification of deduplication logic and field handling across branches
  • Notification blocking behavior: Ensure LedgerService's non-blocking pattern doesn't mask critical notification failures or degrade observability
  • "DM ID:" consistency: Verify guard check implementation is consistent across Blabsy and subscriber modules
  • Participant and admin updates: Validate field merging/overwriting logic when reusing existing chats across different chat types

Possibly related PRs

Suggested reviewers

  • sosweetham
  • xPathin

Poem

🐰 A rabbit hops through webhooks with glee,
"DM IDs?" they skip with a bounce!
Chats reuse, notifications don't block,
No duplicates here—we've got this down!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-idempotency-check-via-description

📜 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 590e742 and 5ce152b.

📒 Files selected for processing (4)
  • platforms/blabsy-w3ds-auth-api/src/controllers/WebhookController.ts (1 hunks)
  • platforms/eCurrency-api/src/services/LedgerService.ts (1 hunks)
  • platforms/eCurrency-api/src/web3adapter/watchers/subscriber.ts (1 hunks)
  • platforms/pictique-api/src/controllers/WebhookController.ts (5 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.

@coodos coodos marked this pull request as ready for review December 8, 2025 17:25
@coodos coodos merged commit 168e5cf into main Dec 8, 2025
3 of 4 checks passed
@coodos coodos deleted the feat/add-idempotency-check-via-description branch December 8, 2025 17:25
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.

3 participants