-
Notifications
You must be signed in to change notification settings - Fork 6
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
PaginationCursor update from dwn-sdk-js
#15
Conversation
3c0aa66
to
359f05a
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #15 +/- ##
==========================================
+ Coverage 80.41% 80.64% +0.23%
==========================================
Files 10 10
Lines 1072 1080 +8
Branches 152 149 -3
==========================================
+ Hits 862 871 +9
+ Misses 210 209 -1 |
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.
Just the one question. Otherwise LGTM!
src/message-store-sql.ts
Outdated
// currently the sort property is explicitly either `dateCreated` | `messageTimestamp` | `datePublished` which are all strings | ||
// if it is not of type string we return an empty result set | ||
const cursorValue = pagination.cursor.value as string; |
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.
Just wanted to double check if this comment is still valid. I'm not immediately seeing an empty result set return if values
isn't a string here.
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.
Good catch! I had some logic here but decided against it for now.
It's not a scenario that would really happen, and I'd like to add some DWN specific error to handle that case.
Created an issue and updated the comment to reflect.
TBD54566975/dwn-sdk-js#664
PaginationCursor
is a new object indwn-sdk-js
.It is comprised of a
messageCid
and avalue
. Thevalue
is of the sort property that you would like to continue your pagination from. ThemessageCid
is used as a tiebreaker in the same way that query pagination already does.