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][IMP] account_financial_report: Open items groupy by salesman #1203

Conversation

carolinafernandez-tecnativa
Copy link
Contributor

Fw-port #1175

  • Add new grouped by field to export Open items report grouped by partner salesperson.
  • If grouped by is empty or selected partner option it will grouped by Open items by partner.
  • The field is added as a selection in order to let expanded new grouped by options in the future.
  • If it is group by salesperson print one page per salespersons
  • If it is selected show partner details order by date and partner

TT49193
@Tecnativa
@pedrobaeza @victoralmau

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jul 15, 2024
Copy link

@HaraldPanten HaraldPanten left a comment

Choose a reason for hiding this comment

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

Functional review. LGTM

@HaraldPanten
Copy link

It's functionally working fine, but IMO the translations should have been done using Weblate.

@pedrobaeza
Copy link
Member

Let's merge it for not having any conflict:

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1203-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 50d289b into OCA:16.0 Jul 15, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@eLBati
Copy link
Member

eLBati commented Jul 22, 2024

Hello, please see OCA/l10n-italy#4285

Isn't there any OCA policy about avoiding possible breaking changes?

Thanks

@pedrobaeza
Copy link
Member

Well, these kind of side effects are barely difficult to pre-detect on repos outside here. We can propose the patch though. @carolinafernandez-tecnativa can you please?

@eLBati
Copy link
Member

eLBati commented Jul 22, 2024

@pedrobaeza @carolinafernandez-tecnativa PR already open at OCA/l10n-italy#4286 thanks

I was wondering if any safeguard policy can be applied.
Maybe a global search in all OCA repos to check if the method is overridden

@pedrobaeza
Copy link
Member

Yeah, that can be a solution indeed. We'll take it into account for the next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants