-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add search option on chat.fetchMessages by min/max timestamp #2250
base: main
Are you sure you want to change the base?
Add search option on chat.fetchMessages by min/max timestamp #2250
Conversation
…into add-filter-timestamp-fetch-message
Reopening #1499 with ESLint fixed and index.d.ts doc added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MessageSearchOptions type definition is not updated
* Loads chat messages, sorted from earliest to latest. | ||
* @param {Object} searchOptions Options for searching messages. Right now only limit is supported. | ||
* @param {Number} [searchOptions.limit] The amount of messages to return. If no limit is specified, the available messages will be returned. Note that the actual number of returned messages may be smaller if there aren't enough messages in the conversation. Set this to Infinity to load all messages. | ||
* @param {Number} [searchOptions.minTimestamp] If specified, filter message by minimum timestamp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably minimum can be rephrased to oldest and maximum earliest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, millis or unix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i personally prefer "minimum timestamp" but I can change it...
msgs = [...loadedMessages.filter(msgFilter), ...msgs]; | ||
} | ||
|
||
if (msgs.length > searchOptions.limit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extraneous chats with timestamp beyond range are handled with splice ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would this be a problem? can you elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the loop breaks before we insert the loadedMessages that was filtered, some messages might be within the range but not returned, maybe line 219 should go first . Unless I missed something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LucianoHanna where we standing with this?
I just spent the last 4 hours debugging an odd behavior, that limits fetching new messages from the device and forces the user to "click" the button to fetch new messages from device. What I can resume from my findings: There are 3 Layers of data:
Looking into what actually happens is as follows:
The thing is, this method is not whell documented, but I have figured out how to call it and force it to load more messages, and even, be able to change "how many messages" to fetch from device at once. The steps are (assuming
|
No description provided.