base_delivery_carrier_files module migration to Odoo v8

[kkarolis]

No description provided.

[coveralls]

Coverage remained the same when pulling f2d3a2e on kkarolis:base_delivery_carrier_files_migration into 75a0207 on OCA:8.0.

[coveralls]

Coverage remained the same when pulling f2d3a2e on kkarolis:base_delivery_carrier_files_migration into 75a0207 on OCA:8.0.

[yvaucher]

Here you can call

carrier_file._generate_files(picking_ids)

And rewrite _generate_files to use self instead of carrier_file

[yvaucher]

Hello,

Thanks for the work done here. I added few inline comments.
Plus, don't forget to move the module out of __unported__ directory

[pedrobaeza]

@kkarolis any news on @yvaucher's remarks?

[kkarolis]

@pedrobaeza I was busy the whole month but started working on it just yesterday. The problem is that the latest odoo updates broke the module and some unexpected errors showed up. I am working to fix them right now

[pedrobaeza]

OK, just tell me if you need help.

[kkarolis]

There is an issue which affects this module(prevents execution of test/carrier_file_manual.yml). It will work after odoo/odoo#4936 is merged.

[pedrobaeza]

We should make a workaround meanwhile. Can you empty that field in your module?

[kkarolis]

Well, it really is outside the scope of this module. The only workaround we could do is to comment out the test itself.

[pedrobaeza]

If you docummented the reason why you do it (putting that due to the issue #...), I think it can be a solution.

[yvaucher]

./base_delivery_carrier_files/carrier_file.py:105:5: E125 continuation line with same indent as next logical line

Please add 4 spaces here so we can see it is not part of the block

[yvaucher]

Thanks for you changes, I added some more remarks.
About commenting a test, I feel it is quite a regression unless you explain why. I could be comprehensible for a temporary bug on official branch that need fixing with no easy work around.

[gurneyalex]

created @kkarolis in the OCA database, but we don't have a CLA as far as I can tell.

[yvaucher]

Why did you renamed this m2m removing ids?
By convention we use _ids everywhere for o2m and m2m

[kkarolis]

Well the convention was to use ids because it actually was ids previously, now its just a recordset right? I even remember a similar remark regarding this earlier in this MR. Do you still wish me to rename it?

[pedrobaeza]

We have talked about this in some occassions, and the "convention" is not clear. It's very confused to have code like this: self.line_id.order_id.picking_ids.ids. I also prefer this naming convention. I already do in all my new modules. The only thing is that it must have oldname='picking_ids' for easying migration path.

[yvaucher]

@kkarolis The diff is not clean, could you rebase instead of doing merges?

[kkarolis]

Hey, I have made the changes, however I still need some help regarding the travis fails. The issue is the one I mentioned earlier, thanks :)

[yvaucher]

@kkarolis What if you define the state field in yaml file ?

A previous hack which was working in earlier version for computed store fields was to do something like that:
odoo/odoo#3922 (comment)

Otherwise it could be due to selection redefinition: odoo/odoo#2797

[kkarolis]

I have tried, adding state: 'draft' to related pickings without any success. I have also tried inserting the following block model with older api, however the issue still remains.

from openerp.osv import fields


class stock_picking(models.Model):
    _inherit = 'stock.picking'
    _columns = {
        'state': fields.selection([
            ('draft', 'Draft'),
            ('cancel', 'Cancelled'),
            ('waiting', 'Waiting Another Operation'),
            ('confirmed', 'Waiting Availability'),
            ('partially_available', 'Partially Available'),
            ('assigned', 'Ready to Transfer'),
            ('done', 'Transferred')], string='State'),
    }

from openerp import fields

[tremlin]

Thanks for the migration!
I have no idea how to fix the yaml demo error except by reverting the state field of stock.picking completely back to the old API. If you decide to keep the new API, then the _state_get method of stock.picking should have the same signature as the _state_get from the core. That is, it should take additional arguments field_name and arg and it should return a dictionary instead of None. Besides of this incompatibility the code worked fine for me.

[paulius-sladkevicius]

What left to do that we can merge it to 8.0?

[yvaucher]

@komsas There is still the error in tests, it might requires to stick to old API in this case. Plus reporting the issue on Odoo

[oca-clabot]

Hey @kkarolis,
We acknowledge that the following users have signed our Contributor License Agreement:

• @kkarolis

Appreciation of efforts,
OCA CLAbot

[yvaucher]

Can you do a rebase to get travis status ?

[yvaucher]

@kkarolis Sorry I missed your rebase

👍

[mileo]

Hi,

How do you fix undefined method get?

We are having the same issue in OCA/l10n-brazil#342, on sale field amount_untaxed with track_visibility='aways'

Thanks in advance