-
Notifications
You must be signed in to change notification settings - Fork 313
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
Extend CallBuilder to support join
.
#436
Conversation
@@ -258,9 +269,11 @@ export class CallBuilder< | |||
// If the key with the link name already exists, create a copy | |||
if (typeof json[key] !== "undefined") { | |||
json[`${key}_attr`] = json[key]; | |||
const record = this._parseRecord(json[key]); | |||
json[key] = async () => record; |
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.
This keeps the same API regardless if the record is coming from the payload or from links. That is, if you want to get the transaction, you'll call record.transaction()
which will return a promise.
* @param {"transactions"} join Records to be included in the response. | ||
* @returns {object} current CallBuilder instance. | ||
*/ | ||
public join(include: "transactions"): this { |
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.
This is a string literal, it will force this value to be transactions
-- Once join supports more types and we know the query param shape, we'll be able to add more validations into this.
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.
Does this error at runtime? I think it should throw a visible error.
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.
@morleyzhi yeah it does, if there is an error then it will bubble up from the server and handled by our error handling. Also if you are in TS --then you will be allowed to pass only "transactions" which is the only valid value for now.
654147d
to
12417dd
Compare
Use data from join Whitelist resources which can be loaded via join. Use string literal for join param. Don't use async/await in test 😭
12417dd
to
3822b0f
Compare
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.
Looks good as long as the join command errors if an invalid join param is provided.
* @param {"transactions"} join Records to be included in the response. | ||
* @returns {object} current CallBuilder instance. | ||
*/ | ||
public join(include: "transactions"): this { |
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.
Does this error at runtime? I think it should throw a visible error.
This PR adds support for the
join
query parameters added on Horizon v0.19.0, which allows side-loading of related resources.For now
join
only supportstransactions
.Fixes #433