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

[15.0][MIG+REF] sale_commission: Refactorization to split and extract the base #387

Merged
merged 105 commits into from
Dec 5, 2022

Conversation

pedrobaeza
Copy link
Member

Restarting from the excellent work done by Quartile, adding these extra things:

  • Squash of the administrative commits.
  • Make specific top-level menu and permissions for commissions.
  • Don't depend on account for base module.
  • Reorganize elements.
  • Change some parts of the code to fit better ORM tools.
  • Adjust permissions in children modules.

@Tecnativa TT36983

martiita and others added 30 commits November 19, 2022 08:52
* PEP 8 fixes
* Replace depreciated osv for orm and other depreciation fixes
* Code fix and optimization
* Add context
* migrate models security and i18n
* add demo data and minor fixes
* add sale agent and commission views
* add stock dependency add views and product/stock overrides
* fixed related fields
* minor fixes: names and labels
* change 'onchange' attribute in view to api.onchange'
* improve comments about migration
* add invoice view
* add wizards
* use api.onchange
* add store attribute on res.partner.agent
* set default currency
* change state field to readonly
* migrate stock picking and settlements views
* fix invoice view
* fix invoice wizard
* change osv.except_osv to openerp.exceptions
* add missed report
* fix taxes in action_invoice_create
* Take default commission when manual adding agents + error on settled filter
* Many2one pointer of comission section
* Fix translation problem + add commission total on sales orders
* Cancel settlement when the associated invoice is cancel and go back to invoiced when re-validated
On generate agent invoice you can only select journal of type "sale". This can cause the numbers of sales invoices not correlated.
IMHO, agent invoices must be of type purchase and this solves it, allowing to select journals of type purchase.
…elect if commission is calculated from subtotal or from margin

