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

[FIX] account: Remove invoice_pdf_report_id on reset to draft #161991

Open
wants to merge 1 commit into
base: saas-16.3
Choose a base branch
from
Open

[FIX] account: Remove invoice_pdf_report_id on reset to draft #161991

wants to merge 1 commit into from
+15 −1

Conversation

iada-odoo
Copy link
Contributor

@iada-odoo iada-odoo commented Apr 15, 2024 •
edited

Current Behavior:
is_move_sent is not updated if there is an existing invoice_pdf_report_id.

Purpose of this PR:
Remove the invoice_pdf_report_id when resetting invoice to draft so that when the invoice is sent again (using the Send and Print wizard), is_move_sent is updated.

Steps to Reproduce on Runbot:

  1. Create and Send and Print invoice. (is_move_sent is set to True)
  2. Reset invoice to draft. (is_move_sent is set to False)
  3. Confirm and resend invoice using Send and Print wizard. (is_move_sent remains False, expected to be changed to True)

Notes:
This PR #116698 added a _compute in Odoo 16, however it was removed in Odoo 17 with this PR #125392. is_move_sent is not updated on the second Send and Print because of the existing invoice_pdf_report_id (generated and linked during the first Send and Print).

opw-3849109

@robodoo
Copy link
Contributor

robodoo commented Apr 15, 2024

Pull request status dashboard.

@C3POdoo C3POdoo requested review from a team and JulienAlardot and removed request for a team April 15, 2024 20:12
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Apr 15, 2024
@iada-odoo iada-odoo force-pushed the 17.0-opw-3849109-iada branch 5 times, most recently from 03c87f4 to 4ec9ad4 Compare April 16, 2024 18:00
@JulienAlardot
Copy link
Contributor

Hello, thanks for this PR but that is not what we want.
The reason is_move_sent isn't reset to False is because we cannot know if it was sent through a download + print or download + mail sent on another channel. That is why it is always considered as sent once the pdf is generated. Even if we re-generate it afterhand
The solution for that ticket customer could be to filter on invoice_pdf_report_id and safely delete the ones they know are not sent if needed

@iada-odoo
Copy link
Contributor Author

Hello, thanks for this PR but that is not what we want. The reason is_move_sent isn't reset to False is because we cannot know if it was sent through a download + print or download + mail sent on another channel. That is why it is always considered as sent once the pdf is generated. Even if we re-generate it afterhand The solution for that ticket customer could be to filter on invoice_pdf_report_id and safely delete the ones they know are not sent if needed

Hello @JulienAlardot thank you for your review and comments.
Currently, the is_move_sent is set to False when the account move is set to draft (after being posted, with pdf generated).
If the original pdf is never removed then is_move_sent is not set to True even if regenerating a new pdf. The workflow is outlined below.

First create an invoice, send and print
Reset to draft, change some fields
Resend using send and print.

If the original pdf is not removed, the new one is not regenerated (and is_move_sent is not updated).
Would it then be more appropriate to keep is_move_sent = True?

@iada-odoo iada-odoo force-pushed the 17.0-opw-3849109-iada branch 2 times, most recently from dd9c674 to 2cc4af7 Compare April 25, 2024 18:05
@iada-odoo
Copy link
Contributor Author

Hello, thanks for this PR but that is not what we want. The reason is_move_sent isn't reset to False is because we cannot know if it was sent through a download + print or download + mail sent on another channel. That is why it is always considered as sent once the pdf is generated. Even if we re-generate it afterhand The solution for that ticket customer could be to filter on invoice_pdf_report_id and safely delete the ones they know are not sent if needed

Hello @JulienAlardot, I have made changes so that is_move_sent is not set to False when the move is reset to draft.
This will then confirm that if a move is sent once, it will be deemed as always sent regardless of the state of the move or regeneration of PDFs.

@iada-odoo iada-odoo force-pushed the 17.0-opw-3849109-iada branch 2 times, most recently from d7a52db to 7c5a642 Compare April 29, 2024 19:46
@JulienAlardot
Copy link
Contributor

After further discussion with PO, the fix is what we want, but we would need it in 16.3 (send&Print rework)
Could you retarget it?

JulienAlardot
JulienAlardot requested changes May 16, 2024
View reviewed changes
addons/account/models/account_move.py Outdated
@@ -4152,7 +4152,7 @@ def button_draft(self):
move.mapped('line_ids.analytic_line_ids').unlink()

self.mapped('line_ids').remove_move_reconcile()
self.write({'state': 'draft', 'is_move_sent': False})
self.write({'state': 'draft'})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.write({'state': 'draft'})
self.state = 'draft'

Since there's only one field left :p

addons/account/tests/test_account_move_send.py Outdated
Comment on lines 1064 to 892
def test_out_invoice_is_move_sent(self):
invoice = self.init_invoice(move_type='out_invoice', amounts=[1000.0], post=True)
option_vals = self.env['account.move.send']._get_wizard_vals_restrict_to({'checkbox_send_mail': True})
wizard = self.create_send_and_print(invoice, **option_vals)

wizard.action_send_and_print()
self.assertTrue(invoice.is_move_sent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Updating the test would be nice, something like

  • Posting a move
  • Testing the state
  • Send & Print
  • Testing the state
  • Reset to draft
  • Testing the state

could work, maybe also testing how it changes when deleting the pdf

@iada-odoo iada-odoo changed the base branch from 17.0 to saas-16.3 May 16, 2024 19:50
@iada-odoo iada-odoo force-pushed the 17.0-opw-3849109-iada branch 3 times, most recently from e30ceb0 to 2c5ccb3 Compare May 16, 2024 20:58
JulienAlardot
JulienAlardot requested changes May 17, 2024
View reviewed changes
Copy link
Contributor

@JulienAlardot JulienAlardot left a comment

Choose a reason for hiding this comment

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

Could you update the commit message Notes: section? It still mentions the v17.0. It should be mentioning ef9e266 16.3 commit

@iada-odoo
[FIX] account: Remove is_move_sent change on reset to draft
7d29132 
Current Behavior:
is_move_sent is not updated if there is an existing invoice_pdf_report_id.

Purpose of this PR:
Remove the `is_move_sent`: False on when resetting the account move to draft.
This guarantees, that once the move has been sent via action_send_and_print, it will be marked as sent.

Steps to Reproduce on Runbot:
Create and Send and Print invoice. (is_move_sent is set to True)
Reset invoice to draft. (is_move_sent is set to False)
Confirm and resend invoice using Send and Print wizard. (is_move_sent remains False, expected to be changed to True)

Notes:
This PR odoo#116698 added a _compute in Odoo 16, however it was removed in saas-16.3  with this PR odoo#125392. is_move_sent is not updated on the second Send and Print because of the existing invoice_pdf_report_id (generated and linked during the first Send and Print).

opw-3849109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JulienAlardot JulienAlardot JulienAlardot requested changes

Assignees
No one assigned
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@iada-odoo @robodoo @JulienAlardot @C3POdoo