Skip to content

fix: ignore messages that don't come from Knock in KnockExpoPushNotificationProvider#637

Merged
kylemcd merged 1 commit intomainfrom
kyle-kno-8889-knockexpopushnotificaitonprovider-assumes-all-notifications
Jul 10, 2025
Merged

fix: ignore messages that don't come from Knock in KnockExpoPushNotificationProvider#637
kylemcd merged 1 commit intomainfrom
kyle-kno-8889-knockexpopushnotificaitonprovider-assumes-all-notifications

Conversation

@kylemcd
Copy link
Copy Markdown
Member

@kylemcd kylemcd commented Jul 10, 2025

Description

As reported in #589, the KnockExpoPushNotificationProvider tries to process messages that don't come from knock which results in 404 errors. This PR adds a check to updateKnockMessageStatusFromNotification that ensures that only messages which contain knock_message_id are processed, the rest being ignored.

@linear
Copy link
Copy Markdown

linear Bot commented Jul 10, 2025

@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
javascript-ms-teams-connect-example ⬜️ Ignored (Inspect) Visit Preview Jul 10, 2025 4:39pm
javascript-nextjs-example ⬜️ Ignored (Inspect) Visit Preview Jul 10, 2025 4:39pm
javascript-slack-connect-example ⬜️ Ignored (Inspect) Visit Preview Jul 10, 2025 4:39pm
javascript-slack-kit-example ⬜️ Ignored (Inspect) Visit Preview Jul 10, 2025 4:39pm

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jul 10, 2025

🦋 Changeset detected

Latest commit: 7646f59

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@knocklabs/expo Patch
@knocklabs/expo-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Copy Markdown
Member Author

kylemcd commented Jul 10, 2025

@kylemcd kylemcd marked this pull request as ready for review July 10, 2025 16:10
@kylemcd kylemcd requested review from cjbell and mattmikolay July 10, 2025 16:10
@kylemcd kylemcd changed the title fix: ignore messages that don't from Knock in KnockExpoPushNotificationProvider fix: ignore messages that don't come from Knock in KnockExpoPushNotificationProvider Jul 10, 2025
Copy link
Copy Markdown
Member Author

kylemcd commented Jul 10, 2025

Merge activity

  • Jul 10, 4:20 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 10, 4:33 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 10, 4:37 PM UTC: @kylemcd merged this pull request with Graphite.

@kylemcd kylemcd changed the base branch from kyle-kno-9102-add-expo-build-step-to-ci to graphite-base/637 July 10, 2025 16:27
@kylemcd kylemcd changed the base branch from graphite-base/637 to main July 10, 2025 16:31
@kylemcd kylemcd requested a review from a team as a code owner July 10, 2025 16:31
@kylemcd kylemcd requested review from thomaswhyyou and removed request for a team July 10, 2025 16:31
@kylemcd kylemcd force-pushed the kyle-kno-8889-knockexpopushnotificaitonprovider-assumes-all-notifications branch from 85c9612 to 7646f59 Compare July 10, 2025 16:33
@kylemcd kylemcd merged commit f1b129c into main Jul 10, 2025
7 of 12 checks passed
@kylemcd kylemcd deleted the kyle-kno-8889-knockexpopushnotificaitonprovider-assumes-all-notifications branch July 10, 2025 16:37
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 10, 2025

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 61.56%. Comparing base (164af54) to head (7646f59).
Report is 54 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...modules/push/KnockExpoPushNotificationProvider.tsx 0.00% 9 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #637       +/-   ##
===========================================
+ Coverage   23.74%   61.56%   +37.81%     
===========================================
  Files         180      175        -5     
  Lines        6775     6850       +75     
  Branches      212      774      +562     
===========================================
+ Hits         1609     4217     +2608     
+ Misses       5166     2607     -2559     
- Partials        0       26       +26     
Files with missing lines Coverage Δ
...modules/push/KnockExpoPushNotificationProvider.tsx 0.44% <0.00%> (-0.02%) ⬇️

... and 102 files with indirect coverage changes

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.

KnockExpoPushNotificaitonProvider assumes all notifications came from knock

2 participants