-
-
Notifications
You must be signed in to change notification settings - Fork 673
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
[18.0][MIG] hr_employee_firstname: Migration to 18.0 #1397
[18.0][MIG] hr_employee_firstname: Migration to 18.0 #1397
Conversation
* remove ensure one * Add decorator * Update name on write and update related partner firstname lastname * Modify README file * Change version format * Consistency with partner_firstname module * Check if the partner isn't already in the list * Remove check on partner_firstname installation
… value not the key of dict (OCA#215) * Update split names at module install to get the value not the key of the dict. * Add test for checking right values in firstname, lastname after install
This issue was flagged by travis in the following build https://travis-ci.org/OCA/hr/jobs/364219109
``sudo`` is used to avoid access error when verify that module is installed. Avoid error in the next case: 1. Create a new user without settings access (HR User) 2. Login with the user in 1, and try to edit an employee
Starting from 14.0, employee model was split into three models: - `hr.employee.public` - `hr.employee.private` - `hr.employee` (a common model) When this module was migrated, such split was not taken into account. This commit moves fields to the common model, so they don't fail when read from the public employee model. Closes OCA#990 Co-Authored-By: Luis González <[email protected]>
Avoid WARNING on inherit to create method: ``` The model odoo.addons.hr_employee_firstname.models.hr_employee is not overriding the create method in batch ```
2ccd6b6
to
7576692
Compare
Hi @luistorresm @luisg123v could you check this, please? Regards. |
LGTM 👍 |
/ocabot migration hr_employee_firstname |
Sorry @luisg123v you are not allowed to mark the addon tobe migrated. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
@@ -5,7 +5,7 @@ | |||
import odoo | |||
from odoo.exceptions import ValidationError | |||
from odoo.tests.common import TransactionCase | |||
from odoo.tools import submap | |||
from odoo.tools.misc import submap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
I couldn't find any related change that doesn't allow the previous syntax, and I think importing methods directly from odoo.tools
is preferred.
If really required, it might be a good idea to document it in the commit description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I ran the tests to verify that they worked fine I got this error:
File "/home/odoo/instance/extra_addons/hr/hr_employee_firstname/tests/__init__.py", line 3, in <module>
from . import test_hr_employee_firstname
File "/home/odoo/instance/extra_addons/hr/hr_employee_firstname/tests/test_hr_employee_firstname.py", line 8, in <module>
from odoo.tools import submap
ImportError: cannot import name 'submap' from 'odoo.tools' (/home/odoo/instance/odoo/odoo/tools/__init__.py)
I tried to import the tools library using from odoo import tools
and then tools.submap
but got the same error.
Hi @legalsylvain, @pedrobaeza, I'm not able to mark this PR as a migration one, despite being maintainer, see #1397 (comment). Could it be OCA/oca-github-bot#226 is not fully addressed? CC @yajo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
/ocabot merge nobump |
Sorry @luisg123v you are not allowed to merge. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
/ocabot merge nobump |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at eece4fb. Thanks a lot for contributing to OCA. ❤️ |
/ocabot migration hr_employee_firstname |
Sorry @luisg123v you are not allowed to mark the addon tobe migrated. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
/ocabot migration hr_employee_firstname |
No description provided.