* added python tests
Tests create invoices in a month previous the current date, which
makes that they fail on January, as previous fiscal year doesn't
exist. This is fixed creating invoices in the current date, and
calling the settle wizard with one month more. This won't fail
on December, because there's no need of the next fiscal year for
this operation
When creating a refund from an invoice, pass correct values
(converted as dictionary) for the agents lines, so that the refund
keep the same commissions (but reversed) than the original invoice.
…omer. (instead of showing when agent is not an agent)
Without this condition, all partners are suppliers, because the default
value for agent_type is agent and the onchange is executed.
* Show wizards results
* Fix refunds
* Fixes OCA#61
* Fixes OCA#63.
add same method for account invoice
FIX after changing customer on SO with lines, can't save sale order anymore
See odoo/odoo#17618
Also add button to recompute agents on SO lines
as May would have June as period start:
((5 - 1) // 3 + 1) * 3

instead, the new formula:
>>> for m in range(1, 13):
...     print (m - 1) // 3 * 3 + 1
...
1
1
1
4
4
4
7
7
7
10
10
10

See
hurrinico#2
and
odoo/odoo#17618

This extends the workaround, fixing "Record not found" after changing invoice date, for example
@yostashiro
Copy link
Member

@pedrobaeza We would like to expedite this migration due to our project situation. If you are too busy to update this PR soon, we will try following up with another PR, adding a commit on top of yours. Would you like us to do so? In that case, can you please comment on each suggested point with your opinion/judgment so that your ideas will be reflected in our update.

@pedrobaeza
Copy link
Member Author

Sorry, and thanks for the quality feedback. I'm on it.

@pedrobaeza pedrobaeza force-pushed the 15.0-mig-sale_commission branch from 001dbf7 to 433a3a2 Compare December 1, 2022 15:52
@pedrobaeza
Copy link
Member Author

I think "Agents" instead of "Commissions" makes more sense

Not on the main menu, as that one contains all the commission related things, including the agents.

@pedrobaeza
Copy link
Member Author

In my opinion, I think enterprise users may confuse a little bit for home app screen UX. Two apps related with accounting will be displayed in home screen. But just a minor thing.

@AungKoKoLin1997 fixed using this trick: https://github.com/OCA/commission/pull/387/files#diff-0be2abbed382f1580ef0a0f9ee00f07e299ef4d714aeecad3d64b1db2ebefab9R8

@pedrobaeza pedrobaeza force-pushed the 15.0-mig-sale_commission branch from 433a3a2 to 3d71603 Compare December 1, 2022 16:23
@pedrobaeza
Copy link
Member Author

Everything sorted out or commented. Please tell me if anything more is found.

@pedrobaeza pedrobaeza force-pushed the 15.0-mig-sale_commission branch from 3d71603 to d171b6b Compare December 1, 2022 16:29
Copy link
Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

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

@pedrobaeza Thank you very much for your fast follow-up. I did some functional tests and there are a few more points to take care of.

account_commission/models/commission.py Outdated Show resolved Hide resolved
account_commission/views/commission_views.xml Outdated Show resolved Hide resolved
account_commission/wizards/wizard_invoice.xml Show resolved Hide resolved
account_commission/readme/USAGE.rst Outdated Show resolved Hide resolved
account_commission/views/commission_settlement_views.xml Outdated Show resolved Hide resolved
account_commission/views/commission_settlement_views.xml Outdated Show resolved Hide resolved
account_commission/models/account_move.py Show resolved Hide resolved
- Make specific top-level menu and permissions for commissions.
- Don't depend on account.
- Reorganize elements.
- Change some parts of the code to fit better ORM tools.
- Adjust permissions in children modules.
- Add <sheet> to some form views to get modern view.
This way, we can restrict which commissions we can select or be
automatically populate in the different documents.
@pedrobaeza pedrobaeza force-pushed the 15.0-mig-sale_commission branch from d171b6b to 359c1c1 Compare December 5, 2022 08:41
@pedrobaeza
Copy link
Member Author

Changes pushed.

Copy link
Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

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

Code review. 👍 Thank you very much!

Comment on lines +55 to +58
if any(self.mapped("invoice_line_ids.any_settled")):
raise exceptions.ValidationError(
_("You can't cancel an invoice with settled lines"),
)
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking that this check should stay in button_draft() to ensure the consistency. I think doing this check in cancel step is a remnant of 12.0 in which cancel was the step to take to revert the posted invoice, which has changed in 13.0. IMO, it should be done in button_draft() now, but it's OK if it's handled in another PR also.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's was my initial thought, but in fact being a draft means the same as other areas: a draft invoice in sales means that the order is not invoiceable while not cancelled, so this one is a good balance being able to re-invoice the settlement clicking the button.

@OCA-git-bot
Copy link
Contributor

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). 🤖

@pedrobaeza
Copy link
Member Author

Let's go with this! I will add later the migration scripts for transforming 14.0 datamodel to this one:

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-387-by-pedrobaeza-bump-major, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7e975f6 into OCA:15.0 Dec 5, 2022
@OCA-git-bot
Copy link
Contributor

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

@mgielissen
Copy link

No migrations script?

@pedrobaeza pedrobaeza deleted the 15.0-mig-sale_commission branch December 5, 2022 15:12
@pedrobaeza
Copy link
Member Author

There are 2 options:

@hildickethan
Copy link
Member

hildickethan commented Dec 26, 2022

@pedrobaeza From what I see in the migration commit, the migration script would be primarily renaming models and fields. I suppose the biggest part is how to handle the module splitting.
Should an Odoo with 14.0 sale_commission that migrates to 15.0 have account_commission auto-installed too? Since it's part of the module they had, if they don't use that part then it can be uninstalled later. I didn't see that it depends on it, I suppose that's not a problem then

I'm looking to migrate this to 16.0, but I don't really have a good test subject to help with this migration script right now, do you have anything planned for it?

Edit: Now that the PR is done and I mentioned you again, probably better to reply there #394

@pedrobaeza
Copy link
Member Author

Yes, basically is to rename some tables, models, columns and fields, and set some values. I will do this on February.

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.