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

[ADD][br_deliverable] move the estimation pricelist from the project to the customer #117

Conversation

victormmtorres
Copy link
Collaborator

Improve customer model to add additional pricelist for estimations

Improve customer model to add additional pricelist for estimations
@oca-clabot
Copy link

Hey @victormartinelicocorp, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

  • Victor Martin (no github login found)

Appreciation of efforts,
OCA CLAbot

@victormmtorres
Copy link
Collaborator Author

@elicoidal @seb-elico this is for the pricelist estimation

Others is weird that ask me to sign again my CLA. What should I do?

@elicoidal
Copy link
Contributor

@jbeficent Any idea about the CLA?

class ResPartner(models.Model):
_inherit = 'res.partner'

pricelist_id = fields.Many2one(
Copy link
Contributor

Choose a reason for hiding this comment

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

should not this be a property (like customer pricelists)?

Copy link
Contributor

Choose a reason for hiding this comment

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

s/pricelist_id/estimation_pricelist_id

Copy link
Collaborator

@seb-elico seb-elico Apr 17, 2017

Choose a reason for hiding this comment

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

@victormartinelicocorp I remember you told me the Sales Pricelist was a computed field. Reason is: when the partner is a contact of a company, his Sales Pricelist is the one from his company. As discussed, we should replicate this behavior.

EDIT: after checking with @elicoidal, we are looking for a slightly different behavior for the estimation pricelist. For the contacts, the sales pricelist is "inherited": it comes from their parent res_partner (e.g. their company). For the estimation pricelist, we don't need to implement this "heritage". But, in multi-company context, we need to be able to define different estimation pricelists for a customer when we work on different companies, this is what @elicoidal means by property (it's a "dynamic" field that can have different value based on the current company).

Copy link
Contributor

Choose a reason for hiding this comment

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

@victormartinelicocorp To be discussed

