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

Master payroll auto loading lock conflict sed #31837

Conversation

Projects
None yet
5 participants
@sed-odoo
Copy link
Contributor

commented Mar 14, 2019

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@sed-odoo sed-odoo requested a review from tivisse Mar 14, 2019

@robodoo robodoo added the CI 🤖 label Mar 14, 2019

@tivisse
Copy link
Contributor

left a comment

Small comments ;)

Thanks.

return True
return False

no_conflict_benefits = benefits.filtered(lambda x: not x._check_if_error())

This comment has been minimized.

Copy link
@tivisse

tivisse Mar 14, 2019

Contributor

No break the batch method.

Maybe modify _check_if_error() to return (bool, benefits_in_error) and avoid to use a filtered.

@@ -100,3 +100,4 @@ def _format_datetime(date):

new_benefits = self.env['hr.benefit']._safe_duplicate_create(vals_list, date_start, date_stop)
new_benefits._compute_conflicts_leaves_to_approve()
new_benefits.action_validate(new_benefits.ids)

This comment has been minimized.

Copy link
@tivisse

tivisse Mar 14, 2019

Contributor

Let's modify action_validate to use recordsets.

reload: function () {
var date_fmt = 'YYYY-MM-DD HH:mm:ss';
var self = this;
this.firstDay = this.model.data.target_date.clone().startOf('month');

This comment has been minimized.

Copy link
@tivisse

tivisse Mar 14, 2019

Contributor

Seem louche, to check with someone smarter than me.

model: 'hr.employee',
method: 'generate_benefit',
args: [this.firstDay.format(date_fmt), this.lastDay.format(date_fmt)],
}).then(function () {

This comment has been minimized.

Copy link
@tivisse

tivisse Mar 14, 2019

Contributor

Rebase and adapt to new Promise mechanism.

/**
* @override
*/
start: function () {

This comment has been minimized.

Copy link
@tivisse

tivisse Mar 14, 2019

Contributor

Try to extract the date manipulation + rpc code to a specific method and reuse it in the start and the reload

@C3POdoo C3POdoo added the RD label Mar 14, 2019

@sed-odoo sed-odoo force-pushed the odoo-dev:master-payroll-auto-loading-lock-conflict-sed branch from f25db72 to 04a9445 Mar 20, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 20, 2019

@@ -81,11 +81,11 @@ def _check_if_error(self):
undefined_type.write({'display_warning': True})
conflict = self._mark_conflicting_benefits(min(self.mapped('date_start')), max(self.mapped('date_stop')))
conflict_with_leaves = self._compute_conflicts_leaves_to_approve()
return undefined_type or conflict or conflict_with_leaves
return undefined_type + conflict + conflict_with_leaves

This comment has been minimized.

Copy link
@LucasLefevre

LucasLefevre Mar 21, 2019

Contributor

What about union instead of concatenation?

This comment has been minimized.

Copy link
@sed-odoo

sed-odoo Mar 21, 2019

Author Contributor

Good idea, it will be corrected in next rebase
Thanks

@sed-odoo sed-odoo closed this Mar 26, 2019

@sed-odoo sed-odoo force-pushed the odoo-dev:master-payroll-auto-loading-lock-conflict-sed branch from 04a9445 to 3cbb8d5 Mar 26, 2019

@robodoo robodoo added closed 💔 and removed CI 🤖 labels Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.