Skip to content
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

[explorer + ts-sdk] Search input string type checks #2028

Merged
merged 19 commits into from
May 19, 2022
Merged

Conversation

stella3d
Copy link
Contributor

This adds some basic string type checks to the TypeScript SDK, for object ID, address, and transaction digest.

We use these new string type checks in the explorer in order to:

@stella3d stella3d requested review from Jibz1, apburnie and 666lcz May 18, 2022 01:36
@stella3d stella3d self-assigned this May 18, 2022
@stella3d
Copy link
Contributor Author

this should close #2022

Copy link
Contributor

@666lcz 666lcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving the search! In addition to the inline comments, could you also add a unit test for the added SDK util functions?

Copy link
Contributor

@apburnie apburnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see improved error handling. We need the 2 error pages to be rationalised to 1. Could we also have some tests set up to ensure the checks on search terms behave as intended?

Stella Cannefax and others added 5 commits May 18, 2022 12:03
Co-authored-by: Chris Li <76067158+666lcz@users.noreply.github.com>
Co-authored-by: Chris Li <76067158+666lcz@users.noreply.github.com>
Co-authored-by: Chris Li <76067158+666lcz@users.noreply.github.com>
@stella3d
Copy link
Contributor Author

i'm unifying the error pages right now, let's not merge yet.

@stella3d
Copy link
Contributor Author

screenshots from the build i'm about to push

search transactions category with a bad query:
image

search objects category with a bad query:
image

search addresses category with a bad query:
image

search all with a bad query:
image

@@ -21,6 +26,24 @@ function getPlaceholderText(category: SearchCategory) {
}
}

function isInputValid(category: SearchCategory, input: string): boolean {
if (overrideTypeChecks) return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to get rid of this but it requires overhauling the test data, i think this works fine for now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - we can revisit this when we move from static JSON to that delivered by the API.

Copy link
Contributor

@apburnie apburnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent - this is ready for merge. It's also great to see Search Disambiguation between transactions and objects/addresses implemented at the search query level.

@apburnie apburnie merged commit 3d8ace1 into main May 19, 2022
@apburnie apburnie deleted the ts-str-checks branch May 19, 2022 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants