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.

Sign up for GitHub

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

Jump to bottom

[10.0][FIX]hr_period #527

Merged
merged 4 commits into from
Dec 14, 2018
Merged

[10.0][FIX]hr_period #527

merged 4 commits into from
Dec 14, 2018

Conversation

AaronHForgeFlow
Copy link

@AaronHForgeFlow AaronHForgeFlow commented Dec 11, 2018 •
edited
Loading

Actions done:

  • Add menu to edit date_range_type for hr periods and hr fiscal years
  • Add security rules to allow to access only periods from your companies
  • Create fiscal years according your company
  • Creating periods according the company of the type of the fiscal year
  • Minor fixes on test

This fixes also travis in OCA/hr 10.0 branch

IMO ready to review now.

Regards.

@AaronHForgeFlow
Copy link
Author

AaronHForgeFlow commented Dec 11, 2018 •
edited
Loading

OCA/hr 10.0 branch is red because of this. Ping @OCA/human-resources-maintainers

@pedrobaeza pedrobaeza added this to the 10.0 milestone Dec 11, 2018
pedrobaeza
pedrobaeza previously requested changes Dec 11, 2018
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

This is not the best way to fix the tests, as this can lead to other side effects with demo data being manipulated by other modules through XML files with data. Please fix company creation instead. One possibility is to add post_install = True and at_install = False.

hr_period/__manifest__.py Outdated Show resolved Hide resolved
@AaronHForgeFlow AaronHForgeFlow force-pushed the 10.0-fix-hr_period branch 3 times, most recently from fd6baba to d28c3c7 Compare December 12, 2018 12:59
@AaronHForgeFlow
Copy link
Author

AaronHForgeFlow commented Dec 12, 2018 •
edited
Loading

Ready to review now

@AaronHForgeFlow
Copy link
Author

@pedrobaeza can you update your review plz

@pedrobaeza pedrobaeza dismissed their stale review December 13, 2018 09:01

Not aware of the features

@pedrobaeza
Copy link
Member

I dismiss the review for now, but one extra comment.

nikul-serpentcs
nikul-serpentcs reviewed Dec 13, 2018
View reviewed changes
Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

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

Minor Change

hr_period/security/hr_period_security.xml Outdated Show resolved Hide resolved
@pedrobaeza
Copy link
Member

Please check Travis status

pedrobaeza
pedrobaeza approved these changes Dec 14, 2018
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

I'm going to fast-track it for greenifying the branch.

@pedrobaeza pedrobaeza merged commit 0e85f0e into OCA:10.0 Dec 14, 2018
@AaronHForgeFlow
Copy link
Author

Thank you!

@AaronHForgeFlow AaronHForgeFlow deleted the 10.0-fix-hr_period branch December 14, 2018 08:48
@MiquelRForgeFlow
Copy link
Contributor

Squash! 💩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikul-serpentcs nikul-serpentcs nikul-serpentcs left review comments

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@AaronHForgeFlow @pedrobaeza @MiquelRForgeFlow @nikul-serpentcs