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

[16.0] [MIG] l10n_it_declaration_of_intent #3026

Conversation

ArcadioPinto
Copy link

Migrazione l10n_it_declaration_of_intent da 14.0 a 16.0

Confermo di aver firmato il CLA https://odoo-community.org/page/cla e di aver letto le linee guida su https://odoo-community.org/page/contributing

@eLBati
Copy link
Member

eLBati commented Nov 11, 2022

/ocabot migration l10n_it_declaration_of_intent

@AmePal
Copy link

AmePal commented Nov 11, 2022

Confermo che, da un punto di vista funzionale, la migrazione è andata a buon fine

Comment on lines 151 to 155
self.narration += _(
"\n\nVostra dichiarazione d'intento nr {partner_document_number} del {partner_document_date}, nostro protocollo nr {number} del {date}, protocollo telematico nr {telematic_protocol}."
"\n\nVostra dichiarazione d'intento nr {partner_document_number} "
"del {partner_document_date}, nostro protocollo nr {number} del "
"{date}, "
"protocollo telematico nr {telematic_protocol}."
Copy link
Contributor

Choose a reason for hiding this comment

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

Questa stringa non ha senso che sia traducibile visto che il testo è in italiano ed è predefinito.
Non penso neanche ci siano casi in cui debba essere in inglese AFAIK.
Di conseguenza toglierei anche i placeholder.

Comment on lines +165 to +166
"Limit passed for declaration {number}."
"\nExcess value: {available_amount}{symbol}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nel mondo anglosassone il simbolo della valuta va prima della cifra. Poi sarà cura del traduttore metterlo dopo invertendo i placeholder.

Suggested change
"Limit passed for declaration {number}."
"\nExcess value: {available_amount}{symbol}"
"Limit passed for declaration {number}."
"\nExcess value: {symbol}{available_amount}"

@eLBati
Copy link
Member

eLBati commented Nov 11, 2022

@primes2h queste modifiche però non riguardano il porting, le lascerei a delle PR dedicate

SimoRubi and others added 20 commits November 11, 2022 14:11
…ing optimizations available in 14.0.

Use common accounting methods for tests setup
Roadmap comment on currency
Shortcut for purchase and sale documents
Simplify amount computation for declaration lines
Check move_type only for short_type computation
Apply date filter only if there is a date
…eclaration_residual_amounts method and KeyError exception in check_declarations_amounts.
… execution such as following:

2022-02-08 08:10:43,744 111657 WARNING 14.0-test-pr2640 odoo.tests.common.onchange: The sequence format has changed. It was previously 'Test invoice 3' and it is now 'Test invoice no valid taxes'.

The sequence will never restart.
The incrementing number in this case is '0'.
2022-02-08 08:10:44,608 111657 WARNING 14.0-test-pr2640 odoo.tests.common.onchange: The sequence format has changed. It was previously 'Test invoice no valid taxes' and it is now 'Test invoice future'.

See https://github.com/OCA/l10n-italy/runs/5100117472?check_suite_focus=true#step:8:334 for further info
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: l10n-italy-14.0/l10n-italy-14.0-l10n_it_declaration_of_intent
Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-14-0/l10n-italy-14-0-l10n_it_declaration_of_intent/
Currently translated at 95.9% (71 of 74 strings)

Translation: l10n-italy-14.0/l10n-italy-14.0-l10n_it_declaration_of_intent
Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-14-0/l10n-italy-14-0-l10n_it_declaration_of_intent/it/
Currently translated at 97.2% (72 of 74 strings)

Translation: l10n-italy-14.0/l10n-italy-14.0-l10n_it_declaration_of_intent
Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-14-0/l10n-italy-14-0-l10n_it_declaration_of_intent/it/
fixed pre-commit stuff

fixed stuff testing and pre commit
@ArcadioPinto ArcadioPinto force-pushed the 16.0-mig-l10n_it_declaration_of_intent branch from 38e3a57 to 015208f Compare November 11, 2022 13:12
@primes2h
Copy link
Contributor

primes2h commented Nov 11, 2022

@primes2h queste modifiche però non riguardano il porting, le lascerei a delle PR dedicate

Ho proposto queste modifiche solo perché sono su parti di codice modicate in modo sostanziale dalle richieste di pre-commit in fase di porting. (aggiunta dei placeolder)
Es. vedi ee3f6ba

Copy link
Contributor

@primes2h primes2h left a comment

Choose a reason for hiding this comment

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

@ArcadioPinto

Coma da linee guida [*] il commit di pre-commit andrebbe messo prima di quello di migrazione.
Inoltre Il testo standard dovrebbe essere `[IMP] $module: pre-commit stuff.

[*] https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0#technical-method-to-migrate-a-module-from-150-to-160-branch

@eLBati
Copy link
Member

eLBati commented Nov 11, 2022

@primes2h queste modifiche però non riguardano il porting, le lascerei a delle PR dedicate

Ho proposto queste modifiche solo perché sono su parti di codice modicate in modo sostanziale dalle richieste di pre-commit in fase di porting. (aggiunta dei placeolder) Es. vedi ee3f6ba

In ogni caso queste modifiche andrebbero fatte sia su 16 che su 14

@primes2h
Copy link
Contributor

@ArcadioPinto

Dimenticavo, puoi accorpare per cortesia i commit come indicato qui?

https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests

Grazie.

@primes2h
Copy link
Contributor

@primes2h queste modifiche però non riguardano il porting, le lascerei a delle PR dedicate

Ho proposto queste modifiche solo perché sono su parti di codice modicate in modo sostanziale dalle richieste di pre-commit in fase di porting. (aggiunta dei placeolder) Es. vedi ee3f6ba

In ogni caso queste modifiche andrebbero fatte sia su 16 che su 14

Certamente.
Questo tipo di richieste da parte di pre-commit però adesso pongono un problema.
L'aggiunta dei placeholder, per esempio, è una modifica sostanziale e non puramente estetica.

Come ne teniamo traccia per le versioni precedenti?

CC: @OCA/local-italy-developers

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.

Grazie della PR!
Puoi squashare i commit amministrativi come indicato in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0#tasks-to-do-in-the-migration?
Anche le modifiche automatiche di pre-commit dovrebbero essere lasciate separate come indicato in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0#technical-method-to-migrate-a-module-from-150-to-160-branch

string="Force Declaration of Intent",
)

def _compute_tax_id(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Non ho trovato chiamate a questo metodo, che ne dici possiamo provare a rimuoverlo?

)

@api.model
def create(self, values):
Copy link
Contributor

Choose a reason for hiding this comment

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

Su questo metodo viene sollevato:

WARNING db_name odoo.api.create: The model odoo.addons.l10n_it_declaration_of_intent.models.declaration is not overriding the create method in batch

Riesci a risolvere?

invoice = self.env["account.move"].browse(invoice_id)
for declaration in self.declaration_ids:
invoice.declaration_of_intent_ids = [(4, declaration.id)]
invoice.declaration_of_intent_ids = self.declaration_ids
Copy link
Contributor

Choose a reason for hiding this comment

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

Qui la logica viene cambiata: prima venivano collegate le varie dichiarazioni alla fattura, mantenendo le dichiarazioni già presenti; ora invece le dichiarazioni nella fattura vengono sostituite.
Come mai viene cambiata la logica?

Copy link

@marcelofrare marcelofrare left a comment

Choose a reason for hiding this comment

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

funziona su runboat
É presente l'anomalia che accoda sempre l'annotazione a pie di fattura con i riferimenti alla dichiarazione, anche quando si porta a bozza e si riconferma la fattura.
Anomalia esistente anche sulla 14.0

@primes2h
Copy link
Contributor

primes2h commented Nov 13, 2022

funziona su runboat É presente l'anomalia che accoda sempre l'annotazione a pie di fattura con i riferimenti alla dichiarazione, anche quando si porta a bozza e si riconferma la fattura. Anomalia esistente anche sulla 14.0

Il bug è presente anche nella 12.0 ma c'è una PR aperta che risolve.
#2328

Se qualcuno vuole fare review possiamo fare il merge della PR e portare la correzione anche nelle versioni successive.

@tafaRU
Copy link
Member

tafaRU commented Nov 25, 2022

@ArcadioPinto hai intenzione di portare avanti la PR? In caso affermativo, potresti verificare i commenti irrisolti?
Grazie.

@tafaRU
Copy link
Member

tafaRU commented Dec 2, 2022

hai intenzione di portare avanti la PR?

me ne occupo io

@tafaRU
Copy link
Member

tafaRU commented Dec 2, 2022

Chiudo in favore di #3081

@tafaRU tafaRU closed this Dec 2, 2022
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.