Skip to content
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

[MIG] 10.0 Porting hr_experience #306

Merged
merged 3 commits into from
Feb 28, 2017

Conversation

jlaloux
Copy link

@jlaloux jlaloux commented Dec 30, 2016

No description provided.

@pedrobaeza pedrobaeza mentioned this pull request Dec 30, 2016
58 tasks
Copy link

@leemannd leemannd left a comment

Choose a reason for hiding this comment

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

Thank you for the port. Only a few comments. Not tested Yet

access_hr_experience_officer,hr.experience.officer,model_hr_experience,base.group_hr_user,1,1,1,1
access_hr_curriculum_officer,hr_curriculum.officer,model_hr_curriculum,base.group_hr_user,1,1,1,1
access_hr_certification_officer,hr.certification.officer,model_hr_certification,base.group_hr_user,1,1,1,1
"id","name","model_id:id","group_id:id","perm_read","perm_write","perm_create","perm_unlink"

Choose a reason for hiding this comment

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

These quotes are usesless. Can you remove them?

<!-- inheritEmployee -->
<record model="ir.ui.view" id="view_employee_form_inherit">
<field name="name">hr.experience.employee.form</field>
<field name="model">hr.employee</field>
<field name="inherit_id" ref="hr.view_employee_form"/>
<field name="arch" type="xml">
<notebook position="inside">
<page string="Academic Experiences" groups="base.group_user,base.group_hr_user">
<page string="Academic Experiences" groups="base.group_user,hr.group_hr_user">

Choose a reason for hiding this comment

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

As 'hr.group_hr_user' has the base rights from 'base.group_user' it can be removed.

@@ -40,7 +41,7 @@
</form>
</field>
</page>
<page string="Professional Experiences" groups="base.group_user,base.group_hr_user">
<page string="Professional Experiences" groups="base.group_user,hr.group_hr_user">

Choose a reason for hiding this comment

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

As 'hr.group_hr_user' has the base rights from 'base.group_user' it can be removed

@@ -64,7 +65,7 @@
</form>
</field>
</page>
<page string="Certifications" groups="base.group_user,base.group_hr_user">
<page string="Certifications" groups="base.group_user,hr.group_hr_user">

Choose a reason for hiding this comment

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

As 'hr.group_hr_user' has the base rights from 'base.group_user' it can be removed

@leemannd
Copy link

@jlaloux After functional testing, it is working properly but there are two minor issue to me.
When entering the module, the base kanban view of employee is replaced by the base tree view of 'Academic Experience'. The menu entry 'Academic' is placed first. I feel like it should be placed after 'Employee' or 'Departments'.

@feketemihai feketemihai added this to the 10.0 milestone Feb 23, 2017
@feketemihai
Copy link
Member

@jlaloux Thanks for your contribution, can you please update @leemannd comments, and please add a sequence to the menu to go after departments...it's a little bit starnge to be the first menu...

Copy link

@leemannd leemannd left a comment

Choose a reason for hiding this comment

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

Just a few last formal corrections. Otherwise LGTM 👍

@@ -2,14 +2,16 @@
# © 2013 Savoir-faire Linux (<http://www.savoirfairelinux.com>).

Choose a reason for hiding this comment

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

s/©/Copyright/

@@ -2,12 +2,12 @@
# © 2013 Savoir-faire Linux (<http://www.savoirfairelinux.com>).

Choose a reason for hiding this comment

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

s/©/Copyright/

@@ -2,18 +2,18 @@
# © 2013 Savoir-faire Linux (<http://www.savoirfairelinux.com>).

Choose a reason for hiding this comment

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

s/©/Copyright/

@@ -2,10 +2,10 @@
# © 2013 Savoir-faire Linux (<http://www.savoirfairelinux.com>).

Choose a reason for hiding this comment

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

s/©/Copyright/

@@ -2,10 +2,10 @@
# © 2013 Savoir-faire Linux (<http://www.savoirfairelinux.com>).

Choose a reason for hiding this comment

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

s/©/Copyright/

@@ -83,7 +83,7 @@ help us smashing it by providing a detailed and welcomed `feedback
<https://github.com/OCA/

Choose a reason for hiding this comment

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

It can be removed

@@ -4,14 +4,14 @@

Choose a reason for hiding this comment

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

s/©/Copyright/

@leemannd
Copy link

@jlaloux Thanks!

@jlaloux
Copy link
Author

jlaloux commented Feb 24, 2017

@leemannd With pleasure

@gurneyalex gurneyalex merged commit 8c5b23c into OCA:10.0 Feb 28, 2017
sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
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.

None yet

4 participants