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] [11.0] Migrate hr_payroll_cancel to 11.0 #436

Merged
merged 7 commits into from
Jun 22, 2018

Conversation

novawish
Copy link

@novawish novawish commented Apr 3, 2018

No description provided.

@pedrobaeza pedrobaeza added this to the 11.0 milestone Apr 3, 2018
@pedrobaeza pedrobaeza mentioned this pull request Apr 3, 2018
71 tasks
Copy link

@jcdrubay jcdrubay left a comment

Choose a reason for hiding this comment

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

Approved except for a typo.

for payslip in self:
if payslip.refunded_id and payslip.refunded_id.state != 'cancel':
raise ValidationError(_("""To cancel the Original Payslip the
Refunded Payslip needed to be canceled first!"""))
Copy link

Choose a reason for hiding this comment

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

typo: Replace needed by needs

#, python-format
msgid ""
"To cancel the Original Payslip the\n"
" Refunded Payslip needed to be canceled first!"
Copy link

Choose a reason for hiding this comment

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

needs > need

#, python-format
msgid ""
"To cancel the Original Payslip the\n"
" Refunded Payslip needed to be canceled first!"
Copy link

Choose a reason for hiding this comment

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

needed > needs

@gabrielo77
Copy link

Why is it writing the state on a single record (payslip) instead of the whole RecordSet (self) ?

@novawish
Copy link
Author

@gabrielo77 Good question, IMO probably the original author's intention is that action always works for only one single record (since the button only appears on form view, and he returned right after the first loop). You are right actually, we should treat it like the whole recordsets no matter what because we were using api.multi.

I did a fix based on your PR, may I add you as a contributor for this module also?

@gabrielo77
Copy link

Sure, add me.

I'm currently working on the test cases for this module. Hope I can push it ASAP so we can have this merged.

@gustavovalverde
Copy link
Member

This seems ready to merge, or is it codecov a needed check?

Copy link

@huyly0909 huyly0909 left a comment

Choose a reason for hiding this comment

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

Everything looks ok.

@pedrobaeza pedrobaeza merged commit cc71bcc into OCA:11.0 Jun 22, 2018
sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
[BSSFL-533] Repair line on Sale order line
Mraimou pushed a commit to camptocamp/hr that referenced this pull request Oct 9, 2020
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