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

[WIP] 9.0 migration of hr_holidays_legal_leave #267

Merged
merged 5 commits into from
Oct 27, 2016

Conversation

gurneyalex
Copy link
Member

No description provided.

@gurneyalex gurneyalex added this to the 9.0 milestone Oct 19, 2016
@pedrobaeza pedrobaeza mentioned this pull request Oct 19, 2016
65 tasks
@gurneyalex gurneyalex force-pushed the 9.0-mig_hr_holidays_legal_leave branch from 967626c to 854038f Compare October 19, 2016 13:12
@feketemihai
Copy link
Member

@damdam-s @gurneyalex Thank for update the module, can i ask to update readme, since the HR config isn't there anymore with the text from version 10.

Configuration

To configure this module, you need to:

Company setup * Go to the company and select the leave type to use as legal leave

with

Configuration

To configure this module, you need to:

assign legal leave type via Leaves > Configuration:
choose Leave type you need
click [Edit]
set Legal/Annual checkbox
click [Save]

@damdam-s
Copy link
Member

@feketemihai no problem. I do it right now

@feketemihai
Copy link
Member

👍

class HolidaysType(models.Model):
_inherit = "hr.holidays.status"

is_annual = fields.Boolean(
Copy link
Member

Choose a reason for hiding this comment

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

Although I know this is what is now in v10, I'm seeing that this is not comfortable for user with access to several companies. We should have a company_id field that is filled by default with user company, but with the possibility to be switched. This is similar to Accounting settings.

Copy link
Member

Choose a reason for hiding this comment

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

@pedrobaeza you're right (I think) but is it correct to have a different behaviour between 9 and 10 versions ?
If you want me to do it, I can but I will make a PR on v10 too to have the same behaviour.

@gurneyalex @feketemihai cc

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please. I think it's a good change. We can cherry-pick the commit directly in v10 when done in v9.

Copy link
Member

Choose a reason for hiding this comment

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

@pedrobaeza done in df9830f. Is it what you wanted ?

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.

Yeah, thanks

@pedrobaeza pedrobaeza merged commit 47b1d72 into OCA:9.0 Oct 27, 2016
@damdam-s
Copy link
Member

great! thanks

@pedrobaeza
Copy link
Member

Ouch, it lacks the company_id field in the view. I'll add it directly

@damdam-s
Copy link
Member

aww sorry for that :(

@pedrobaeza
Copy link
Member

pedrobaeza commented Oct 27, 2016

I have include both commits in v10, cc @yelizariev

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