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] l10n_it_split_payment: fix unbalanced lines #2515

Merged
merged 3 commits into from
Aug 24, 2022

Conversation

Borruso
Copy link
Contributor

@Borruso Borruso commented Nov 16, 2021

Per riprodurre l'errore:

  • crea fattura
  • aggiungere una riga split payment
  • controllare le righe contabili generate (Ok)
  • aggiungere nuova riga split payment
  • controllare le righe contabili generate (Errate)

Mentre se faccio fattura da ordine e non aggiungo poi righe aggiuntive allora le righe contabili generate sono corrette

Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

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

Grazie della PR!
Vorrei provarla ma non so come riprodurre l'errore, puoi aggiungere in descrizione almeno gli step per riprodurre il problema?
Se poi riesci sarebbe bene anche aggiungere un test per evitare regressioni in futuro.

In base a https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo poi servirebbe anche una issue di tracciamento per portare l'eventuale correzioni alle altre versinoi supportate.

@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch from d4983e8 to 2b9ef52 Compare November 19, 2021 09:55
@Borruso
Copy link
Contributor Author

Borruso commented Nov 19, 2021

Grazie della PR! Vorrei provarla ma non so come riprodurre l'errore, puoi aggiungere in descrizione almeno gli step per riprodurre il problema? Se poi riesci sarebbe bene anche aggiungere un test per evitare regressioni in futuro.

In base a https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo poi servirebbe anche una issue di tracciamento per portare l'eventuale correzioni alle altre versinoi supportate.

Aggiunto test

@Borruso Borruso requested a review from SimoRubi November 19, 2021 10:03
@tafaRU tafaRU removed the question label Nov 19, 2021
@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch from 2b9ef52 to 1012ba6 Compare November 20, 2021 18:03
Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

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

Grazie degli step e del test!

In base a https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo poi servirebbe anche una issue di tracciamento per portare l'eventuale correzioni alle altre versinoi supportate.

Aggiungo che per poter vedere le righe contabili devo confermare la fattura e per poter aggiungere la seconda riga devo prima riportarla in bozza.
Una volta fatto ciò in runbot ottengo l'errore:
image
che suppongo sia quello che vuoi correggere.

Ho provato nel runbot generato dalla tua PR ma non riesco ad aggiungere la seconda riga perché continua a togliermela da sotto il mouse, qui sotto il video che parte dalla fattura con lo split payment già confermata e riportata in bozza:
https://user-images.githubusercontent.com/32064796/142841062-ba6eaa50-5ffc-4953-be2e-4fa00b4d1fb1.mp4

Puoi verificare?

Copy link
Contributor

@SirTakobi SirTakobi left a comment

Choose a reason for hiding this comment

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

Aspettavo a fare review perché i test falliscono, ma tanto vale scriverlo direttamente 😄
Puoi correggere i test?

@Borruso
Copy link
Contributor Author

Borruso commented Jul 13, 2022

Aspettavo a fare review perché i test falliscono, ma tanto vale scriverlo direttamente smile Puoi correggere i test?

falliscono i test di fatturapa_out_sp

@SirTakobi
Copy link
Contributor

Aspettavo a fare review perché i test falliscono, ma tanto vale scriverlo direttamente smile Puoi correggere i test?

falliscono i test di fatturapa_out_sp

Sì ma quei test non mi pare abbiano problemi in 14.0, quindi è probabile che il fallimento dipenda dalle modifiche che stai aggiungendo in questa PR, che ne pensi?
Tra l'altro, il modulo che stai modificando è dipendenza diretta di l10n_it_fatturapa_out_sp

@tafaRU
Copy link
Member

tafaRU commented Jul 18, 2022

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 14.0.

@OCA-git-bot OCA-git-bot force-pushed the 14.0-fix-unbalanced-lines branch from dec57ed to 48e61b9 Compare July 18, 2022 10:39
@tafaRU
Copy link
Member

tafaRU commented Jul 19, 2022

@Borruso FYI sto lavorando su questa.

Copy link
Member

@tafaRU tafaRU left a comment

Choose a reason for hiding this comment

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

Per quanto riguarda i problemi sollevati nei test, debuggando ho scoperto che:

Provando la tua PR su runboat ho trovato un problema:
Peek 2022-07-20 12-38

