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.

Sign up for GitHub

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

Jump to bottom

[16.0][IMP] l10n_it_fatturapa_import_zip: import only xml + enable customer xml import + manage payment for customer invoice + set debit account for customer invoice #4300

Merged
merged 3 commits into from
Oct 18, 2024
Merged

[16.0][IMP] l10n_it_fatturapa_import_zip: import only xml + enable customer xml import + manage payment for customer invoice + set debit account for customer invoice #4300

merged 3 commits into from
Oct 18, 2024

Conversation

andreampiovesana
Copy link
Contributor

@andreampiovesana andreampiovesana commented Jul 26, 2024 •
edited
Loading

Enable import only xml
Enable customer xml import
Set payment term for customer
Set debit account for customer

MaurizioPellegrinet
MaurizioPellegrinet approved these changes Jul 26, 2024
View reviewed changes
Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

LGTM

@SirAionTech
Copy link
Contributor

Come ti dicevo poco fa in chiamata, il primo commit 12222ba di questa PR è anche in #4299.
Se ti servono quelle modifiche, dovresti includerle seguendo https://github.com/OCA/maintainer-tools/wiki/Use-temporary-reference(s)-to-another-pull-request(s).

@andreampiovesana andreampiovesana mentioned this pull request Jul 26, 2024
Closed
SirAionTech
SirAionTech requested changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@SirAionTech SirAionTech 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!
Riporto #4300 (comment):

il primo commit 12222ba di questa PR è anche in #4299.
Se ti servono quelle modifiche, dovresti includerle seguendo https://github.com/OCA/maintainer-tools/wiki/Use-temporary-reference(s)-to-another-pull-request(s).

Potresti creare una issue come descritto in https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo#apertura-issue?

Potresti aggiungere un test per coprire il nuovo caso d'uso? Sarà molto simile a quello esistente, solo dovrai eseguire un metodo diverso per l'import e controllare che non vengano create fatture.

Potresti modificare il messaggio del secondo commit per spiegare un po' meglio il motivo delle modifiche?
Al momento è

16.0-fatturapa_import_zip_enable_import_only_xml

che non è nemmeno una frase.
Nota che deve seguire https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message

l10n_it_fatturapa_import_zip/models/attachment.py Outdated
@@ -200,6 +200,40 @@ def action_import(self):

self.state = "done"

def action_import_no_invoice(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Tutto questo metodo è quasi un copia/incolla di action_import, è possibile non duplicare il codice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto

l10n_it_fatturapa_import_zip/wizards/wizard_import_fatturapa.py Outdated
Comment on lines 124 to 125
if partner.property_supplier_payment_term_id:
payment_term_id = partner.property_supplier_payment_term_id.id
Copy link
Contributor

Choose a reason for hiding this comment

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

Questo è la stessa cosa che fa super, non si potrebbe chiamare quello invece di ripetere qui il suo codice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto

@andreampiovesana andreampiovesana mentioned this pull request Jul 29, 2024
Open
2 tasks
@andreampiovesana andreampiovesana changed the title [16.0][IMP] l10n_it_fatturapa_import_zip: import only xml and enable customer xml import [16.0][IMP] l10n_it_fatturapa_import_zip: import only xml + enable customer xml import + manage payment for customer invoice + set debit account for customer invoice Jul 31, 2024
@francesco-ooops francesco-ooops linked an issue Jul 31, 2024 that may be closed by this pull request
Possibilità di import zip dei soli file XML, e poi di importare il file XML anche per le fatture di vendita #4301
Open
2 tasks
@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch from ef8c252 to 6ac556a Compare August 1, 2024 19:31
@andreampiovesana
Copy link
Contributor Author

Grazie della PR! Riporto #4300 (comment):

il primo commit 12222ba di questa PR è anche in #4299.
Se ti servono quelle modifiche, dovresti includerle seguendo https://github.com/OCA/maintainer-tools/wiki/Use-temporary-reference(s)-to-another-pull-request(s).

Potresti creare una issue come descritto in https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo#apertura-issue?

Fatto

Potresti aggiungere un test per coprire il nuovo caso d'uso? Sarà molto simile a quello esistente, solo dovrai eseguire un metodo diverso per l'import e controllare che non vengano create fatture.

Puoi aiutarmi tu a farlo?

Potresti modificare il messaggio del secondo commit per spiegare un po' meglio il motivo delle modifiche? Al momento è

16.0-fatturapa_import_zip_enable_import_only_xml

che non è nemmeno una frase. Nota che deve seguire https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message

fatto

@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch 3 times, most recently from 4f65d93 to f9afdbc Compare August 2, 2024 06:46
MaurizioPellegrinet
MaurizioPellegrinet approved these changes Aug 2, 2024
View reviewed changes
Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

Funzionale OK

@SirAionTech
Copy link
Contributor

Quando devo aggiornare la revisione ricordati per favore di indicarlo in github come descritto in https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review
image

@SirAionTech SirAionTech added the needs fixing Has conflicts or is failing mandatory CI checks label Aug 21, 2024
@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch from f9afdbc to 546a1a9 Compare September 5, 2024 20:53
@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch 2 times, most recently from 39e4581 to ea7c8a4 Compare September 13, 2024 13:32
@SirAionTech SirAionTech removed the needs fixing Has conflicts or is failing mandatory CI checks label Sep 13, 2024
@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch from ea7c8a4 to b197b85 Compare September 13, 2024 15:01
marcelofrare
marcelofrare approved these changes Sep 13, 2024
View reviewed changes
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.

LGTM

@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch from b197b85 to 2bd75e6 Compare September 26, 2024 19:47
@andreampiovesana andreampiovesana force-pushed the 16.0-imp-fatturapa_import_zip-payment_term_for_customer branch from 2bd75e6 to 8b9022f Compare September 27, 2024 06:58
eLBati
eLBati approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

LGTM

@eLBati
Copy link
Member

eLBati commented Oct 18, 2024

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-4300-by-eLBati-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 18, 2024
@OCA-git-bot
Merge PR #4300 into 16.0
943420b 
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-4300-by-eLBati-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 18, 2024
@OCA-git-bot
Merge PR #4300 into 16.0
02fd099 
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-4300-by-eLBati-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d8cb80c into OCA:16.0 Oct 18, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at ddb4444. 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
Reviewers

@MaurizioPellegrinet MaurizioPellegrinet MaurizioPellegrinet approved these changes

@marcelofrare marcelofrare marcelofrare approved these changes

@eLBati eLBati eLBati approved these changes

@SirAionTech SirAionTech Awaiting requested review from SirAionTech

Assignees
No one assigned
Labels
16.0 merged 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possibilità di import zip dei soli file XML, e poi di importare il file XML anche per le fatture di vendita
7 participants
@andreampiovesana @SirAionTech @eLBati @OCA-git-bot @marcelofrare @MaurizioPellegrinet @francesco-ooops