bleroy / Nwazet.Commerce

Idiomatic commerce module for Orchard CMS.
BSD 3-Clause "New" or "Revised" License
26 stars 21 forks source link

Tier pricing based on quantity #23

Closed bleroy closed 8 years ago

bleroy commented 10 years ago

Originally reported by: Josh Berry (Bitbucket: joshberry, GitHub: joshberry)


I have a requirement to provide different pricing levels based on the quantity of a Product ordered. I can definitely achieve this by setting up a series of Discounts for each Product but it's a clunky solution for the admin when most Products need tier pricing.

If I developed a more convenient solution to this problem, would that be something you'd be interested in pulling into the module?

Also, do you have any tips on the best way to develop the solution? I'm considering modifying the ProductPart to allow for multiple prices (although this could have a ripple effect into quite a few other areas) or maybe just reusing the Discount functionality and providing a convenient way to automatically create all the necessary Discounts from the Product editor. Haven't dug into it too much yet so I figured I'd see if you had any guidance.

Thanks for this great module and all your contributions to Orchard!


bleroy commented 10 years ago

Original comment by Bertrand Le Roy (Bitbucket: bleroy, GitHub: bleroy):


Thanks for the contribution.

bleroy commented 10 years ago

Original comment by Bertrand Le Roy (Bitbucket: bleroy, GitHub: bleroy):


That looks fine.

bleroy commented 10 years ago

Original comment by Josh Berry (Bitbucket: joshberry, GitHub: joshberry):


I think I figured this out. Since I'm using infoset storage the ProductSettingsStub needs to share the same ContentItem as the site settings. Here's my updated stub:

#!c#

public class SiteStub : ISite, IContent {
        public SiteStub(bool allowOverrides, bool defineDefaults, List<PriceTier> sitePriceTiers) {
            ContentItem = new ContentItem {
                VersionRecord = new ContentItemVersionRecord {
                    ContentItemRecord = new ContentItemRecord()
                },
                ContentType = "Site"
            };
            ContentItem.Weld(new InfosetPart());

            var settings = new ProductSettingsStub(true, false, new List<PriceTier>(), ContentItem);
            ContentItem.Weld(settings);
        }

        public ContentItem ContentItem { get; private set; }
        public string BaseUrl { get; set; }
        public string HomePage { get; set; }
        public int PageSize { get; set; }
        public string PageTitleSeparator { get { return ""; } }
        public ResourceDebugMode ResourceDebugMode { get; set; }
        public string SiteCulture { get; set; }
        public string SiteName { get; set; }
        public string SiteSalt { get; set; }
        public string SiteTimeZone { get; set; }
        public string SuperUser { get; set; }
        public int Id { get; set; }
    }
#!c#

    public class ProductSettingsStub : ProductSettingsPart {
        public ProductSettingsStub(bool allowOverrides, bool defineDefaults, List<PriceTier> sitePriceTiers, ContentItem contentItem) {
            ContentItem = contentItem;
            AllowProductOverrides = allowOverrides;
            DefineSiteDefaults = defineDefaults;
            PriceTiers = sitePriceTiers;
        }
    }

Seems to work correctly now. Am I going about this the right way?

bleroy commented 10 years ago

Original comment by Josh Berry (Bitbucket: joshberry, GitHub: joshberry):


I could use some help with my unit tests. I created a few site settings (ProductSettingsPart) that I retrieve in the PriceService like this:

#!c#

var productSettings = _wca.GetContext().CurrentSite.As<ProductSettingsPart>(); 

I need to stub these settings for the unit tests. I tried to follow your example by passing a SiteStub instance into your existing WorkContextAccessorStub constructor like so:

#!c#

private static readonly IWorkContextAccessor WorkContextAccessor =
            new WorkContextAccessorStub(new Dictionary<Type, object> {
                {typeof(IUser),
                    new UserStub(
                        "Joe",
                        "joe@orchardproject.net",
                        new[] {
                            "Moderator",
                            "Reseller",
                            "Customer"})},
                {typeof(ISite), new SiteStub(true, false, new List<PriceTier>()) }
            });

The properties on the SiteStub are working just fine but my ProductSettingsStub properties are not being retained by the time they are used in the PriceService. I suspect it has something to do with how I'm setting up the ProductSettingsStub. The other part stubs in the project (DiscountStub for example) are all backed by records but my ProductSettingsPart is not.

Here's the relevant stub classes:

