search

dopecodez / Wikipedia

Wikipedia for node and the browser
MIT License
82 stars 19 forks source link

feat: add wiki.autocomplete #42

Closed bigmistqke closed 1 year ago

bigmistqke commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

dopecodez commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/dopecodez/Wikipedia/blob/345ba33851b046ebe98a0d4723d5fb3a6043248a/test/index.test.ts#L43 for more idea on what im talking about.

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

dopecodez commented 1 year ago

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 commented 1 year ago

lovely. thanks for the feedback!

bigmistqke commented 1 year ago

(oops accidentally closed pr)

dopecodez commented 1 year ago

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