Skip to content

Conversation

@hugosantosred
Copy link
Member

Migrated module magentoerpconnect_order_comment to version 8.0 with new API

Migrated module magentoerpconnect_order_comment to version 8.0 with new API
Copy link
Member

Choose a reason for hiding this comment

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

Help messages are automatically translated, you should not use _() on them.
(I know it was wrong before but it is the right opportunity to correct it)

@guewen
Copy link
Member

guewen commented Nov 17, 2015

Only a small remark regarding a correction with _(), apart that, that's fine!

Thanks
👍

@hugosantosred
Copy link
Member Author

I didn't know that about help on fields and translations. Thanks! @guewen

'Store id',
help=MAGENTO_HELP)

@api.model
Copy link
Member

Choose a reason for hiding this comment

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

On the create method, you need to apply the @api.returns('self', lambda value: value.id) decorator for backwards compatibility with old API overrides of this method (you know, just in case...)

Copy link
Member

Choose a reason for hiding this comment

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

I was sure it was inherited. This comment would tend to confirm that, but it could be wrong https://github.com/odoo/odoo/blob/9.0/openerp/api.py#L77
Had you ever had issues with that?

Copy link
Member

Choose a reason for hiding this comment

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

You are right: I can't seem to reproduce any such issues with a quick test. Thanks for the pointer!

@StefanRijnhart
Copy link
Member

Can you catch ImportError for the import of bs4 and then at runtime (in a method where you use the import) check if bs4 is defined, and raise then?

try:
    from bs4 import BeautifulSoup
except ImportError:
    BeautifulSoup = False

...

    if not BeautifulSoup:
        raise ....
    return {'comment': BeautifulSoup(comment).get_text()}

@StefanRijnhart
Copy link
Member

You seem to miss 0ead0327ecc2, and I think you would need to take the i18n directory as well.

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.

3 participants