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

Option to Sync Variant Prices on Multiple Channels #2651

Closed
dylviz opened this issue Jan 28, 2024 · 6 comments
Closed

Option to Sync Variant Prices on Multiple Channels #2651

dylviz opened this issue Jan 28, 2024 · 6 comments
Labels
next minor Candidate for the next minor release

Comments

@dylviz
Copy link
Contributor

dylviz commented Jan 28, 2024

Is your feature request related to a problem? Please describe.
This use case is most apparent in a MV scenario. This feature would allow the ability for channels to synchronize with other channels on variant price changes.

For example,
When I update the price for a variant on Channel 2 (as non-superadmin vendor), it doesn't keep Channel 1 in sync. When I query for products, it returns the price on Channel 1. With this feature Channe 2 and Channel 1 will have the option to stay in sync.

Describe the solution you'd like
@michaelbromley proposed a straight-forward solution here. Where channel synchronizations are optionally configured and we simply run a check on the variant update() for channels to sync with.

I agree with this approach but I've been putting some thought into how the UI will configure which channels to sync while also accounting for additional edge-cases that would arise. Some concerns include (depending on the UI implementation):

  • Vendor selects to sync with a channel that the variant is not assigned to
  • Adding too much configuration on the UI clutters the interface
  • In a MV scenario, only the superadmin should be in charge of configuring channel syncing

I believe that V1 of this feature should not include modifications to the AdminUI and should be as simple and straight-forward as possible. This would only exist to serve a very tiny number of developers that may face this issue. It would make sense to have v2 of this feature include a UI if there's requests for it.

The solution could be a simple configuration parameter that's passed in the vendure-config.ts file, syncProductsOnAssignedChannels: true/false. This solution would enable all products to by synced with only the channels the product is assigned. This V1 solution would not allow for more detailed configurations but could be improved in the future if needed.

Describe alternatives you've considered
On the store frontend, I've considered modifying the product-detail fetches to include a header channel token that aligns with the correct seller. This would pull the price info from the correct channel. However, this causes issues because in a MV scenario, the channel context of the Order should remain in the Default Channel.

Additional context
I've never contributed to this repo but I don't mind trying to get a PR for this feature

@michaelbromley michaelbromley added the next minor Candidate for the next minor release label Feb 2, 2024
@michaelbromley
Copy link
Member

I think this could be handled with a strategy-based approach. This would be the most flexible I think. Something like:

export interface ProductVariantPriceUpdateStrategy extends InjectableStrategy {

  /**
   * This would be invoked whenever a price is changed.
   */
  onPriceUpdate(
    ctx: RequestContect, 
    updatedPrice: ProductVariantPrice, 
    allPrices: ProductVariantPrice[]): Promise<ProductVariantPrice[]> {
    
    // For example, let's get all the other prices with the same currencyCode
    const pricesWithSameCurrency = allPrices.filter(p => p.currencyCode === updatedPrice.currencyCode);
   
    // we can now sync the prices
    pricesWithSameCurrency.forEach(p => p.price = updatedPrice.price);
 
    // any returned ProductVariantPrice entities will have the new price
    // persisted to the DB.
    return pricesWithSameCurrency;
  }

}

I quite like this approach because:

  • It requires zero changes to the data model
  • It can handle different sync directions. E.g. one-way sync only from default channel to child channels, or two-way sync as in this example.
  • It can be used for other use cases. For instance, let's say you wanted to have automatic currency conversion. This mechanism could be used to do a lookup on exchange rates and update other currencies accordingly.

@dylviz
Copy link
Contributor Author

dylviz commented Feb 9, 2024

I think this could be handled with a strategy-based approach. This would be the most flexible I think. Something like:

export interface ProductVariantPriceUpdateStrategy extends InjectableStrategy {

  /**
   * This would be invoked whenever a price is changed.
   */
  onPriceUpdate(
    ctx: RequestContect, 
    updatedPrice: ProductVariantPrice, 
    allPrices: ProductVariantPrice[]): Promise<ProductVariantPrice[]> {
    
    // For example, let's get all the other prices with the same currencyCode
    const pricesWithSameCurrency = allPrices.filter(p => p.currencyCode === updatedPrice.currencyCode);
   
    // we can now sync the prices
    pricesWithSameCurrency.forEach(p => p.price = updatedPrice.price);
 
    // any returned ProductVariantPrice entities will have the new price
    // persisted to the DB.
    return pricesWithSameCurrency;
  }

}

I quite like this approach because:

* It requires zero changes to the data model

* It can handle different sync directions. E.g. one-way sync only from default channel to child channels, or two-way sync as in this example.

* It can be used for other use cases. For instance, let's say you wanted to have automatic currency conversion. This mechanism could be used to do a lookup on exchange rates and update other currencies accordingly.

Very nice approach! This way will be much better

@NoahPerez
Copy link

NoahPerez commented Feb 22, 2024

@dylviz Where should I add ProductVariantPriceUpdateStrategy strategy-based approach or export it as a plugin?

or should I wait for Vendure 2.2 ?

@michaelbromley
Copy link
Member

@NoahPerez It's not available yet - it is going to be released in v2.2. You can try it now by installing the pre-release v2.2.0-next.3

@NoahPerez
Copy link

@michaelbromley okay thanks, I will install the pre-release "@vendure/core": "2.2.0"
Do you know when you will release V2.2 ? i saw some nice improvements

@michaelbromley
Copy link
Member

Within the next 2-3 weeks is very likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next minor Candidate for the next minor release
Projects
Status: Done
Development

No branches or pull requests

3 participants