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

Improve trustworthiness of a content items price. #2897

Merged

Conversation

jacquesporveau
Copy link
Contributor

A line items price is more trustworthy than a variants price in regards
to what a customer is paying for something.

In the case that there are applications that charge different prices for variants based on criteria then using a line items price is a more accurate representation and a better source of truth as to what someone will pay for an item.

For shipping calculators that depend on how much something is costs this is an improvement.

@jacquesporveau
Copy link
Contributor Author

I don't believe that the Netlify test failures are due to changes that I've made. Seems to be checking solidus guides and I have not made any changes to those said guides.

@tvdeyen
Copy link
Member

tvdeyen commented Oct 5, 2018

Please rebase to get rid of the Netlify errors.

@jacquesporveau jacquesporveau force-pushed the jacquesporveau/fix-content-item-bugs branch from 5edca04 to 05cd72d Compare October 5, 2018 22:22
Copy link
Member

@gmacdougall gmacdougall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for adding the test coverage.

Great work Dennis!

Dennis Marchand added 2 commits October 9, 2018 08:59
There is a bug taking place in this class that I plan to fix. Before I
do that I want to place the class under test to observe it's current
  beaviour and make my bugfix more obvious.
A line items price is more trustworthy than a variants price in regards
to what a customer is paying for something.
@jacquesporveau jacquesporveau force-pushed the jacquesporveau/fix-content-item-bugs branch from 05cd72d to 351bb48 Compare October 9, 2018 15:59
Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix!

Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@tvdeyen tvdeyen merged commit 65f3157 into solidusio:master Oct 11, 2018
@jacquesporveau
Copy link
Contributor Author

Thanks guys!

@jacquesporveau jacquesporveau deleted the jacquesporveau/fix-content-item-bugs branch October 11, 2018 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants