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

[FIX] Tax computation fixed on stock_picking_taxes module #1357

Open
wants to merge 1 commit into
base: 8.0
Choose a base branch
from

Conversation

forvas
Copy link

@forvas forvas commented Mar 21, 2017

In stock_picking_taxes module, when a tax has its attribute price_include set as True (Tax included in price), amount_untaxed, amount_tax and amount_total fields are computed wrong.

Example A (Right, same values on sale order and stock picking):
Data -> Tax = IVA 21% (price_include set as False), Price = 8.00€.
Result -> Amount untaxed = 8.00€, Amount tax = 1.68€, Amount total = 9.68€.

Example B (Wrong, different values on order and stock picking):
Data -> Tax = IVA 21% (price_include set as True), Price = 8.00€.
Result -> Amount untaxed = 8.00€, Amount tax = 1.68€, Amount total = 9.68€.
When result should be -> Amount untaxed = 6.61€, Amount tax = 1.39€, Amount total = 8.00€.

Attempt to correct that issue in this pull request.


En el módulo stock_picking_taxes, cuando a un impuesto se le pone el atributo Impuestos incluidos en el precio a Verdadero, los campos base imponible, impuestos y total se recalculan mal en los albaranes.

Ejemplo A (Correcto, los valores son idénticos en pedido y en albarán):
Datos -> Impuesto = IVA 21% (price_include establecido a Falso), Precio = 8.00€.
Resultado -> Base imponible = 8.00€, Impuestos = 1.68€, Total = 9.68€.

Example B (Mal, valores diferentes en pedido y en albarán):
Datos -> Impuesto = IVA 21% (price_include establecido a Verdadero), Precio = 8.00€.
Resultado -> Base imponible = 8.00€, Impuestos = 1.68€, Total = 9.68€.
Cuando el resultado debería ser -> Base imponible = 6.61€, Impuestos = 1.39€, Total = 8.00€.

Este pull request intenta solucionar este problema.

¡Saludos!

@codecov-io
Copy link

codecov-io commented Mar 21, 2017

Codecov Report

Merging #1357 into 8.0 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              8.0    #1357   +/-   ##
=======================================
  Coverage   58.76%   58.76%           
=======================================
  Files         269      269           
  Lines        8085     8085           
=======================================
  Hits         4751     4751           
  Misses       3334     3334
Impacted Files Coverage Δ
stock_picking_taxes/models/stock_picking.py 32.81% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dc7dfc...59709ed. Read the comment docs.

@Daniel-CA
Copy link
Member

Hello @forvas,
This change was added by @pedrobaeza to fix #565 bug, don't know if this approach is the correct. Maybe Pedro can clarify it.

@pedrobaeza
Copy link
Member

El commit apuntado no es precisamente ése, Dani, pero Juan, no veo qué tiene que ver el partner asociado con lo de precios incluidos o no incluidos.

@Daniel-CA
Copy link
Member

Cierto Pedro, me lie con la linea por que la función compute_all se llama también un poco más abajo, pero veo correcto la modificación de @forvas, al final la signature de esa función es:
def compute_all(self, price_unit, quantity, product=None, partner=None, force_excluded=False)
Y le está pasando el partner 2 veces.

@pedrobaeza
Copy link
Member

Vale, entendido entonces el problema. Pero hay varias cosas que cambiar:

  • Que el partner que se queda sea el de la factura, no el del albarán.
  • Ya que en la signatura es un keyword argument, que se utilice como keyword, no como posicional (es decir, que lleve lo de partner=...)

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.

4 participants