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

Migrate FAQ page into the Typesense backend #401

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

ekamuktia
Copy link
Collaborator

@ekamuktia ekamuktia commented Jul 25, 2021

Closes #377

Description

Using react-instantsearch-dom, creating custom components for SearchBox, Hits, and RefinementList

Current Tasks

  • Create custom components for SearchBox, Hits, and RefinementList
  • Update url query parameters based on search keyword/filter

@netlify
Copy link

netlify bot commented Jul 25, 2021

✔️ Deploy Preview for wargabantuwarga ready!

🔨 Explore the source changes: c719fb6

🔍 Inspect the deploy log: https://app.netlify.com/sites/wargabantuwarga/deploys/61112a64ec09e70008223f64

😎 Browse the preview: https://deploy-preview-401--wargabantuwarga.netlify.app

@ekamuktia ekamuktia marked this pull request as draft July 25, 2021 07:36
@famasya
Copy link

famasya commented Jul 25, 2021

Neat!

Should we highlight relevant words in the search results? I think it will provide clarity for the user, as it gives context why presented results are relevant.

image

@resir014
Copy link
Member

@ekamuktia After some discussions with @zainfathoni earlier, we decided to go forward with using Typesense, since the current search infrastructure gave us more problems than expected (e.g. #405). Therefore we need to migrate to Typesense sooner than expected.

Let's see if we can get this up and running asap as a proof-of-concept, so we can apply it to other places (e.g. province search).

@ekamuktia
Copy link
Collaborator Author

@resir014 @zainfathoni
The UI itself is customizeable, so it doesn't matter whether we use original typesense widget (https://www.algolia.com/doc/guides/building-search-ui/widgets/showcase/react/) or customize it to current UI
On my local development, the problem is solved but somehow on the deploy preview the handleFilterChange doesn't work.
Changed it to non modal filter because I thought it's probably the problem, but same thing still happens.
Any idea why?

Local:

bandicam.2021-07-25.19-46-51-524.mp4

Deploy Preview:

bandicam.2021-07-25.20-09-17-685.mp4

lib/typesense.ts Outdated
}) {
const searchAdapter = new TypesenseInstantSearchAdapter({
server: {
apiKey: "FByczfHEjsTCihgkkYlq2YbAgUKMoyVP", // Be sure to use the search-only-api-key
Copy link
Member

Choose a reason for hiding this comment

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

I think we should start storing these secrets on an environment variable. @zainfathoni what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Just let me or @resir014 know what environment variable to store, Mbak @ekamuktia, we can do it for you. 😁

@resir014
Copy link
Member

@zainfathoni @ekamuktia Created an environment variable in the Netlify dashboard called NEXT_PUBLIC_TYPESENSE_API_KEY. Let's put all secret values (e.g. apiKey) there.

@zainfathoni
Copy link
Member

What about the GitHub Actions build, @resir014? I think we should also put some env vars on the GitHub Secrets, shouldn't we?

@codecov
Copy link

codecov bot commented Jul 27, 2021

Codecov Report

Merging #401 (6f35e72) into main (d6bc5e1) will decrease coverage by 27.47%.
The diff coverage is 36.50%.

❗ Current head 6f35e72 differs from pull request most recent head c719fb6. Consider uploading reports for the commit c719fb6 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##             main     #401       +/-   ##
===========================================
- Coverage   79.69%   52.22%   -27.48%     
===========================================
  Files         110       90       -20     
  Lines        1266     1057      -209     
  Branches      418      321       -97     
===========================================
- Hits         1009      552      -457     
- Misses        251      498      +247     
- Partials        6        7        +1     
Impacted Files Coverage Δ
components/search/refinement-modal.tsx 0.00% <0.00%> (ø)
components/search/custom-instant-search.tsx 31.94% <31.94%> (ø)
components/faq-list.tsx 33.33% <33.33%> (ø)
components/search/custom-refinement-list.tsx 33.33% <33.33%> (ø)
components/search/custom-search-box.tsx 37.50% <37.50%> (ø)
components/search/custom-hits.tsx 100.00% <100.00%> (ø)
lib/typesense.ts 100.00% <100.00%> (ø)
pages/faq.tsx 100.00% <100.00%> (ø)
components/contact-details.tsx 0.00% <0.00%> (-100.00%) ⬇️
components/open-map-button.tsx 0.00% <0.00%> (-100.00%) ⬇️
... and 93 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6bc5e1...c719fb6. Read the comment docs.

@ekamuktia
Copy link
Collaborator Author

@zainfathoni @ekamuktia Created an environment variable in the Netlify dashboard called NEXT_PUBLIC_TYPESENSE_API_KEY. Let's put all secret values (e.g. apiKey) there.

For this, how can I use it on the code? Like this process.env.NEXT_PUBLIC_TYPESENSE_API_KEY?

@famasya @zainfathoni @resir014
Finally, I found the problem why the filter didn't work on deploy preview and fixed it.
Some things to note:

  • For FAQ, I couldn't find way to sort it based on the document order (tried objectID:asc and id:asc, but both failed)
  • For test case, since the data is from index, then it cannot use mock data. Test case needs to be updated.
  • Since I'm using new components, some functionality still not applied here (loading, empty state, preserve url parameter, etc)

@famasya
Copy link

famasya commented Jul 27, 2021

@zainfathoni @ekamuktia Created an environment variable in the Netlify dashboard called NEXT_PUBLIC_TYPESENSE_API_KEY. Let's put all secret values (e.g. apiKey) there.

For this, how can I use it on the code? Like this process.env.NEXT_PUBLIC_TYPESENSE_API_KEY?

@famasya @zainfathoni @resir014
Finally, I found the problem why the filter didn't work on deploy preview and fixed it.
Some things to note:

  • For FAQ, I couldn't find way to sort it based on the document order (tried objectID:asc and id:asc, but both failed)
  • For test case, since the data is from index, then it cannot use mock data. Test case needs to be updated.
  • Since I'm using new components, some functionality still not applied here (loading, empty state, preserve url parameter, etc)

For the sorting, it's intended since the schema I wrote doesn't store any numeric value to be sorted. Results are sorted by relevance by default.

Do you need any sortable field for this @ekamuktia ? If so, I'll put that on

@zainfathoni
Copy link
Member

zainfathoni commented Jul 27, 2021

@zainfathoni @ekamuktia Created an environment variable in the Netlify dashboard called NEXT_PUBLIC_TYPESENSE_API_KEY. Let's put all secret values (e.g. apiKey) there.

For this, how can I use it on the code? Like this process.env.NEXT_PUBLIC_TYPESENSE_API_KEY?

Yes, something like that.
And to make it work in the GitHub actions, you need to pass secrets.NEXT_PUBLIC_TYPESENSE_API_KEY to the env: property in the configuration.

I've updated the GitHub Secrets accordingly.

Screenshot 2021-07-28 at 07 31 20

@ekamuktia
Copy link
Collaborator Author

ekamuktia commented Jul 28, 2021

Tried to use process.env.NEXT_PUBLIC_TYPESENSE_API_KEY but the testing failed so I can't commit/push.
Tried adding setupFiles: ["dotenv/config"] in jest.config.js as mentioned here but it didn't work.

@ekamuktia
Copy link
Collaborator Author

For the sorting, it's intended since the schema I wrote doesn't store any numeric value to be sorted. Results are sorted by relevance by default.

Do you need any sortable field for this @ekamuktia ? If so, I'll put that on

@famasya
For FAQ, I may need it based on the row order on the sheet (just like current order)
And for Contact data, I need to order by verifikasi

@famasya famasya mentioned this pull request Jul 29, 2021
3 tasks
@baraeb92 baraeb92 added enhancement New feature or request help wanted Extra attention is needed high-priority Issue with high priority labels Aug 4, 2021
@baraeb92 baraeb92 added this to the Long-term Plan milestone Aug 4, 2021
… into feature/typesense-faq

� Conflicts:
�	__tests__/pages/faq.test.tsx
�	package.json
�	pages/faq.tsx
�	yarn.lock
@mazipan
Copy link
Member

mazipan commented Aug 8, 2021

Putting a reference: https://www.algolia.com/doc/guides/building-search-ui/going-further/server-side-rendering/react/

In case we want to support static filtering via path param

@mazipan
Copy link
Member

mazipan commented Aug 11, 2021

How can I checkout from this branch ya 😂?

@zainfathoni
Copy link
Member

How can I checkout from this branch ya 😂?

You can use GitHub VSCode Extension, Mas @mazipan.
There's an option to checkout to the Pull Request, and when you make commits on top of it, you can push it into the remote branch, even if the branch is on a forked repository. 😎

Screenshot 2021-08-13 at 06 48 39

Comment on lines +3 to +4
// import { faqBuilder } from "~/lib/__mocks__/builders/faq";
// import faqs from "~/lib/data/faqs";

Choose a reason for hiding this comment

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

Commented code should not be pushed.

import faqs from "~/lib/data/faqs";
import FaqPage from "../../pages/faq";

// import { perBuild } from "@jackfranklin/test-data-bot";

Choose a reason for hiding this comment

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

Commented code should not be pushed.

const filtersParam: { [key: string]: string[] } = {};
const filterFields = filterSettings?.map((cur) => cur.field) ?? [];
Object.entries(queryParams).forEach(([key, value]) => {
if (key == "q") {

Choose a reason for hiding this comment

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

Please use strict equality operator ===

if (keywordsParam) {
searchParams.query = keywordsParam;
}
} else if (key == "sort") {

Choose a reason for hiding this comment

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

Please use strict equality operator ===

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed high-priority Issue with high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate FAQ page into the Typesense backend
7 participants