Nota, se può essere d'aiuto: il problema specifico si risolve sostituendo in questi due punti (primo e secondo) il metodo write con update. Così facendo però si introduce di nuovo l'effetto collaterale della scomparsa delle Invoice Lines.

-edit-
Se la fattura viene salvata le Invoice Lines ricompaiono.

l10n_it_split_payment/models/account.py Outdated Show resolved Hide resolved
l10n_it_split_payment/models/account.py Outdated Show resolved Hide resolved
@tafaRU
Copy link
Member

tafaRU commented Jul 22, 2022

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 14.0.

@OCA-git-bot OCA-git-bot force-pushed the 14.0-fix-unbalanced-lines branch from 48e61b9 to 5e59b4c Compare July 22, 2022 10:02
@matteoopenf
Copy link
Contributor

abbiamo testato su runboat e sembra funzionare e non crea più lo sbilanciamento, credo quindi solo i test vado sistemati, appena si riesce anche con i test riproviamo e approvo

@tafaRU
Copy link
Member

tafaRU commented Jul 22, 2022

abbiamo testato su runboat e sembra funzionare e non crea più lo sbilanciamento, credo quindi solo i test vado sistemati, appena si riesce anche con i test riproviamo e approvo

stiamo lavorandoci, vedi Borruso#11

@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch from 1e4fe17 to 51cd055 Compare July 22, 2022 12:45
@Borruso Borruso requested a review from tafaRU July 22, 2022 12:48
l10n_it_split_payment/models/account.py Outdated Show resolved Hide resolved
@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch from 51cd055 to 71d5008 Compare July 27, 2022 07:47
@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch from 71d5008 to 34cb070 Compare August 5, 2022 09:33
@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch 2 times, most recently from 3165394 to 66fe90e Compare August 5, 2022 13:29
Copy link
Member

@tafaRU tafaRU left a comment

Choose a reason for hiding this comment

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

@Borruso nei vari force-pushed che hai fatto hai alterato il contenuto del commit 0c55bf6 che ora è privo di senso.
Hai pertanto due possibilità:

  1. riportarlo simile a com'era in Borruso@6b03b4d
  2. schiacchiarlo con il tuo precedente (288a730)

Per me va bene anche la seconda, next time però fai più attenzione 😉
Grazie.

Borruso and others added 3 commits August 24, 2022 12:31
Steps to reproduce:

* choose partner with fiscal position (Split Payment = True)
* create an invoice with a line
* edit the price unit

Current behavior:

See https://user-images.githubusercontent.com/3512779/179964568-955cbe00-5346-4532-a53b-efa1b1eb663e.gif for further info
2022-07-18 10:49:29,798 268 ERROR odoo odoo.addons.l10n_it_fatturapa_out_sp.tests.test_fatturapa_xml_validation: FAIL: TestFatturaPAXMLValidation.test_4_xml_export
Traceback (most recent call last):
  File "/__w/l10n-italy/l10n-italy/l10n_it_fatturapa_out_sp/tests/test_fatturapa_xml_validation.py", line 91, in test_4_xml_export
    self.check_content(xml_content, "IT06363391001_00004.xml")
  File "/__w/l10n-italy/l10n-italy/l10n_it_fatturapa_out/tests/fatturapa_common.py", line 209, in check_content
    self.assertEqual(etree.tostring(test_fatt), etree.tostring(xml))
AssertionError: b'<ns[1596 chars]016/0016</Numero><ImportoTotaleDocumento>17.08[1123 chars]ica>' != b'<ns[1596 chars]016/06/0001</Numero><ImportoTotaleDocumento>17[1126 chars]ica>'

See https://github.com/OCA/l10n-italy/runs/7387629100?check_suite_focus=true#step:8:909 for reference
@Borruso Borruso force-pushed the 14.0-fix-unbalanced-lines branch from 66fe90e to 7a40954 Compare August 24, 2022 10:39
@Borruso Borruso requested review from tafaRU and removed request for SimoRubi August 24, 2022 10:43
Copy link
Member

@tafaRU tafaRU left a comment

Choose a reason for hiding this comment

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

Grazie!

@tafaRU
Copy link
Member

tafaRU commented Aug 24, 2022

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-2515-by-tafaRU-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 62f8040 into OCA:14.0 Aug 24, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at adc1c45. Thanks a lot for contributing to OCA. ❤️

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

Successfully merging this pull request may close these issues.

7 participants