#!c#

 public class SiteStub : ISite, IContent {
        public SiteStub(bool allowOverrides, bool defineDefaults, List<PriceTier> sitePriceTiers) {
            ContentItem = new ContentItem {
                VersionRecord = new ContentItemVersionRecord {
                    ContentItemRecord = new ContentItemRecord()
                },
                ContentType = "Site"
            };
            var settings = new ProductSettingsStub(true, false, new List<PriceTier>());

            PageSize = 7; // This value is retained just fine
            ContentItem.Weld(settings); // The values I set on this part's properties are lost

        }

        public ContentItem ContentItem { get; private set; }
        public string BaseUrl { get; set; }
        public string HomePage { get; set; }
        public int PageSize { get; set; }
        public string PageTitleSeparator { get { return ""; } }
        public ResourceDebugMode ResourceDebugMode { get; set; }
        public string SiteCulture { get; set; }
        public string SiteName { get; set; }
        public string SiteSalt { get; set; }
        public string SiteTimeZone { get; set; }
        public string SuperUser { get; set; }
        public int Id { get; set; }
    }
#!c#

    public class ProductSettingsStub : ProductSettingsPart {
        public ProductSettingsStub(bool allowOverrides, bool defineDefaults, List<PriceTier> sitePriceTiers) {
            ContentItem = new ContentItem {
                ContentType = "ProductSettings"
            };

            ContentItem.Weld(this);
            ContentItem.Weld(new InfosetPart());

            AllowProductOverrides = allowOverrides;
            DefineSiteDefaults = defineDefaults;
            PriceTiers = sitePriceTiers;
        }
    }

Can you provide any guidance on what I'm doing wrong? When I debug the tests my ProductSettingsStub constructor is indeed setting the property values but then they are lost by the time the object get's injected into the PriceService. I'd really appreciate any advice you have.

bleroy commented 10 years ago

Original comment by Josh Berry (Bitbucket: joshberry, GitHub: joshberry):


Never-mind, once I opened the test project and built it I was able to run the tests in NUnit just fine.

bleroy commented 10 years ago

Original comment by Josh Berry (Bitbucket: joshberry, GitHub: joshberry):


Thanks for the feedback. I've converted the one-to-many relationship to infoset storage, implemented store-wide defaults and provided the ability to specify an absolute number or percentage. I'd like to add a few unit tests to cover the price tier calculation logic but am not sure how to use the unit tests included in the module. Can you provide some guidance on how to properly run them with NUnit?

bleroy commented 10 years ago

Original comment by Bertrand Le Roy (Bitbucket: bleroy, GitHub: bleroy):


This implementation won't allow for the creation of store-wide price tiers, as I was mentioning in my previous message. What I was suggesting was actually very close to what you've been doing (I suggested having a part for overriding the store-wide discount). I agree now that discounts are not the best implementation, but I still insist that you need store-wide defaults. The "use tiered pricing" could become "override tiered pricing". I would do a few things differently however. I would not implement a many-to-one relationship, as that data is not going to be queried anywhere. Instead, use infoset storage with a serialized form of your tiered pricing data. Less complex, and better performing. Allow for each tier pricing to be specified either as an absolute number, or as a percentage. Add hint text explaining that tier quantities are the minimum quantity to trigger that tier. Cheers!

bleroy commented 10 years ago

Original comment by Josh Berry (Bitbucket: joshberry, GitHub: joshberry):


Thanks Bertrand. I experimented with this over the weekend and ran into a few challenges with the discount approach.

I prototyped another approach where I added a TierPriceRecord with a many-to-one relationship to the ProductPart. Then updated the ProductPart to allow users to optionally add multiple tier prices (quantity breakpoint, price). This seemed to work pretty well and allowed all the discount functionality to remain the same. The only change outside the ProductPart was a small update to the PriceService to grab the right tier price before calculating the lowest price.

Here's a screen capture of what the UI looks like:

I initially thought tier pricing would be better implemented using discounts too but after looking at it the second approach seems more straightforward. What do you think?

bleroy commented 10 years ago

Original comment by Bertrand Le Roy (Bitbucket: bleroy, GitHub: bleroy):


Yes, this is something I'm interested in. I think it should be implemented as a discount. You should be able to set-up defaults site-wide (for example, qty 10-49 gives 10%, 50-99 gives 20%, etc.), with a possibility to override per item. This means that you may have to implement a custom part in addition to the discount. Makes sense? Thanks for offering to contribute, and let me know here if you need any help with the design.