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

Feature Request - Custom Totals Database Schema Improvement #1247

Closed
fooman opened this issue May 7, 2015 · 5 comments
Closed

Feature Request - Custom Totals Database Schema Improvement #1247

fooman opened this issue May 7, 2015 · 5 comments
Assignees
Labels
feature request Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@fooman
Copy link
Contributor

fooman commented May 7, 2015

Custom Totals are a great feature of Magento. I do believe we can improve their implementation and reduce overall complexity by keeping custom totals in separate dedicated database tables similar to how we store line items.

With the current structure to add a new custom total to cover base and order currency plus tax, at my last count, one has to add:

1.) 13 columns to table sales/order
2.) 4 columns to sales/invoice
3.) 4 columns to sales/creditmemo

Not everyone gets this right including for example Enterprise custom totals:

getData('base_' . $code)

vs.

sales/order::gw_card_base_price_invoiced

I believe if we had the following tables

sales/order_totals
sales/order_invoice_totals
sales/order_creditmemo_totals

with the following columns:

total_id
parent_id
code 
amount 
base_amount 
tax_amount 
base_tax_amount
calculation_order 

and additionally for the sales/order_totals table

description 
amount_invoiced 
base_amount_invoiced 
amount_refunded 
base_amount_refunded 
tax_amount_invoiced 
base_tax_amount_invoiced  
tax_amount_refunded  
base_tax_amount_refunded  

this would help a lot with the aim to
a.) remove duplication of efforts (every custom total shouldn't need to add all these columns)
b.) take away complexity of having to set up the database to reduce chances for errors

Additionally to keep track of tax I think these should be linked through to the same table that we use to keep track of taxed items to allow for multiple taxes on the totals (and be able to reconstruct this later). If we don't link to the tax table we should at least add tax_percentage.

@barbazul
Copy link
Contributor

barbazul commented May 8, 2015

+1

Current totals implementation y near to impossible to get right, and there
is very little documentation on the subject.

Also, there is too much code duplication as most custom totals work more or
less the same
El 07/05/2015 19:02, "Kristof, Fooman" notifications@github.com escribió:

Custom Totals are a great feature of Magento. I do believe we can improve
their implementation and reduce overall complexity by keeping custom totals
in separate dedicated database tables similar to how we store line items.

With the current structure to add a new custom total to cover base and
order currency plus tax, at my last count, one has to add:

1.) 13 columns to table sales/order
2.) 4 columns to sales/invoice
3.) 4 columns to sales/creditmemo

Not everyone gets this right including for example Enterprise custom
totals:

getData('base_' . $code)

vs.

sales/order::gw_card_base_price_invoiced

I believe if we had the following tables

sales/order_totals
sales/order_invoice_totals
sales/order_creditmemo_totals

with the following columns:

code
amount
base_amount
tax_amount
base_tax_amount
calculation_order

and additionally for the sales/order_totals table

description
amount_invoiced
base_amount_invoiced
amount_refunded
base_amount_refunded
tax_amount_invoiced
base_tax_amount_invoiced
tax_amount_refunded
base_tax_amount_refunded

this would help a lot with the aim to
a.) remove duplication of efforts (every custom total shouldn't need to
add all these columns)
b.) take away complexity of having to set up the database to reduce
chances for errors

Additionally to keep track of tax I think these should be linked through
to the same table that we use to keep track of taxed items to allow for
multiple taxes on the totals (and be able to reconstruct this later). If we
don't link to the tax table we should at least add tax_percentage.


Reply to this email directly or view it on GitHub
#1247.

@choukalos
Copy link

@fooman @barbazul how would you like this to impact service contracts or do you want that? If impact to service contracts should it just be passed through to the appropriate service contracts for get's but not puts/posts?

@choukalos choukalos self-assigned this May 8, 2015
@fooman
Copy link
Contributor Author

fooman commented May 10, 2015

The service contracts on the order, invoice, creditmemo should remain the same I guess, ie $order->getShippingAmount() would still produce the same value. However I'd advocate for an addition to the OrderInterface:

/**
 * Gets totals for the order.
 *
 * @return \Magento\Sales\Api\Data\OrderTotalInterface[] Array of totals.
 */
public function getTotals();

@choukalos
Copy link

Created JIRA story - MAGETWO-37216 to backlog.

@vpelipenko vpelipenko added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label May 12, 2015
@vkorotun vkorotun removed the CS label Aug 4, 2016
@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

magento-team pushed a commit that referenced this issue Jun 30, 2017
[performance] MAGETWO-69130: Non effective query for catalog search & layered navigation
magento-team pushed a commit that referenced this issue Jan 22, 2018
…1247

 - Merge Pull Request magento-engcom/magento2ce#1247 from serhii-balko/magento2:github-7213
 - Merged commits:
   1. a282973
   2. 6a58610
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

6 participants