I have recently started using Spree for a new version of my company's web
site and order system. We have some pretty specific needs for quantity
based pricing that none of the existing volume pricing extensions were a
perfect fit for.

I've been working on my own extension to get exactly what we need and to
help me learn how to work with this platform.

I noticed some choices on order detail views that required additional
Deface overrides that could possibly be avoided with some pretty minor
changes to the core.

The company I work for does mostly bulk orders. The volume discount
price/quantity breakouts are defined several suppliers that we work with.
From supplier to supplier the breakout can vary in terms of what the price
vs quantity values are. One thing each supplier has in common is the
discounts start at unit 1. This is something we need to pass on to the
customer.

At a minimum the other volume pricing extensions do the following things:
Decorate Spree::LineItem and redefine the copy_price method
Decorate Spree::Variant to provide a volume_price method
Add a new class that defines the volume discount system which gets
associated with Spree::Variant

I followed this pattern and it got me most of the way. When I was nearly
done implementing the logic our extension needed I realized that I kept
seeing the "list price" of the master variant when I expected to see the
volume price for a particular quantity. This happened because the views
kept using the variant record to get the per-unit price instead of the
actual line item. It usually does this by calling
item.variant.display_amount. In each of these places the same number could
be accessed by calling item.single_money.

The views in question are:
spree/admin/orders/_line_item.html.erb
spree/admin/shared/_order_details.html.erb
spree/order_mailer/confirm_email.text.erb
spree/order_mailer/cancel_email.text.erb
spree/shared/_order_details.html.erb

This change would let developers manipulate the per-unit price using
Spree::LineItem without requiring the use of overrides on those views.

Since this is obviously not a bug in spree_core I figured it would make the
most sense to post about it in the group. Being new to Spree I cannot rule
out that these design decisions happend for a reason I am unaware of and
was hoping if anyone knew about such a situation that they chime in.

On a side note I am pretty (sure but not positive)
that https://github.com/spree/spree_volume_pricing is going to have invalid
per unit prices on the checkout summary, order e-mails and in front/back
end order detail pages. It is not overriding any of the views mentioned
above and does not seem to make any changes to the price field of the
Spree::Variant class. Updating the core views to use the price from
Spree::LineItem would solve this problem for that extension. The view
changes do not allow a volume discount on quantities of 1 item, however.


