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

[ADD][9.0] hr contract operating unit #24

Merged
merged 7 commits into from Mar 27, 2017

Conversation

AaronHForgeFlow
Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow commented Sep 14, 2016

HR Contract with Operating Units

This module introduces the following features:

  • Adds the Operating Unit (OU) to the Employee Contract.
  • Security rules are defined to ensure that users can only see the Contracts of that Operating Units in which they are allowed access to.

@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+2.8%) to 77.778% when pulling 10bdb53 on Eficent:9.0-hr_contract_operating_unit into ae21fc8 on OCA:9.0.

@elicoidal
Copy link

The commit message is misleading (this is not expense but contract model).

Other than that 👍 !
Thanks for the contribution

Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

Commit message is misleading but overall 👍

* Security rules are defined to ensure that users can only see the Contracts of that Operating Units in which they are allowed access to.


Installation

Choose a reason for hiding this comment

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

Remove the following 2 sections if empty

<!-- Copyright 2015 Eficent Business and IT Consulting Services S.L.
Serpent Consulting Services Pvt. Ltd.
License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl-3.0) -->
<openerp>

Choose a reason for hiding this comment

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

change to <odoo>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I do the same in the views files?

@AaronHForgeFlow
Copy link
Contributor Author

I apologize for the commit message. Thank you @elicoidal

@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 77.778% when pulling 2e81b16 on Eficent:9.0-hr_contract_operating_unit into ae21fc8 on OCA:9.0.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 77.778% when pulling 2e81b16 on Eficent:9.0-hr_contract_operating_unit into ae21fc8 on OCA:9.0.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 77.778% when pulling 2e81b16 on Eficent:9.0-hr_contract_operating_unit into ae21fc8 on OCA:9.0.

@coveralls
Copy link

coveralls commented Sep 15, 2016

Coverage Status

Coverage increased (+2.8%) to 77.778% when pulling bcb5000 on Eficent:9.0-hr_contract_operating_unit into ae21fc8 on OCA:9.0.

@AaronHForgeFlow
Copy link
Contributor Author

@elicoidal can you please review again?

Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

👍

This module introduces the following features:

* Adds the Operating Unit (OU) to the Employee Contract.

Choose a reason for hiding this comment

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

remove the empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thank you @elicoidal


* Adds the Operating Unit (OU) to the Employee Contract.

* Security rules are defined to ensure that users can only see the Contracts of that Operating Units in which they are allowed access to.

Choose a reason for hiding this comment

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

non blocking but better to stick to 80 char lines

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@serpentcs-dev1 serpentcs-dev1 left a comment

Choose a reason for hiding this comment

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

code review LGTM 👍

@JordiBForgeFlow JordiBForgeFlow merged commit 140af97 into OCA:9.0 Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants