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.

Sign up for GitHub

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

Jump to bottom

feat(commerce): add location facets #4562

Merged
merged 5 commits into from
Oct 23, 2024
Merged

feat(commerce): add location facets #4562

merged 5 commits into from
Oct 23, 2024

Conversation

Spuffynism
Copy link
Contributor

@Spuffynism Spuffynism commented Oct 21, 2024 •
edited by louis-bompart
Loading

Add support for location facets. We went with the separate controller approach as we believe the location facets might evolve differently from the regular facets over time.

In follow-up PRs I:

COMHUB-247

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 21, 2024 •
edited
Loading

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 236.6 236.6 0
commerce 339.3 340.5 0.3
search 412 412 0
insight 400 400 0
recommendation 248.8 248.8 0
ssr 405.7 405.7 0
ssr-commerce 351.6 352.8 0.3

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
case-assist 0 6 0
insight 0 27 0
commerce 0 15 0
Detailed logs search : buildInteractiveResult
search : buildInteractiveInstantResult
search : buildInteractiveRecentResult
search : buildInteractiveCitation
search : buildGeneratedAnswer
recommendation : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support

@Spuffynism Spuffynism force-pushed the feature/comhub-247-support-location-facets branch from a4082d7 to 503ec58 Compare October 21, 2024 20:47
@Spuffynism Spuffynism marked this pull request as ready for review October 21, 2024 20:49
@Spuffynism Spuffynism requested review from a team as code owners October 21, 2024 20:49
@Spuffynism Spuffynism changed the title [WIP] feat(commerce): add location facets feat(commerce): add location facets Oct 21, 2024
@Spuffynism Spuffynism mentioned this pull request Oct 21, 2024
Merged
This was referenced Oct 21, 2024
Merged
Merged
@Spuffynism Spuffynism changed the title feat(commerce): add location facets feat(headless): add commerce location facets Oct 22, 2024
@Spuffynism Spuffynism changed the title feat(headless): add commerce location facets feat(commerce): add location facets Oct 22, 2024
alexprudhomme
alexprudhomme approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@alexprudhomme alexprudhomme left a comment

Choose a reason for hiding this comment

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

Looks good to me !

packages/headless/src/api/commerce/facet-search/base/base-facet-search-request.ts Outdated Show resolved Hide resolved
@Spuffynism Spuffynism mentioned this pull request Oct 22, 2024
Merged
pvanool
pvanool approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@pvanool pvanool left a comment

Choose a reason for hiding this comment

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

Thanks

agillesCoveo
agillesCoveo approved these changes Oct 22, 2024
View reviewed changes
Copy link

@agillesCoveo agillesCoveo left a comment

Choose a reason for hiding this comment

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

Looks good to me

BenKandelaars
BenKandelaars approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@BenKandelaars BenKandelaars left a comment

Choose a reason for hiding this comment

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

Looks good. We need to sync up on the exclude state in a follow up

@louis-bompart louis-bompart requested a review from a team as a code owner October 23, 2024 15:06
@louis-bompart louis-bompart force-pushed the feature/comhub-247-support-location-facets branch from a911833 to 82fc779 Compare October 23, 2024 15:35
@BenKandelaars BenKandelaars added this pull request to the merge queue Oct 23, 2024
Merged via the queue into master with commit 1dbd9a9 Oct 23, 2024
97 checks passed
@BenKandelaars BenKandelaars deleted the feature/comhub-247-support-location-facets branch October 23, 2024 16:41
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2024
@Spuffynism
feat(commerce): add location facets actions loader (#4569)
05cb497 
See: #4562

[COMHUB-247]

[COMHUB-247]:
https://coveord.atlassian.net/browse/COMHUB-247?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
github-merge-queue bot pushed a commit that referenced this pull request Oct 24, 2024
@Spuffynism
feat(commerce): add breadcrumbs and parameters support for location f…
47a9758 
…acets (#4571)

See #4562



https://github.com/user-attachments/assets/e3ace74e-f014-4866-92a4-b54c2547ba3b



[COMHUB-247]

[COMHUB-247]:
https://coveord.atlassian.net/browse/COMHUB-247?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
github-merge-queue bot pushed a commit that referenced this pull request Oct 24, 2024
@Spuffynism
feat(commerce): support setting latitude and longitude on context con…
7a33a0b 
…troller (#4572)

See #4562

[COMHUB-247]

[COMHUB-247]:
https://coveord.atlassian.net/browse/COMHUB-247?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
github-merge-queue bot pushed a commit that referenced this pull request Oct 31, 2024
@Spuffynism
feat(commerce): exclude unused location facets properties (#4574)
0b25980 
A few properties aren't supported by location facets. We remove:
- `preventAutoSelect`
- `freezeCurrentValues`
- `excluded` state, and related methods

We also address leftover comments from
#4562

See #4562

[COMHUB-247]

[COMHUB-247]:
https://coveord.atlassian.net/browse/COMHUB-247?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbeaudoincoveo fbeaudoincoveo fbeaudoincoveo left review comments

@BenKandelaars BenKandelaars BenKandelaars approved these changes

@agillesCoveo agillesCoveo agillesCoveo approved these changes

@pvanool pvanool pvanool approved these changes

@alexprudhomme alexprudhomme alexprudhomme approved these changes

@olamothe olamothe Awaiting requested review from olamothe olamothe was automatically assigned from coveo/search

@fpbrault fpbrault Awaiting requested review from fpbrault fpbrault was automatically assigned from coveo/search

@erocheleau erocheleau Awaiting requested review from erocheleau erocheleau was automatically assigned from coveo/salesforce-integration

@SimonMilord SimonMilord Awaiting requested review from SimonMilord SimonMilord was automatically assigned from coveo/salesforce-integration

@mmitiche mmitiche Awaiting requested review from mmitiche mmitiche was automatically assigned from coveo/salesforce-integration

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Spuffynism @BenKandelaars @fbeaudoincoveo @alexprudhomme @pvanool @agillesCoveo @louis-bompart