Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1: mismatched response length #2

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

Fimeo
Copy link
Contributor

@Fimeo Fimeo commented Jan 23, 2025

The last fix seems to pose a problem

In the case of a payload containing several recipients, the line

r.Data[i].MessageItem = msgs[i]

causes an index out of bouds when adding message items in each response object.

Match each MessageItem with the response id.

According to the expo documentation, the output order of push token receipts is guaranteed by following the order of the messages in the input payload.

This merge request offers a solution issue #1 on mismatched response length.

The signature of the push single method had to be adapted to return an array of MessageResponse (one for each recipient of the Message).

Also fixes the NewClient with nil argument to avoid panic on nil pointer exception.

Update readme to match new function signature.

Contains breaking changes on push single method.

Thanks

@Fimeo Fimeo mentioned this pull request Jan 23, 2025
@Fimeo Fimeo changed the title fix(client): mismatched response length #1 fix(client): mismatched response length Jan 23, 2025
@Fimeo Fimeo changed the title fix(client): mismatched response length Fix #1: mismatched response length Jan 23, 2025
Copy link
Owner

@9ssi7 9ssi7 left a comment

Choose a reason for hiding this comment

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

Super except for a small change. Thanks for your contribution

client.go Outdated Show resolved Hide resolved
@9ssi7 9ssi7 merged commit 6e52671 into 9ssi7:main Jan 23, 2025
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