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

[10.0] [ADD] hr_expense_operating_unit #84

Merged

Conversation

nikul-serpentcs
Copy link
Member

HR Expense with Operating Units

This module introduces the following features:

  • Adds the Operating Unit (OU) to the Expense.

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

  • Adds Operating Unit (OU) to the account moves while generating accounting entries from the expense.

@nikul-serpentcs
Copy link
Member Author

@aheficent Could you please review.

@AaronHForgeFlow
Copy link
Contributor

@nikul-serpentcs Will you include here the commit history of https://github.com/OCA/operating-unit/pull/30/commits ?

@nikul-serpentcs
Copy link
Member Author

sure @aheficent

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

Please include the commit history here #30

self.hr_expense2 = self._create_hr_expense(
self.b2c, self.emp)

self._post_journal_entries(self.hr_expense1)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ok. However, I miss a test that check the operating units for the journal items created.

@AaronHForgeFlow
Copy link
Contributor

Thank you @nikul-serpentcs I'll approve this when the commit history is fixed

@nikul-serpentcs
Copy link
Member Author

@aheficent but still open branch here #30
if it is merged, I will fix it.

@nikul-serpentcs
Copy link
Member Author

@aheficent I somehow managed to preserve the commits 😄

becoming GIT Guru 🙏

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow 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 attending the comments

Copy link
Contributor

@MiquelRForgeFlow MiquelRForgeFlow 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.

Functional and Code Review LGTM 👍

@AaronHForgeFlow
Copy link
Contributor

Don't panic, travis stopped but it is not red :) I restarted the job

@AaronHForgeFlow AaronHForgeFlow merged commit ee45151 into OCA:10.0 Jun 27, 2018
@nikul-serpentcs nikul-serpentcs deleted the 10.0-hr_expense_operating_unit branch June 28, 2018 04:11
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.

6 participants