Merge lp://staging/~therp-nl/openobject-server/7.0-lp1111298-remove_additional_company_assignments into lp://staging/openobject-server/7.0

Proposed by Stefan Rijnhart (Opener)
Status: Merged
Merged at revision: 4998
Proposed branch: lp://staging/~therp-nl/openobject-server/7.0-lp1111298-remove_additional_company_assignments
Merge into: lp://staging/openobject-server/7.0
Diff against target: 35 lines (+0/-7)
2 files modified
openerp/addons/base/base_data.xml (+0/-6)
openerp/addons/base/currency_data.xml (+0/-1)
To merge this branch: bzr merge lp://staging/~therp-nl/openobject-server/7.0-lp1111298-remove_additional_company_assignments
Reviewer Review Type Date Requested Status
Olivier Dony (Odoo) Needs Fixing
Review via email: mp+166211@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hi Stefan,

Thanks for catching this missing part of the original fix! It's my fault for not reviewing it more carefully.

Your fix looks good, however it breaks the ORM test suite related to one2many fields. This happens because those tests are using "res.company.currency_ids" as an example o2m field, and are strongly coupled with res.currency default data :-(

You can see the result of the test failure on the runbot log (search for "list index out of range"):
   http://runbot.openerp.com/openerp-dev-7-0-bug-1111298-therp-11987/logs/test-all.txt

If you have some more time to spend on this, you should be able to fix it by setting a company on the res.currency records just before the relevant tests. That change will be rolled back at the end of the tests. Alternatively you can create extra test data or change the tests to use a different o2m field from any of the `base` models.
(Otherwise I'll try to fix it while merging)

Many thanks!

PS: you can have your merge proposals automatically tested by runbot if you subscribe the ~therp-nl team in runbot (the link to do so is in the partners portal).

review: Needs Fixing
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I ended up fixing the tests while merging, thanks!

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks, sorry I did not get round to fixing the tests myself.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Don't be sorry, it's great that you provided the patch in first place :-)
I tried not to make the ownership of the patch lines confusing, by explicitly making an extra commit in a temporary branch before merging the whole thing at once.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.