8000 [18.0][IMP] base_tier_validation: Allow attachment generation after validation by bosd · Pull Request #1076 · OCA/server-ux · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[18.0][IMP] base_tier_validation: Allow attachment generation after validation #1076

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
Merged
merged 1 commit into from
May 12, 2025
Merged

[18.0][IMP] base_tier_validation: Allow attachment generation after validation #1076

merged 1 commit into from
May 12, 2025

Conversation

bosd
Copy link
Contributor
@bosd bosd commented May 8, 2025 •
edited
Loading

On a lot of models, it is a regular flow to have a pdf file generated after the validation flow has finished.
e.g. on sale orders or invoices.

As discovered / discussed in: OCA/account-invoicing#1998 (comment)

@OCA-git-bot
Copy link
Contributor

Hi @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

@bosd bosd mentioned this pull request May 8, 2025
Open
@LoisRForgeFlow
Copy link
Contributor

@victoralmau Maybe you have some good inputs for this one. In this PR you said that it was not needed version 17 and 18 (#1069 (comment)).

@LoisRForgeFlow LoisRForgeFlow changed the title [IMP]base_tier_validation: Allow attachment generation after validation [18.0][IMP] base_tier_validation: Allow attachment generation after validation May 9, 2025
@victoralmau
Copy link
Member

I have rechecked the behavior of this (e.g. through the [purchase_request_tier_validation](https://github.com/OCA/purchase-workflow/tree/18.0/purchase_request_tier_validation) module) and user (Marc Demo) if you can:

  • Add attachments manually.
  • Create notes with attachments.
  • Send an email with attachments.

ejemplo-2

Can someone else check it to confirm that this change is not necessary (also not in v17)?

@bosd
Copy link
Contributor Author
bosd commented May 9, 2025

I encountered the error on sale orders and invoices on V18.
Wierd, that purchase orders seems to be working.

@bosd
Copy link
Contributor Author
bosd commented May 9, 2025

One can check the full flow in this pr: OCA/account-invoicing#2002
First added the test to show it is failling without the fixes.
2nd commit adding the Fix for the account_move model
3 commit.. adding the modified base module with message_main_attachment_id in exclusion list. Tests are passing now 🎊

In this PR you said that it was not needed version 17 and 18

Additional Context:
I can confirm that in earlier versions of odoo 18, the account_move_tier_validation was working and the extension of these exclusions was not needed. Somewhere among the last couple of months the module stopped working. (e.g. unable to send invoice to customer after validation).

Maybe there is a code change in odoo core?

victoralmau
victoralmau approved these changes May 9, 2025
View reviewed changes
Copy link
Member
@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

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

Ok, thanks for the clarification, that explains why I thought it was not necessary.
We will then add these changes to correct this problem.

Can you also add it to 17?

@bosd
Copy link
Contributor Author
bosd commented May 9, 2025

Can you also add it to 17?

Sure, althoug did not check if it is now required on v17 as well.

LoisRForgeFlow
LoisRForgeFlow approved these changes May 12, 2025
View reviewed changes
Copy link
Contributor
@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Thanks for checking guys!

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 18.0-ocabot-merge-pr-1076-by-LoisRForgeFlow-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 68da338 into OCA:18.0 May 12, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@bosd
Copy link
Contributor Author
bosd commented May 12, 2025

@LoisRForgeFlow Thanks for the merge!

Just tested the account_move_tier_validation on the V17 branch. There this fix is not needed.

...(yet)

@bosd bosd deleted the 18.0-fix-base_tier_validation branch May 12, 2025 08:13
@bosd bosd mentioned this pull request May 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@victoralmau victoralmau victoralmau approved these changes

@LoisRForgeFlow LoisRForgeFlow LoisRForgeFlow approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@bosd @OCA-git-bot @LoisRForgeFlow @victoralmau
0