comodel_name='product.pricelist',
domain=[('type', '=', 'sale')],
help='''Pricelist used for the estimation of the Business Requirements
Deliverables linked to this project.
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "Linked to this project"

@elicoidal elicoidal changed the title [IMP] business_requirement_deliverable [IMP][br_deliverable] move the estimation pricelist from the project to the customer Apr 16, 2017
Copy link
Contributor

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

What about the pricelist calculation methods in the deliverable lines?

class ResPartner(models.Model):
_inherit = 'res.partner'

pricelist_id = fields.Many2one(
Copy link
Contributor

Choose a reason for hiding this comment

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

s/pricelist_id/estimation_pricelist_id

@elicoidal
Copy link
Contributor

elicoidal commented Apr 16, 2017

I am thinking that we should maybe think about migration script but it is complex (cross module).
Nevertheless what I would least do is add a remark in both modules README (Known Issues/Roadmap) that
"As of version 8.0.5.0.0, the estimation pricelist has been moved from Project to Partner object. There is no migration script for the change and the data will have to be migrated manually"

@@ -6,7 +6,7 @@
'category': 'Business Requirements Management',
'summary': 'Manage the Business Requirement Deliverables and \
Resources for your customers',
'version': '8.0.4.0.0',
'version': '8.0.4.0.1',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should bump to 8.0.5.0.0 because this modules implies a manual migration of the data

class ResPartner(models.Model):
_inherit = 'res.partner'

pricelist_id = fields.Many2one(
Copy link
Collaborator

@seb-elico seb-elico Apr 17, 2017

Choose a reason for hiding this comment

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

@victormartinelicocorp I remember you told me the Sales Pricelist was a computed field. Reason is: when the partner is a contact of a company, his Sales Pricelist is the one from his company. As discussed, we should replicate this behavior.

EDIT: after checking with @elicoidal, we are looking for a slightly different behavior for the estimation pricelist. For the contacts, the sales pricelist is "inherited": it comes from their parent res_partner (e.g. their company). For the estimation pricelist, we don't need to implement this "heritage". But, in multi-company context, we need to be able to define different estimation pricelists for a customer when we work on different companies, this is what @elicoidal means by property (it's a "dynamic" field that can have different value based on the current company).

@elicoidal
Copy link
Contributor

I am thinking that we should maybe think about migration script but it is complex (cross module).
Nevertheless what I would least do is add a remark in both modules README (Known Issues/Roadmap) that
"As of version 8.0.5.0.0, the estimation pricelist has been moved from Project to Partner object. There is no migration script for the change and the data will have to be migrated manually"

@victormartinelicocorp
done

@elicoidal
Copy link
Contributor

@victormartinelicocorp if you can fix this one we can merge it

@victormmtorres
Copy link
Collaborator Author

@victormartinelicocorp if you can fix this one we can merge it

@elicoidal I'll try to use 2 hours at end of day if I couldn't dedicate this time I let you know

@seb-elico
Copy link
Collaborator

Here is a list of the users:
Victor Martin (no github login found)

Others is weird that ask me to sign again my CLA. What should I do?

@victormartinelicocorp It's very strange indeed, it looks like the OCA CLA bot didn't detect your GitHub login. This is probably the root cause of this error...

@jbeficent What to you think?

Propery on new API is mean company dependent
@victormmtorres victormmtorres changed the title [IMP][br_deliverable] move the estimation pricelist from the project to the customer [WIP][br_deliverable] move the estimation pricelist from the project to the customer Apr 25, 2017
@elicoidal elicoidal added this to the 8.0 milestone Apr 26, 2017
Add company dependent (property) pricelist for estimation
@victormmtorres
Copy link
Collaborator Author

The remark on known issues is added and also now is company dependent.

@elicoidal @seb-elico Could you review?

@victormmtorres victormmtorres changed the title [WIP][br_deliverable] move the estimation pricelist from the project to the customer [ADD][br_deliverable] move the estimation pricelist from the project to the customer Apr 26, 2017
@elicoidal
Copy link
Contributor

@seb-elico @dreispt @pedrobaeza I think we can move forward

Copy link
Sponsor Member

@dreispt dreispt left a comment

Choose a reason for hiding this comment

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

Other than a couple of nitpicks, LGTM

@@ -16,6 +16,9 @@ Two new concepts complement the main business requirements model:
* Deliverable lines
* Resource lines

A new field for pricelist estimation has been added to the partner to be used in the Deliverable
lines and Resources lines.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Since we are describing the module features, instead of "has been added" I would rather say "is available".

company_dependent=True,
help='''Pricelist used for the estimation of the Business Requirements
Deliverables linked to this project.
Currency of the Deliverables will be the one from this pricelist.''')
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Prefer using triple double quotes instead of triple single quotes.

@elicoidal
Copy link
Contributor

@dreispt I corrected the README and quotes. I move forward :)

@elicoidal elicoidal merged commit 104e840 into OCA:8.0 Apr 27, 2017
ruter-lyu pushed a commit to ruter-lyu/business-requirement that referenced this pull request Mar 15, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
ruter-lyu pushed a commit to ruter-lyu/business-requirement that referenced this pull request Mar 15, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 6, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 21, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 21, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 26, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jul 25, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Aug 9, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Aug 19, 2019
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
Tardo pushed a commit to Tecnativa/business-requirement that referenced this pull request Jan 27, 2020
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
Tardo pushed a commit to Tecnativa/business-requirement that referenced this pull request Mar 3, 2020
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
Tardo pushed a commit to Tecnativa/business-requirement that referenced this pull request Mar 5, 2020
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
Tardo pushed a commit to Tecnativa/business-requirement that referenced this pull request Mar 30, 2020
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victoralmau pushed a commit to Tecnativa/business-requirement that referenced this pull request Nov 11, 2021
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victoralmau pushed a commit to Tecnativa/business-requirement that referenced this pull request Nov 15, 2021
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
victoralmau pushed a commit to Tecnativa/business-requirement that referenced this pull request Nov 15, 2021
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
MosabWadea pushed a commit to MosabWadea/business-requirement that referenced this pull request Mar 5, 2022
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
bizzappdev pushed a commit to BizzAppDev-Systems/business-requirement that referenced this pull request Mar 4, 2023
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
dreispt pushed a commit to dreispt/business-requirement that referenced this pull request Apr 27, 2024
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
dreispt pushed a commit to dreispt/business-requirement that referenced this pull request Apr 27, 2024
…to the customer (OCA#117)

* [IMP] business_requirement_deliverable
Improve customer model to add additional pricelist for estimations

* Update README.rst

* Bumped version number

* [ADD] business_requirement_deliverable
Propery on new API is mean company dependent

* [ADD] business_requirement_deliverable
Add company dependent (property) pricelist for estimation

* Refreshed the README

* Removed the single quotes
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.

None yet

5 participants