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

fix(specs): correct type for highlightResult and snippetResult #783

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Jul 4, 2022

🧭 What and Why

🎟 JIRA Ticket: APIC-565

Use correct types.

@millotp millotp requested a review from a team July 4, 2022 13:47
@millotp millotp self-assigned this Jul 4, 2022
@millotp millotp requested review from damcou and shortcuts and removed request for a team July 4, 2022 13:47
@netlify
Copy link

netlify bot commented Jul 4, 2022

Deploy Preview for api-clients-automation ready!

Name Link
🔨 Latest commit c368f48
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/62c30d179c33fb0008371b34
😎 Deploy Preview https://deploy-preview-783--api-clients-automation.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@algolia-bot
Copy link
Collaborator

algolia-bot commented Jul 4, 2022

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Good catch!

specs/search/common/schemas/Hit.yml Show resolved Hide resolved
specs/search/common/schemas/Hit.yml Outdated Show resolved Hide resolved

highlightResultMap:
type: object
description: Highlighted attributes.
Copy link
Member

Choose a reason for hiding this comment

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

The description is not ideal but we can add a similar one to the snippetResultMap too

Copy link
Member

@shortcuts shortcuts Jul 4, 2022

Choose a reason for hiding this comment

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

Suggested change
description: Highlighted attributes.
description: Highlighted section and words matched by a query.

From https://www.algolia.com/doc/guides/getting-started/how-algolia-works/in-depth/features/#highlighting-and-snippeting

@millotp
Copy link
Collaborator Author

millotp commented Jul 4, 2022

wait it's more complicated than I thought, it should be a oneOf 😭

@shortcuts
Copy link
Member

wait it's more complicated than I thought, it should be a oneOf 😭

For when it's matched you mean?

@millotp
Copy link
Collaborator Author

millotp commented Jul 4, 2022

the real type is:

type Inner = {
 value: string;
 matchLevel: (enum);
 matchedWords: string[];
 fullyHighlighted?: boolean
}
type HighlighResult = Record<String, Inner | Inner[]>;

which is completely different from the doc of course

@shortcuts
Copy link
Member

which is completely different from the doc of course

Ah.... good news is, it should not be a big deal on our side :D

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

great fixes!!!!

specs/search/common/schemas/Hit.yml Show resolved Hide resolved
@millotp millotp merged commit 23a72c3 into main Jul 4, 2022
@millotp millotp deleted the fix/highligh-result branch July 4, 2022 16:14
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.

3 participants