Merge lp:~savoirfairelinux-openerp/openerp-hr/experience into lp:openerp-hr/6.1
Status: | Rejected |
---|---|
Rejected by: | Sandy Carter (http://www.savoirfairelinux.com) on 2014-07-11 |
Proposed branch: | lp:~savoirfairelinux-openerp/openerp-hr/experience |
Merge into: | lp:openerp-hr/6.1 |
Diff against target: |
771 lines (+709/-0) 12 files modified
hr_experience/__init__.py (+26/-0) hr_experience/__openerp__.py (+59/-0) hr_experience/hr_academic.py (+52/-0) hr_experience/hr_academic_view.xml (+61/-0) hr_experience/hr_certification.py (+50/-0) hr_experience/hr_certification_view.xml (+59/-0) hr_experience/hr_experience_view.xml (+24/-0) hr_experience/hr_professional.py (+49/-0) hr_experience/hr_professional_view.xml (+55/-0) hr_experience/i18n/hr_experience.pot (+208/-0) hr_experience/security/hr_experience_security.xml (+62/-0) hr_experience/security/ir.model.access.csv (+4/-0) |
To merge this branch: | bzr merge lp:~savoirfairelinux-openerp/openerp-hr/experience |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Yannick Vaucher @ Camptocamp | code review, no tests | Needs Fixing on 2014-02-13 | |
Joël Grand-Guillaume @ camptocamp | code review, no tests | Needs Fixing on 2013-12-20 | |
Joao Alfredo Gama Batista | Needs Fixing on 2013-11-15 | ||
Maxime Chambreuil (http://www.savoirfairelinux.com) | code review | 2013-11-12 | Approve on 2013-11-13 |
Review via email:
|
Description of the change
[ADD] add hr_experience module.It adds a new menu in hr module and inherits the employee view form
Hi,
Thanks for the contrib. A few remarks:
* Line 27-29 use from . import XY
* From a security point of view, is the hr_user the right person to create and delete the hr.academic, hr.professionnal, hr.certification object ? I would may be suggest hr_manager instead for that while keeping hr_user the read right, what do you think ?
Regards,
Joël
Hello Joêl,
We want employees to be able to create and manage their own resume, including academic and professional experiences and certifications.
Our next step is to integrate with LinkedIn to allow the employee to import its LinkedIn data :
https:/
Thnaks maxime
Looks good.
Just one thing about security, I think you just need some ir.rule to make sure hr_user1 can't modify hr_user2 experience data.
You might need a user_id field on all those objects.
1 month without activity, I set this as WIP
Unmerged revisions
- 313. By Maxime Chambreuil (http://www.savoirfairelinux.com) on 2014-04-21
-
[ADD] Security rules
- 312. By Maxime Chambreuil (http://www.savoirfairelinux.com) on 2013-12-31
-
[IMP] Use specific term instead of partner.
- 311. By Maxime Chambreuil (http://www.savoirfairelinux.com) on 2013-12-31
-
[IMP] Update translation file
- 310. By Maxime Chambreuil (http://www.savoirfairelinux.com) on 2013-12-23
-
[FIX] Based on MP reviews. Improve XML readability
- 309. By El Hadji Dem (http://www.savoirfairelinux.com) on 2013-12-18
-
[IMP] separate academic, certification and professional experiences, pep8
- 308. By El Hadji Dem (http://www.savoirfairelinux.com) on 2013-12-18
-
[IMP] separate academic, certification and professional experience, pep8
- 307. By EL HADJI DEM <email address hidden> on 2013-11-15
-
[IMP] add pot file
- 306. By EL HADJI DEM <email address hidden> on 2013-11-12
-
[ADD] add hr_experience module
Hi El Hadji,
Thanks for your contribution. As for the review I have this 2 points that you can improve.
l.100.103.132: Even in 6.1 the osv is considered deprecated (osv.osv actually points to orm.Model) so for the import is better to use "from openerp.osv import orm, fields" and for the models it's better to inherit from orm.Model istead of osv.osv.
l.129.137: In 6.1 we don't need to instantiate the model after defining it.