[FIX] account: select generated pdf streams from invoices without attachments #160670
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
C3POdoo
requested review from
a team,
clbr-odoo,
rco-odoo and
HydrionBurst
and removed request for
a team
mizosoft
force-pushed
the
16.0-opw-3827700-reload_from_attachment_misbehaviour-mohu
branch
from
9e44b56
to
1d65467
Compare
mizosoft
force-pushed
the
16.0-opw-3827700-reload_from_attachment_misbehaviour-mohu
branch
from
1d65467
to
6759c9b
Compare
mizosoft
force-pushed
the
16.0-opw-3827700-reload_from_attachment_misbehaviour-mohu
branch
from
6759c9b
to
a15ec77
Compare
mizosoft
force-pushed
the
16.0-opw-3827700-reload_from_attachment_misbehaviour-mohu
branch
3 times, most recently
from
f56dfcc
to
4bb5f24
Compare
mizosoft
changed the title
mizosoft
force-pushed
the
16.0-opw-3827700-reload_from_attachment_misbehaviour-mohu
branch
from
5fecf05
to
10bb54d
Compare
william-andre
approved these changes
Apr 17, 2024
There was a problem hiding this comment.
Always better to use recordsets instead of lists of records.
Since it's a test, I'll 🙈 but try to remember that in the future 😉
(Same for the creation, better to do it in batch)
@robodoo r+
| }) for i in range(50)] # Make this a multipage invoice. | ||
| }) | ||
|
|
||
| self.assert_invoice_creation([first_invoice, second_invoice, third_invoice], second_invoice) |
There was a problem hiding this comment.
Suggested change
| self.assert_invoice_creation([first_invoice, second_invoice, third_invoice], second_invoice) | |
| self.assert_invoice_creation((first_invoice + second_invoice + third_invoice), second_invoice) |
Comment on lines
+196
to
+197
| self.assertTrue( | ||
| invoice_to_report.id in [invoice.id for invoice in invoices], "Invoice to report must be in invoices list") |
There was a problem hiding this comment.
Suggested change
| self.assertTrue( | |
| invoice_to_report.id in [invoice.id for invoice in invoices], "Invoice to report must be in invoices list") | |
| self.assertIn(invoice_to_report, invoices, "Invoice to report must be in invoices list") |
Comment on lines
+200
to
+201
| for invoice in invoices: | ||
| invoice.action_post() |
There was a problem hiding this comment.
Suggested change
| for invoice in invoices: | |
| invoice.action_post() | |
| invoices.action_post() |
Comment on lines
+217
to
+218
| aggregate_report_content, content_type = reports._render_qweb_pdf(invoices_report_ref, | ||
| res_ids=[invoice.id for invoice in invoices]) |
There was a problem hiding this comment.
Suggested change
| aggregate_report_content, content_type = reports._render_qweb_pdf(invoices_report_ref, | |
| res_ids=[invoice.id for invoice in invoices]) | |
| aggregate_report_content, content_type = reports._render_qweb_pdf(invoices_report_ref, | |
| res_ids=invoices.ids) |
|
Neat suggestions. I'll keep that in mind, thank you! |
robodoo
pushed a commit
that referenced
this pull request
Apr 17, 2024
…achments Issue ----- Report creation for multiple invoices fails (e.g. with <AttributeError: "NoneType" object has no attribute "getvalue">) when some, but not all, of the invoices across the selection have a previously generated report as an attachment, and the 'Reload from attachment' option is enabled for the report action. Steps ----- - Enable 'Reload from attachment' from Settings -> Techical -> Actions -> Reports -> invoices. - Create 3 invoices without any previous reports as attachments. - Select the middle invoice and generate a report (Print -> Invoices). An attachment will be created for the invoice. - Select the 3 invoices then generate a report (Print -> Invoices). Cause ----- When "Reload from attachment" is enabled, the algorithm for creating aggregate reports for a number of invoices selects only those that don't have an attachment to generate a pdf for. These are copied into a separate list "res_ids_wo_stream". When the pdfs streams are generated, they're put back into the return value "collected_streams. However, this was done from the original ids list "res_ids" rather than the selected one, which resulted in an indexing issue. opw-3827700 Closes #157975 closes #160670 Signed-off-by: William André (wan) <wan@odoo.com>
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
1 similar comment
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
mizosoft
added a commit
to odoo-dev/odoo
that referenced
this pull request
Apr 22, 2024
Issue ----- Make `test_ir_actions_report.py` configure the invoices report to create attachments if it is not configured to do so. Previously (in odoo#160670) it was assumed that this is the case, so the test failed otherwise. Also, the portion of the test concerned with \odoo#160670 is moved to `base` instead of `account`.
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
5 similar comments
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
|
@mizosoft @william-andre this pull request has forward-port PRs awaiting action (not merged or closed): |
This was referenced May 3, 2024
mizosoft
added a commit
to odoo-dev/odoo
that referenced
this pull request
May 7, 2024
Issue ----- Make `test_ir_actions_report.py` configure the invoices report to create attachments if it is not configured to do so. Previously (in odoo#160670) it was assumed that this is the case, so the test failed otherwise. Also, the portion of the test concerned with \odoo#160670 is moved to `base` instead of `account`.
mizosoft
added a commit
to odoo-dev/odoo
that referenced
this pull request
May 16, 2024
Issue ----- Make `test_ir_actions_report.py` configure the invoices report to create attachments if it is not configured to do so. Previously (in odoo#160670) it was assumed that this is the case, so the test failed otherwise. Also, the portion of the test concerned with \odoo#160670 is moved to `base` instead of `account`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Report creation for multiple invoices fails (e.g. with <AttributeError: "NoneType" object has no attribute "getvalue">)
when some, but not all, of the invoices across the selection have a previously generated report
as an attachment, and the 'Reload from attachment' option is enabled for the report action.
Steps
for the invoice.
Cause
When "Reload from attachment" is enabled, the algorithm for creating aggregate reports for a number
of invoices selects only those that don't have an attachment to generate a pdf for. These are copied
into a separate list "res_ids_wo_stream". When the pdfs streams are generated, they're put back into
the return value "collected_streams. However, this was done from the original ids list "res_ids" rather
than the selected one, which resulted in an indexing issue.
opw-3827700
Closes #157975