--
You received this message because you are subscribed to the Google Groups "Spree" group.
To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Nate Lowrie at Mar 28, 2013 at 1:40 pm
    Zach,

    I would say that your issues with the seeing the current price of the
    variant instead of the LineItem price in the view ARE a HUGE bug as was
    mentioned a few weeks ago. My thoughts on the matter are:


    1. Orders should only ever reference the price in the order line
    entries. If you are looking back in the admin interface at orders that were
    placed when items were a different price than current, it's going to make
    people very confused by displaying the current price because the totals
    won't jive.
    2. If the order has not been completed and the price on the variant
    differs from the line item price, we should have a callback to update the
    price.
    3. In the admin section, we should have the ability to update the price
    of the order line. I often cut a deal with customers on large purchases and
    would like the ability to change prices.
    4. I think Spree needs to rethink the way it looks at pricing
    altogether.
    1. Simple pricing is fine for some stores, but I feel like spree
    needs to support pricing rules out of the box to let a user define pricing
    rules to support the following use cases:
    1. Sale prices for things based on a percentage or flat value off
    of base price. Has a start and end date.
    2. Volume pricing which defines a percentage or flat value off of
    base price when a specific quantity range is ordered.
    2. Pricing rules would function like promotions. Find the prices
    returned by all active, eligible rules and use the lowest price.
    3. By going to a simple price rule model, it enables extensions to
    allow advanced pricing features by adding new pricing rule templates
    instead of hacking there own system that might interfere with other pricing
    options. The pricing rules architecture could be very similar to way
    shipping calculators are currently done.
    4. If nothing else, implement the ability to run a sale without
    having to change the base prices on the start/end dates. Enough users have
    asked for this I am surprised it's not already there.

    Can one of the core team members post about why the current variant price
    is always displayed in the view instead of the order line price?

    Regards,

    Nate
    On Wednesday, March 27, 2013 1:59:39 PM UTC-4, Zach Karpinski wrote:

    I have recently started using Spree for a new version of my company's web
    site and order system. We have some pretty specific needs for quantity
    based pricing that none of the existing volume pricing extensions were a
    perfect fit for.

    I've been working on my own extension to get exactly what we need and to
    help me learn how to work with this platform.

    I noticed some choices on order detail views that required additional
    Deface overrides that could possibly be avoided with some pretty minor
    changes to the core.

    The company I work for does mostly bulk orders. The volume discount
    price/quantity breakouts are defined several suppliers that we work with.
    From supplier to supplier the breakout can vary in terms of what the price
    vs quantity values are. One thing each supplier has in common is the
    discounts start at unit 1. This is something we need to pass on to the
    customer.

    At a minimum the other volume pricing extensions do the following things:
    Decorate Spree::LineItem and redefine the copy_price method
    Decorate Spree::Variant to provide a volume_price method
    Add a new class that defines the volume discount system which gets
    associated with Spree::Variant

    I followed this pattern and it got me most of the way. When I was nearly
    done implementing the logic our extension needed I realized that I kept
    seeing the "list price" of the master variant when I expected to see the
    volume price for a particular quantity. This happened because the views
    kept using the variant record to get the per-unit price instead of the
    actual line item. It usually does this by calling
    item.variant.display_amount. In each of these places the same number could
    be accessed by calling item.single_money.

    The views in question are:
    spree/admin/orders/_line_item.html.erb
    spree/admin/shared/_order_details.html.erb
    spree/order_mailer/confirm_email.text.erb
    spree/order_mailer/cancel_email.text.erb
    spree/shared/_order_details.html.erb

    This change would let developers manipulate the per-unit price using
    Spree::LineItem without requiring the use of overrides on those views.

    Since this is obviously not a bug in spree_core I figured it would make
    the most sense to post about it in the group. Being new to Spree I cannot
    rule out that these design decisions happend for a reason I am unaware of
    and was hoping if anyone knew about such a situation that they chime in.

    On a side note I am pretty (sure but not positive) that
    https://github.com/spree/spree_volume_pricing is going to have invalid
    per unit prices on the checkout summary, order e-mails and in front/back
    end order detail pages. It is not overriding any of the views mentioned
    above and does not seem to make any changes to the price field of the
    Spree::Variant class. Updating the core views to use the price from
    Spree::LineItem would solve this problem for that extension. The view
    changes do not allow a volume discount on quantities of 1 item, however.

    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Zach Karpinski at Mar 28, 2013 at 4:00 pm
    So I did a bit of digging around in the history for
    app/views/spree/shared/_order_details.html.erb and it looks like the change
    to use Spree::Variant instead of Spree::LineItem happened when they added
    currency support into the models for the 1.3 branch.

    Here is the commit where things changed (pretty recently)
    https://github.com/spree/spree/blob/58ec5cd5124e66c60811a075b290248962159f9e/core/app/views/spree/admin/shared/_order_details.html.erb

    I haven't looked into the history for the other files in question but this
    certainly is when the 1.3 branch would start showing "confusing"
    information on checkout pages and admin pages when folks also used
    spree_volume_pricing.

    The suggested change of using Spree:LineItem.single_money displays currency
    format too so it seems in line with the goal of the commit in question.

    -Zach
    On Thursday, March 28, 2013 8:40:21 AM UTC-5, Nate Lowrie wrote:

    Zach,

    I would say that your issues with the seeing the current price of the
    variant instead of the LineItem price in the view ARE a HUGE bug as was
    mentioned a few weeks ago. My thoughts on the matter are:


    1. Orders should only ever reference the price in the order line
    entries. If you are looking back in the admin interface at orders that were
    placed when items were a different price than current, it's going to make
    people very confused by displaying the current price because the totals
    won't jive.
    2. If the order has not been completed and the price on the variant
    differs from the line item price, we should have a callback to update the
    price.
    3. In the admin section, we should have the ability to update the
    price of the order line. I often cut a deal with customers on large
    purchases and would like the ability to change prices.
    4. I think Spree needs to rethink the way it looks at pricing
    altogether.
    1. Simple pricing is fine for some stores, but I feel like spree
    needs to support pricing rules out of the box to let a user define pricing
    rules to support the following use cases:
    1. Sale prices for things based on a percentage or flat value
    off of base price. Has a start and end date.
    2. Volume pricing which defines a percentage or flat value off
    of base price when a specific quantity range is ordered.
    2. Pricing rules would function like promotions. Find the prices
    returned by all active, eligible rules and use the lowest price.
    3. By going to a simple price rule model, it enables extensions to
    allow advanced pricing features by adding new pricing rule templates
    instead of hacking there own system that might interfere with other pricing
    options. The pricing rules architecture could be very similar to way
    shipping calculators are currently done.
    4. If nothing else, implement the ability to run a sale without
    having to change the base prices on the start/end dates. Enough users have
    asked for this I am surprised it's not already there.

    Can one of the core team members post about why the current variant price
    is always displayed in the view instead of the order line price?

    Regards,

    Nate
    On Wednesday, March 27, 2013 1:59:39 PM UTC-4, Zach Karpinski wrote:

    I have recently started using Spree for a new version of my company's web
    site and order system. We have some pretty specific needs for quantity
    based pricing that none of the existing volume pricing extensions were a
    perfect fit for.

    I've been working on my own extension to get exactly what we need and to
    help me learn how to work with this platform.

    I noticed some choices on order detail views that required additional
    Deface overrides that could possibly be avoided with some pretty minor
    changes to the core.

    The company I work for does mostly bulk orders. The volume discount
    price/quantity breakouts are defined several suppliers that we work with.
    From supplier to supplier the breakout can vary in terms of what the price
    vs quantity values are. One thing each supplier has in common is the
    discounts start at unit 1. This is something we need to pass on to the
    customer.

    At a minimum the other volume pricing extensions do the following things:
    Decorate Spree::LineItem and redefine the copy_price method
    Decorate Spree::Variant to provide a volume_price method
    Add a new class that defines the volume discount system which gets
    associated with Spree::Variant

    I followed this pattern and it got me most of the way. When I was nearly
    done implementing the logic our extension needed I realized that I kept
    seeing the "list price" of the master variant when I expected to see the
    volume price for a particular quantity. This happened because the views
    kept using the variant record to get the per-unit price instead of the
    actual line item. It usually does this by calling
    item.variant.display_amount. In each of these places the same number could
    be accessed by calling item.single_money.

    The views in question are:
    spree/admin/orders/_line_item.html.erb
    spree/admin/shared/_order_details.html.erb
    spree/order_mailer/confirm_email.text.erb
    spree/order_mailer/cancel_email.text.erb
    spree/shared/_order_details.html.erb

    This change would let developers manipulate the per-unit price using
    Spree::LineItem without requiring the use of overrides on those views.

    Since this is obviously not a bug in spree_core I figured it would make
    the most sense to post about it in the group. Being new to Spree I cannot
    rule out that these design decisions happend for a reason I am unaware of
    and was hoping if anyone knew about such a situation that they chime in.

    On a side note I am pretty (sure but not positive) that
    https://github.com/spree/spree_volume_pricing is going to have invalid
    per unit prices on the checkout summary, order e-mails and in front/back
    end order detail pages. It is not overriding any of the views mentioned
    above and does not seem to make any changes to the price field of the
    Spree::Variant class. Updating the core views to use the price from
    Spree::LineItem would solve this problem for that extension. The view
    changes do not allow a volume discount on quantities of 1 item, however.

    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Michael Sevestre at Mar 28, 2013 at 5:55 pm
    Zach: Good find!

    Maybe this recent commit simply introduced a bug?
    Is this line
    <td class="price"><%= item.variant.display_amount %></td>

    the only line that needs to be changed?

    I have to agree with Nate that this behavior is weird. Orders should
    definitely only reference price in order lines entries

    Cheers,
    Michael



    On Thu, Mar 28, 2013 at 5:00 PM, Zach Karpinski wrote:

    So I did a bit of digging around in the history for
    app/views/spree/shared/_order_details.html.erb and it looks like the change
    to use Spree::Variant instead of Spree::LineItem happened when they added
    currency support into the models for the 1.3 branch.

    Here is the commit where things changed (pretty recently)

    https://github.com/spree/spree/blob/58ec5cd5124e66c60811a075b290248962159f9e/core/app/views/spree/admin/shared/_order_details.html.erb

    I haven't looked into the history for the other files in question but this
    certainly is when the 1.3 branch would start showing "confusing"
    information on checkout pages and admin pages when folks also used
    spree_volume_pricing.

    The suggested change of using Spree:LineItem.single_money displays
    currency format too so it seems in line with the goal of the commit in
    question.

    -Zach
    On Thursday, March 28, 2013 8:40:21 AM UTC-5, Nate Lowrie wrote:

    Zach,

    I would say that your issues with the seeing the current price of the
    variant instead of the LineItem price in the view ARE a HUGE bug as was
    mentioned a few weeks ago. My thoughts on the matter are:


    1. Orders should only ever reference the price in the order line
    entries. If you are looking back in the admin interface at orders that were
    placed when items were a different price than current, it's going to make
    people very confused by displaying the current price because the totals
    won't jive.
    2. If the order has not been completed and the price on the variant
    differs from the line item price, we should have a callback to update the
    price.
    3. In the admin section, we should have the ability to update the
    price of the order line. I often cut a deal with customers on large
    purchases and would like the ability to change prices.
    4. I think Spree needs to rethink the way it looks at pricing
    altogether.
    1. Simple pricing is fine for some stores, but I feel like spree
    needs to support pricing rules out of the box to let a user define pricing
    rules to support the following use cases:
    1. Sale prices for things based on a percentage or flat value
    off of base price. Has a start and end date.
    2. Volume pricing which defines a percentage or flat value off
    of base price when a specific quantity range is ordered.
    2. Pricing rules would function like promotions. Find the prices
    returned by all active, eligible rules and use the lowest price.
    3. By going to a simple price rule model, it enables extensions to
    allow advanced pricing features by adding new pricing rule templates
    instead of hacking there own system that might interfere with other pricing
    options. The pricing rules architecture could be very similar to way
    shipping calculators are currently done.
    4. If nothing else, implement the ability to run a sale without
    having to change the base prices on the start/end dates. Enough users have
    asked for this I am surprised it's not already there.

    Can one of the core team members post about why the current variant price
    is always displayed in the view instead of the order line price?

    Regards,

    Nate
    On Wednesday, March 27, 2013 1:59:39 PM UTC-4, Zach Karpinski wrote:

    I have recently started using Spree for a new version of my company's
    web site and order system. We have some pretty specific needs for quantity
    based pricing that none of the existing volume pricing extensions were a
    perfect fit for.

    I've been working on my own extension to get exactly what we need and to
    help me learn how to work with this platform.

    I noticed some choices on order detail views that required additional
    Deface overrides that could possibly be avoided with some pretty minor
    changes to the core.

    The company I work for does mostly bulk orders. The volume discount
    price/quantity breakouts are defined several suppliers that we work with.
    From supplier to supplier the breakout can vary in terms of what the price
    vs quantity values are. One thing each supplier has in common is the
    discounts start at unit 1. This is something we need to pass on to the
    customer.

    At a minimum the other volume pricing extensions do the following things:
    Decorate Spree::LineItem and redefine the copy_price method
    Decorate Spree::Variant to provide a volume_price method
    Add a new class that defines the volume discount system which gets
    associated with Spree::Variant

    I followed this pattern and it got me most of the way. When I was
    nearly done implementing the logic our extension needed I realized that I
    kept seeing the "list price" of the master variant when I expected to see
    the volume price for a particular quantity. This happened because the
    views kept using the variant record to get the per-unit price instead of
    the actual line item. It usually does this by calling
    item.variant.display_amount. In each of these places the same number could
    be accessed by calling item.single_money.

    The views in question are:
    spree/admin/orders/_line_item.**html.erb
    spree/admin/shared/_order_**details.html.erb
    spree/order_mailer/confirm_**email.text.erb
    spree/order_mailer/cancel_**email.text.erb
    spree/shared/_order_details.**html.erb

    This change would let developers manipulate the per-unit price using
    Spree::LineItem without requiring the use of overrides on those views.

    Since this is obviously not a bug in spree_core I figured it would make
    the most sense to post about it in the group. Being new to Spree I cannot
    rule out that these design decisions happend for a reason I am unaware of
    and was hoping if anyone knew about such a situation that they chime in.

    On a side note I am pretty (sure but not positive) that
    https://github.com/spree/**spree_volume_pricing<https://github.com/spree/spree_volume_pricing>is going to have invalid per unit prices on the checkout summary, order
    e-mails and in front/back end order detail pages. It is not overriding any
    of the views mentioned above and does not seem to make any changes to the
    price field of the Spree::Variant class. Updating the core views to use
    the price from Spree::LineItem would solve this problem for that extension.
    The view changes do not allow a volume discount on quantities of 1 item,
    however.


    --
    You received this message because you are subscribed to the Google Groups
    "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Nate Lowrie at Mar 28, 2013 at 6:12 pm
    On another note, while we are talking about fixing this, those method names
    are super non-helpful. I couldn't tell without looking at the code that
    single_display_amount was to display the unit price and display_amount was
    to display the total, much less that both functions do a currency
    conversion. Perhaps when we fix this we can rename them to something a
    little more descriptive like currency_formatted_unit_price and
    currency_formatted_total?

    Regards,

    Nate
    On Thursday, March 28, 2013 2:04:41 PM UTC-4, Nate Lowrie wrote:

    I think so. I check the 1-2-stable branch and all seemed to be good there.

    I think you want to change the line you referenced to:

    <td class="price"><%= item.single_display_amount %></td>

    The commit is here:


    https://github.com/spree/spree/commit/58ec5cd5124e66c60811a075b290248962159f9e

    I see 5 places where we need to change it based on that commit.

    Regards,

    Nate
    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Brian Quinn at Mar 28, 2013 at 8:01 pm
    Nate, I agree some of the method names a bit confusing there. I'm open to pull request to rename those to something more understandable.

    Zach, I also agree that we should only be referring to the line_item price when referring to an order, so pull request to fix any remaining issues there is welcome.

    --
    Brian Quinn

    Co-Founder, CTO
    Spree Commerce, Inc.

    Register now for SpreeConf
    May 20-21 in Washington, D.C.
    http://spreeconf.com

    On Thursday 28 March 2013 at 18:12, Nate Lowrie wrote:

    On another note, while we are talking about fixing this, those method names are super non-helpful. I couldn't tell without looking at the code that single_display_amount was to display the unit price and display_amount was to display the total, much less that both functions do a currency conversion. Perhaps when we fix this we can rename them to something a little more descriptive like currency_formatted_unit_price and currency_formatted_total?

    Regards,

    Nate
    On Thursday, March 28, 2013 2:04:41 PM UTC-4, Nate Lowrie wrote:
    I think so. I check the 1-2-stable branch and all seemed to be good there.

    I think you want to change the line you referenced to:

    <td class="price"><%= item.single_display_amount %></td>

    The commit is here:

    https://github.com/spree/spree/commit/58ec5cd5124e66c60811a075b290248962159f9e

    I see 5 places where we need to change it based on that commit.

    Regards,

    Nate
    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com (mailto:spree-user+unsubscribe@googlegroups.com).
    For more options, visit https://groups.google.com/groups/opt_out.
    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Zach Karpinski at Mar 28, 2013 at 9:46 pm
    I've sent a pull request for the 1-3-stable branch, let me know if that
    isn't right.

    I first forked Spree this morning my local machine isn't running the tests
    properly yet (none of them). This makes me a little nervous sending over
    a pull request without testing it but the 5 changes are pretty simple. I
    did find at least one place in the existing 1-3-stable branch that is doing
    the same
    thing https://github.com/spree/spree/blob/1-3-stable/core/app/views/spree/orders/_line_item.html.erb
    On Thursday, March 28, 2013 3:01:34 PM UTC-5, Brian Quinn wrote:

    Nate, I agree some of the method names a bit confusing there. I'm open
    to pull request to rename those to something more understandable.

    Zach, I also agree that we should only be referring to the line_item price
    when referring to an order, so pull request to fix any remaining issues
    there is welcome.

    --
    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Ryan Bigg at Apr 1, 2013 at 11:27 pm
    It all looks good. Thanks Zach!
    On Fri, Mar 29, 2013 at 8:46 AM, Zach Karpinski wrote:

    I've sent a pull request for the 1-3-stable branch, let me know if that
    isn't right.
    I first forked Spree this morning my local machine isn't running the tests
    properly yet (none of them). This makes me a little nervous sending over
    a pull request without testing it but the 5 changes are pretty simple. I
    did find at least one place in the existing 1-3-stable branch that is doing
    the same
    thing https://github.com/spree/spree/blob/1-3-stable/core/app/views/spree/orders/_line_item.html.erb
    On Thursday, March 28, 2013 3:01:34 PM UTC-5, Brian Quinn wrote:

    Nate, I agree some of the method names a bit confusing there. I'm open
    to pull request to rename those to something more understandable.

    Zach, I also agree that we should only be referring to the line_item price
    when referring to an order, so pull request to fix any remaining issues
    there is welcome.

    --
    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
    --
    You received this message because you are subscribed to the Google Groups "Spree" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to spree-user+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Zach Karpinski at Apr 2, 2013 at 2:43 pm
    I got a notice from Travis CI about the build failing on this. I followed
    the same list of commands that the build e-mail contained and had the same
    problem. Is there something I am missing in my tests or on my branch that
    needs to be included?

    $ bundle exec rake

    rake aborted!

    Don't know how to build task 'default'
    On Monday, April 1, 2013 6:27:25 PM UTC-5, Ryan Bigg wrote:

    It all looks good. Thanks Zach!
    --
    Don't miss SpreeConf on May 20-21: http://spreeconf.com
    Spree is hiring: http://spreecommerce.com/careers

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupspree-user @
categoriesrubyonrails
postedMar 27, '13 at 6:06p
activeApr 2, '13 at 2:43p
posts9
users5
websitespreecommerce.com
irc#RubyOnRails

People

Translate

site design / logo © 2022 Grokbase