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

feat: add wiki.autocomplete #42

Merged
merged 7 commits into from
Dec 20, 2022

Conversation

bigmistqke
Copy link
Contributor

fetches autocompletions from the OpenSearch-API

await wiki.autocomplete('test')
results in
["Test", "Testosterone", "Testicle", "Test cricket", "Test-driven development", "Testosterone (medication)", "Testicular cancer", "Tests of general relativity", "Test (wrestler)", "Test of English as a Foreign Language"]

@bigmistqke
Copy link
Contributor Author

autocomplete is a bit of a misnomer maybe, as it implies that the results are all extensions of the initial query, which is not the case.

image

my pr is referring to querying this list. the results often are extensions of the query, but does not have to be.

maybe 'suggestions' is a better name? 'opensearch' is a possibility too, as that refers to the action-type, but this is not very descriptive.

@dopecodez
Copy link
Owner

Thanks for the PR @bigmistqke let me check this tomorrow and add the necessary documentation and merge this.

source/index.ts Outdated Show resolved Hide resolved
@dopecodez
Copy link
Owner

The MR looks good just a couple of points @bigmistqke.

  1. Can we add an error unit test case?
  2. We also need to add the documentation for this method. If you're not comfortable with markup ill add the same once you confirm.

@bigmistqke
Copy link
Contributor Author

Cool!

  1. I think I already implemented a unit-test, index.test.ts line 72, but I am a test noob so correct me if I am wrong.
  2. documentation added

I just saw the distinction between wiki- and page-methods. I think for autocompletions it might make sense to only have it available in wiki, what do you think?

@dopecodez
Copy link
Owner

dopecodez commented Dec 16, 2022

I think having it directly on wiki makes perfect sense. No sense for open-search to be a page method.

  • I think I already implemented a unit-test, index.test.ts line 72, but I am a test noob so correct me if I am wrong.

Your unit test is fine but its a test for the success response. Ideally while unit testing a application, we should also add a test for the error response. Basically, a test for autocompletionsError.

You can check out

test('Throws search error if some error occurs', async () => {
for more idea on what im talking about.

Other than this, the MR looks ready to merge.
cc: @bigmistqke

docs/resultTypes.md Outdated Show resolved Hide resolved
test/index.test.ts Outdated Show resolved Hide resolved
@dopecodez
Copy link
Owner

MR looks all good now, ill merge this tomorrow and do a new release as i need my home computer to release this to npm.

@bigmistqke
Copy link
Contributor Author

lovely. thanks for the feedback!

@bigmistqke bigmistqke closed this Dec 19, 2022
@bigmistqke bigmistqke reopened this Dec 19, 2022
@bigmistqke
Copy link
Contributor Author

(oops accidentally closed pr)

@dopecodez dopecodez merged commit 9abc14f into dopecodez:master Dec 20, 2022
@dopecodez
Copy link
Owner

released on 1.2.0 🎆 . Thanks for the PR @bigmistqke 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants