search

OCA / commission

Odoo Commission Management
GNU Affero General Public License v3.0
115 stars 344 forks source link

[15.0][IMP] sale_commission: print partial and document total in report #572

Closed toita86 closed 3 weeks ago

toita86 commented 1 month ago

FWP of https://github.com/OCA/commission/pull/570 the implementation is a little bit different due to other modules dependance on the position of certain elements in the report.

OCA-git-bot commented 1 month ago

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

toita86 commented 1 month ago

Hi @aleuffre, while FW you feature to adapt it to v15.0 I needed to change the position of the div that contains the total.

but now looks like this: image

Do you have any suggestion that could make it more similar to you implementation?

aleuffre commented 1 month ago

Hi @aleuffre, while FW you feature to adapt it to v15.0 I needed to change the position of the div that contains the total.

but now looks like this: Do you have any suggestion that could make it more similar to you implementation?

I took heavy inspiration from a similar section in the Sales Order report. As you can see, what I did was put the total outside the table and tfoot element, because the tfoot like the thead is printed again on every page, which is something we wanted to avoid.

So yes, I would try to put the total in a div, outside the table itself

toita86 commented 1 month ago

So yes, I would try to put the total in a div, outside the table itself

At first i didn't wanted to move it outside because the module account_commission used the tfoot on to add some spaces https://github.com/OCA/commission/blob/40491a3c2b1d86d6a3ab28a7e0997e3f8f907216/account_commission/views/report_settlement_templates.xml#L41

by removing the xpath element added in account_commission and moving the div outside everything looks fine in IMO. Now this is the output image

OCA-git-bot commented 3 weeks ago

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

OCA-git-bot commented 3 weeks ago

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

OCA-git-bot commented 3 weeks ago

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