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

feat: allow numeric timestamp in listMessages #143

Conversation

peterferguson
Copy link
Contributor

When querying messages based on Dates the timestamps are usually coming from a local cache (in our case, but I assume in most others too).

In order to avoid multiple conversions number / string -> Date -> number, I think it makes sense to allow the users to pass a number to listMessages & listBatchMessages.

@peterferguson peterferguson requested a review from a team as a code owner October 27, 2023 08:02
Copy link
Contributor

@nplasterer nplasterer left a comment

Choose a reason for hiding this comment

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

Hey thanks for your contribution! It looks good but would you mind updating or adding a test in this file to make sure it still works across the iOS and Android bridge 🙏

https://github.com/xmtp/xmtp-react-native/blob/main/example/src/tests.ts#L30-L69

@nplasterer
Copy link
Contributor

Hey @peterferguson just nudging on this to see if you have time to add the tests? Let me know. 🙏

@peterferguson
Copy link
Contributor Author

Hey @nplasterer I pushed a test but I haven't actually been able to run it.

I didn't see a test command & also simulator builds with this repo in them fail on my machine (see related post)

Copy link
Contributor

@nplasterer nplasterer left a comment

Choose a reason for hiding this comment

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

I made a small tweak to the test and got them passing. Will look into the issue posted in discord. 🤔

iOS Android
Screenshot 2023-11-07 at 6 56 45 PM Screenshot 2023-11-07 at 6 54 57 PM

@nplasterer nplasterer merged commit f1e74ef into xmtp:main Nov 8, 2023
1 of 2 checks passed
Copy link
Contributor

github-actions bot commented Nov 8, 2023

🎉 This PR is included in version 1.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@peterferguson peterferguson deleted the feat/allow-numeric-timestamp-in-listMessages branch June 27, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants