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.

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

[ADD] sale_commission_product_criteria_domain #434

Merged
merged 3 commits into from
Aug 22, 2023
Merged

[ADD] sale_commission_product_criteria_domain #434

merged 3 commits into from
Aug 22, 2023

Conversation

ilyasProgrammer
Copy link
Member

@ilyasProgrammer ilyasProgrammer commented Jun 5, 2023 •
edited
Loading

No description provided.

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-sale_commission_product_criteria_domain branch 5 times, most recently from 258cfc8 to 2996e54 Compare June 7, 2023 06:03
@francesco-ooops
Copy link
Contributor

@ilyasProgrammer can you rebase?

francesco-ooops
francesco-ooops approved these changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional ok!

geomer198
geomer198 reviewed Aug 4, 2023
View reviewed changes
Copy link

@geomer198 geomer198 left a comment

Choose a reason for hiding this comment

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

Please fix comments

sale_commission_product_criteria_domain/models/commission.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/commission.py
def write(self, values):
res = super().write(values)
if self.group_id and len(self.group_id.commission_ids.ids) == 0:
self.group_id.commission_ids = [(6, 0, self.commission_id.ids)]
Copy link

Choose a reason for hiding this comment

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

Better use write function.

Copy link
Member Author

Choose a reason for hiding this comment

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

@geomer198 why ?

Copy link

Choose a reason for hiding this comment

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

I think this solution is possible.

sale_commission_product_criteria_domain/models/commission.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/commission.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/commission.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/commission.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/commission_group.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/commission_group.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/partner.py Outdated Show resolved Hide resolved
sale_commission_product_criteria_domain/models/partner.py Outdated Show resolved Hide resolved
@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-sale_commission_product_criteria_domain branch 2 times, most recently from 4b98af5 to efbee9c Compare August 8, 2023 07:14
@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-sale_commission_product_criteria_domain branch 4 times, most recently from e6cde2f to d4d4069 Compare August 16, 2023 12:22
francesco-ooops
francesco-ooops approved these changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Functional ok after adding tests

As per description:

This module allows to manage the following commission structure use case:

Agent A receives commission 50 when selling product 1 to customer X
Agent A receives commission 20 when selling product 1 to customer Y

geomer198
geomer198 approved these changes Aug 17, 2023
View reviewed changes
Copy link

@geomer198 geomer198 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 ok!

@francesco-ooops
Copy link
Contributor

@pedrobaeza can we merge? We worked hard on this :)

@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 pedrobaeza added this to the 14.0 milestone Aug 17, 2023
pedrobaeza
pedrobaeza reviewed Aug 17, 2023
View reviewed changes
sale_commission_product_criteria_domain/__manifest__.py Outdated
"category": "Sales Management",
"license": "AGPL-3",
"depends": [
"sale_commission_product_criteria_discount",
Copy link
Member

Choose a reason for hiding this comment

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

This dependency shows a non-desired coupling between modules. Maybe this should be split into 2: one for base product criteria, and another for the discounts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats right. I redid it to depend only on sale_commission_product_criteria

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-sale_commission_product_criteria_domain branch from d3c8192 to 08b0857 Compare August 18, 2023 08:55
@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-sale_commission_product_criteria_domain branch 2 times, most recently from cabe534 to 641c2c4 Compare August 22, 2023 09:29
@ilyasProgrammer
Copy link
Member Author

@pedrobaeza Could you merge please

pedrobaeza
pedrobaeza reviewed Aug 22, 2023
View reviewed changes
sale_commission_product_criteria_domain/readme/DESCRIPTION.rst Outdated

Agent A receives commission 20 when selling product 1 to customer Y

This behavior is integrated with both pricelist-like Commission Type structure (sale_commission_product_criteria) and commission by discount (sale_commission_product_criteria_discount)
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect text?

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-sale_commission_product_criteria_domain branch from 641c2c4 to e4a6d48 Compare August 22, 2023 10:52
@pedrobaeza
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-434-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 15d165a into OCA:14.0 Aug 22, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedrobaeza pedrobaeza pedrobaeza left review comments

@geomer198 geomer198 geomer198 approved these changes

@francesco-ooops francesco-ooops francesco-ooops approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.
5 participants
@ilyasProgrammer @francesco-ooops @OCA-git-bot @pedrobaeza @geomer198