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

port onchanges to API 8.0 to fix #22 #23

Merged

Conversation

yvaucher
Copy link
Member

@yvaucher yvaucher commented Nov 5, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.63%) when pulling 50a0698 on yvaucher:8.0-framework-agreement-sourcing-fix-onchanges into 3304f44 on OCA:8.0.

@yvaucher
Copy link
Member Author

yvaucher commented Nov 5, 2014

I have to refactor this a bit -> WIP

'total_cost': price * self.proposed_qty,
'supplier_id': agreement.supplier_id.id}
self.write(vals)
res = {}
if not enough_qty:
Copy link
Member Author

Choose a reason for hiding this comment

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

This check seems useless as in framework_agreement get_cheapest_agreement_for_qty only returns an agreement if it found a matching one with enough qty.

@nbessi @lepistone have we somewhere in OCA modules an override of get_cheapest_agreement_for_qty that could return something else than:

(None, None) or (agreement_id, True) ?

Copy link
Member

Choose a reason for hiding this comment

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

@yvaucher I don't know off the top of my head. In any case, ping me if I can help.

@yvaucher yvaucher force-pushed the 8.0-framework-agreement-sourcing-fix-onchanges branch from 9f67ae8 to da16dad Compare November 6, 2014 12:02
This restore former warning message when we can find an agreement with
not enough quantities
This allow to find and finish the framework agreement almost finished.
As it is preferable to finish agreements completely instead of
contracting new ones and be left with multiple remainings of agreements.
@yvaucher yvaucher force-pushed the 8.0-framework-agreement-sourcing-fix-onchanges branch from da16dad to e350d69 Compare November 6, 2014 13:05
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.46%) when pulling e350d69 on yvaucher:8.0-framework-agreement-sourcing-fix-onchanges into 65baeb2 on OCA:8.0.

@foutoucour
Copy link

Do you plan to use openerp.fields classes to have a full port to v8.0?
It seems the new api is getting better and better.

also you are using osv and orm. They are a redondant, aren't they?

@yvaucher
Copy link
Member Author

yvaucher commented Nov 7, 2014

@foutoucour I think porting fields in this PR is out of scope of the fix. And odoo 8.0 old api is not yet deprecated. 😃

This PR does not intend to port the whole code to v8.0. I just fixed the onchanges tooking the opportunity to port them to new API.
I agree that v8.0 api is getting better. Nevertheless, after several issues with it, we decided not to port to v8.0 when not necessary.

About osv and orm:

They are not redundant, as I use direct path for old style fields:
https://github.com/odoo/odoo/blob/8.0/openerp/osv/fields.py

This to use new fields in onchanges. (just seeing I'm calling the wrong datetime BTW)

And didn't changed the old stylish path to Model (which is in fact in models.py now but this is out of scope of this PR)
https://github.com/odoo/odoo/blob/8.0/openerp/osv/orm.py

@lepistone
Copy link
Member

Thanks! 👍

@foutoucour
Copy link

👍
Thanks for the port!

lepistone added a commit that referenced this pull request Nov 10, 2014
…-fix-onchanges

port onchanges to API 8.0 to fix #22
@lepistone lepistone merged commit 9fd6a2a into OCA:8.0 Nov 10, 2014
@yvaucher yvaucher deleted the 8.0-framework-agreement-sourcing-fix-onchanges branch November 10, 2014 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants