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

Search indices are not currency aware #2280

Open
asonnleitner opened this issue Jul 12, 2023 · 11 comments
Open

Search indices are not currency aware #2280

asonnleitner opened this issue Jul 12, 2023 · 11 comments
Assignees
Labels
P3: minor Non-critical, no workarounds exist type: bug 🐛 Something isn't working
Milestone

Comments

@asonnleitner
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Following the new multi-currency support, the search strategies do not recognize the currency of the request context since they are unaware of it. I observed that when querying for a specific product, everything functions as expected. However, the search strategies might need an update to enable a currency-aware search.

Describe the solution you'd like
If the search strategies miss this functionality, they might need to be updated to facilitate a currency-aware search, including the Elasticsearch plugin. I'm willing to offer my help with this necessary changes if it's clear what needs to be done.

Describe alternatives you've considered
No alternatives have been considered.

Additional context
N/A

I have noticed several other currency-related issues.

  1. When assigning a product from the default channel, where availableCurrencyCodes = ["USD", "EUR"], and I wish to assign it to a channel where only EUR is available, the predefined value appears as USD. After assigning the product to the channel, the variant price is displayed in USD, even though the assigned channel does not support USD.

Here is a quick video to showcase that: https://drive.google.com/file/d/1vVymbdNXyYx5THdr5RsH_tVd9_ir9sj1/view?usp=sharing

@asonnleitner asonnleitner changed the title Update to Search Strategies for Multi-Currency Support Search indices are not currency aware Jul 14, 2023
@kosarlukascz
Copy link

What abou this issue? @michaelbromley

@michaelbromley
Copy link
Member

michaelbromley commented Sep 7, 2023

@kosarlukascz Hi, I've not had a chance to look deeply into this yet, but in principle yes this sounds like something that was overlooked with the changes to currency handling that was introduced in v2.0.

@asonnleitner Currently the SearchIndexItem has a composite primary key on the productVariantId, languageCode, and channelId fields. So one solution is to add a new currencyCode field and add that to the primary key.

We should consider performance/resource implications though, because if a shop has many currencies, then we will be duplicating lots of data. So perhaps a different storage approach could be explored, e.g. storing prices as a JSON column with prices in each currency encoded in a JSON object or an array.

I'm open to ideas here.

@seminarian
Copy link
Contributor

seminarian commented Oct 17, 2023

@michaelbromley In context of an international multi-vendor marketplace I think, when leveraging the search query, having the ability to filter on a specific currency is a valuable feature.

E.g. Some sellers only add Products in $ and some other sellers in $ and EUR, and so on.
The storefront (linked to domain/region) could then present all the ProductVariants defined in those currencies by filtering on currencyCode.

I don't think a JSON column would be ideal in such a case?
I'm not that knowledgeable about the specifics of ElastichSearch so feel free to correct me if there could be done a high-performance filtering on currencyCode with a datastructure like that.

If this indeed would not be possible:
I like the idea of having a composite primary key on productVariantId, languageCode, channelId and currencyCode.
Let's assume that performance/resource implications only really matter when using ElasticSearch/TypeSense for storing the SearchIndex.
In those cases, What if we went for a SearchIndex per currencyCode?
Optionally, this could be configurable through a plugin option.

@casperiv0
Copy link
Contributor

casperiv0 commented Dec 21, 2023

@michaelbromley, sorry for the ping here. However, is there a current workaround for this since we're experiencing this issue in production environments. This is more a bug than a feature, no?

PS: I should be able to help out on the primary key solution if that's the current way to go!

@michaelbromley
Copy link
Member

Hi,

There's no work-around right now other than implementing a customized version of the search plugin, the source of which is here: https://github.com/vendure-ecommerce/vendure/tree/fb0ea1326e5ee6cd240db146415dffc13af9ab9b/packages/core/src/plugin/default-search-plugin

I'd like to see a solution to this which is backwards-compatible. I.e. it can be applied as an opt-in so that it is not a breaking change.

This could be achieved by adding an opt-in flag to the DefaultSearchPlugin's init options, and then using the EntityMetadataModifier mechanism to dynamically modify the entity metadata for the SearchIndexItem entity.

/bounty $100

@Rutik7066
Copy link

Hey @michaelbromley i am excited to work on it. Could you please assign this to me.

@michaelbromley
Copy link
Member

sure @Rutik7066. I think you still need to add the "attempt" comment to get your attempt into Algora's system.

This is a bit of a tricky one to get right in terms of preserving backward-compatibility. If you like you can open a draft PR and let me know if you want advice on any specific parts.

@Rutik7066
Copy link

Rutik7066 commented Jan 19, 2024

Sure @michaelbromley i'll open draft pr.
/attempt #2280

Algora profile Completed bounties Tech Active attempts Options
@Rutik7066 4 bounties from 3 projects
Go, Rust,
TypeScript & more
Cancel attempt

@algora-pbc algora-pbc bot removed the 💎 Bounty label Jan 26, 2024
@dlhck dlhck added the type: bug 🐛 Something isn't working label Sep 24, 2024
@dlhck dlhck moved this from 📅 Planned to 📦 Backlog in Vendure OS Roadmap Sep 24, 2024
@dlhck dlhck added the P3: minor Non-critical, no workarounds exist label Sep 24, 2024
@dlhck dlhck added this to the v3.4.0 milestone Sep 27, 2024
@casperiv0
Copy link
Contributor

What's the status on this issue 😅? We're planning on launching a new campaign which is live in GB and IRL, with this issue our prices will not show correctly in IRL (since GBP will be used instead of EUR).

@casperiv0
Copy link
Contributor

casperiv0 commented Dec 10, 2024

Working on a solution, only for the default search plugin which adds currencyCode to the SearchIndexItem so we can filter on that key, also using the options like Michael mentioned before

Will open a PR a bit later, it's def. not a robust solution, but will work for us for now :)

@casperiv0
Copy link
Contributor

The PR is open here :) - #3268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3: minor Non-critical, no workarounds exist type: bug 🐛 Something isn't working
Projects
Status: 📅 Planned
Development

No branches or pull requests

7 participants