chore(sdk): Refactor the SDK to avoid passing the full context to the DataMappers #738
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonesho
reviewed
Sep 17, 2024
| this.portal = new PortalDataMapper(conf, this.web3Client, this, this.walletClient); | ||
| this.utils = new UtilsDataMapper(conf, this.web3Client, this, this.walletClient); | ||
| this.schema = new SchemaDataMapper(conf, this.web3Client, this.walletClient); | ||
| const findOneSchemaById = this.schema.findOneById.bind(this.schema); |
There was a problem hiding this comment.
Not sure if it's correct to pass this.schema as the function parameter to findOneById.bind(), iiuc, the function parameter should be an string given that async findOneById(id: string), not sure why here it was passed with the SchemaDataMapper object, also bind would not do any type check, so better avoid to use it if possible, see here
jonesho
reviewed
Sep 17, 2024
| const matchingSchema = await this.veraxSdk.schema.findOneById(attestationPayload.schemaId); | ||
| const matchingSchema = this.findOneSchemaById | ||
| ? await this.findOneSchemaById(attestationPayload.schemaId) | ||
| : undefined; |
There was a problem hiding this comment.
More percise, !matchingSchema return true can now be either "No matching Schema" or the given findOneSchemaById is undefined, and if that's the case, I think the msg inthrow new Error("No matching Schema") need to change accordingly
alainncls
force-pushed
the
chore/refactor-the-sdk-to-avoid-passing-the-full-context-to-the-datamappers
branch
from
c7d77a9
to
0250e98
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Related ticket
Fixes #728
Type of change
Check list