Block helper functions for successful function calls #60
Conversation
🦋 Changeset detectedLatest commit: afe3407 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
gabehamilton
force-pushed
the
block-helpers-function-calls
branch
2 times, most recently
from
62c28a7 to
196a92e
Compare
gabehamilton
force-pushed
the
block-helpers-function-calls
branch
from
196a92e to
dc2a472
Compare
| .map((exeuctionOutcomeWithReceipt) => Action.fromReceiptView(exeuctionOutcomeWithReceipt.receipt)) | ||
| .filter((executionOutcomeWithReceipt) => | ||
| Action.isActionReceipt(executionOutcomeWithReceipt.receipt) && | ||
| (!successfulOnly || (executionOutcomeWithReceipt.executionOutcome.outcome.status as {SuccessValue: any})) |
There was a problem hiding this comment.
This assertion doesn't actually check if the action was only successful - it only checks if status exists, which it does for both successful/failure. as is just a type cast, and won't affect the outcome of this.
We need to check for the existence of status.SuccessValue, and perhaps status.SuccessReceiptId too, instead.
There was a problem hiding this comment.
@pkudinov hopefully your function is better than mine here.
| .filter((functionCallOperation) => functionCallOperation && (!method || functionCallOperation?.methodName === method)) | ||
| .map((functionCallOperation) => ({ | ||
| ...functionCallOperation, | ||
| args: this.base64decode(functionCallOperation.args), |
There was a problem hiding this comment.
| args: this.base64decode(functionCallOperation.args), | |
| decoded_args: this.base64decode(functionCallOperation.args) |
It may be better to expose this as decoded_args, as well as the raw args. base64decode will silently fail, so having both allows developers to use the raw args if decoded_args is undefined.
| console.error( | ||
| 'Error parsing base64 JSON - skipping data', | ||
| error | ||
| ); |
There was a problem hiding this comment.
I don't really like the idea of libraries logging things, because they can't be suppressed.
Let's just return undefined explicitly here, without the log.
| if ( | ||
| !functionCall || | ||
| !functionCall.args || | ||
| !functionCall.args.data || | ||
| !Object.keys(functionCall.args.data) || | ||
| !Object.keys(functionCall.args.data)[0] | ||
| ) { | ||
| console.error( | ||
| "Set operation did not have arg data in expected format" | ||
| ); | ||
| return; | ||
| } | ||
| const accountId = Object.keys(functionCall.args.data)[0]; |
There was a problem hiding this comment.
| if ( | |
| !functionCall || | |
| !functionCall.args || | |
| !functionCall.args.data || | |
| !Object.keys(functionCall.args.data) || | |
| !Object.keys(functionCall.args.data)[0] | |
| ) { | |
| console.error( | |
| "Set operation did not have arg data in expected format" | |
| ); | |
| return; | |
| } | |
| const accountId = Object.keys(functionCall.args.data)[0]; | |
| const accountId = Object.keys(functionCall.args.data ?? {})[0]; | |
| if (!accountId) { | |
| return false; | |
| } |
I believe this is the same?
Same comment around logging, can we please remove? Also we should be returning false here and below?
| * @param contract Defaults to 'social.near', pass in a different contract if needed | ||
| * @param method Defaults to 'set', pass in a different method if needed | ||
| */ | ||
| socialOperations(operation: string, contract: string = "social.near", method: string = "set") { |
There was a problem hiding this comment.
Can we add a return type here please?
| @@ -75,4 +75,24 @@ describe("Block", () => { | |||
| ); | |||
| expect(authorToPostId).toMatchSnapshot(); | |||
| }); | |||
|
|
|||
| it("parses successful function calls", async () => { | |||
There was a problem hiding this comment.
Can we add a test for actions(successfulOnly) too please?
| @@ -167,7 +169,7 @@ class DeleteAccount { | |||
| export type Operation = | |||
| | 'CreateAccount' | |||
| | DeployContract | |||
| | FunctionCall | |||
There was a problem hiding this comment.
Was this incorrect? Is this a breaking change?
There was a problem hiding this comment.
@pkudinov This file is where I ran into the Operations types not matching the data. I only changed FunctionCall but they should probably all be corrected.
Co-authored-by: Morgan McCauley <[email protected]>
|
Should we close now that #61 is merged? |
|
Yes, closing, #61 looks great. I do think more helper functions in either near-lake or queryapi would still be useful, extracting: SocialDB operations, NFTs, FTs, DAO proposals. Hiding all the error handling so that indexers can be just a few lines will make for great demos and onboarding. |
See Feature Request near/queryapi#366
This is more specific than other methods in the class. However, this specificity helps users. In future we could add additional "more specific" helper methods for fungible tokens and NFTs.
Should we add Wrapper types for all Action Operations? see near/queryapi#409