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

add attribute to allow products to, optionally, not be combined by SKU #34

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@mcmillhj
Contributor

mcmillhj commented Feb 12, 2016

patch for issue #13:

  • There is now a new attribute in lib/Interchange6/Cart/Product.pm called combine that accepts either a boolean value (0, 1) or a coderef.
  • There is now logic in lib/Interchange6/Cart.pm that examines the combine attribute before merging a products in the Cart that have the same SKU.
  • A new test, t/01-combine-by-sku.t tests all variations of values that can be passed to the combine attribute
@mcmillhj

This comment has been minimized.

Show comment
Hide comment
@mcmillhj

mcmillhj Feb 12, 2016

Contributor

Typos are the bane of my existence. I should have run the release tests as well. Going to fix the typo then squash into a single commit.

Contributor

mcmillhj commented Feb 12, 2016

Typos are the bane of my existence. I should have run the release tests as well. Going to fix the typo then squash into a single commit.

add attribute to allow products to, optionally, not be combined by SKU
patch for issue#13:
   There is now a new attribute in lib/Interchange6/Cart/Product.pm called `combine` that accepts either a boolean value (0, 1) or a coderef.
   There is now logic in  lib/Interchange6/Cart.pm that examines the `combine` attribute *before* merging a products in the Cart that have the same SKU.
   A new test, t/01-combine-by-sku.t tests all variations of values that can be passed to the `combine` attribute

correct type in MANIFEST file

t/01-combine-by-sku.t instead of t/01-combine-by.sku.t
@SysPete

This comment has been minimized.

Show comment
Hide comment
@SysPete

SysPete Feb 14, 2016

Member

Thanks @mcmillhj - this is perfect. I think I'm going to create a new branch in the main repo for this change. Now we have combine we need to think about what changes are needed for update and remove methods and need to take care how this affects Interchange6::Schema::Result::CartProduct class and how we handle integration in Dancer::Plugin::Interchange6::Cart.

Member

SysPete commented Feb 14, 2016

Thanks @mcmillhj - this is perfect. I think I'm going to create a new branch in the main repo for this change. Now we have combine we need to think about what changes are needed for update and remove methods and need to take care how this affects Interchange6::Schema::Result::CartProduct class and how we handle integration in Dancer::Plugin::Interchange6::Cart.

SysPete added a commit that referenced this pull request Apr 6, 2016

Merge pull request #34 from mcmillhj/feature/optionally-dont-combine-…
…products-by-sku

patch for issue#13:
   There is now a new attribute in lib/Interchange6/Cart/Product.pm
called `combine` that accepts either a boolean value (0, 1) or a
coderef.
   There is now logic in  lib/Interchange6/Cart.pm that examines the
`combine` attribute *before* merging a products in the Cart that have
the same SKU.
   A new test, t/01-combine-by-sku.t tests all variations of values that
can be passed to the `combine` attribute

correct type in MANIFEST file

t/01-combine-by-sku.t instead of t/01-combine-by.sku.t
@SysPete

This comment has been minimized.

Show comment
Hide comment
@SysPete

SysPete Apr 6, 2016

Member

Merged after local rebase.

Many thanks @mcmillhj for this contribution!

Member

SysPete commented Apr 6, 2016

Merged after local rebase.

Many thanks @mcmillhj for this contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment