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

[ADD] beesdoo_shift: next_shift_id on cooperative.status #273

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

robinkeunen
Copy link
Member

@robinkeunen robinkeunen commented Feb 14, 2022

  • add a boolean : worker subscribed before alert date
  • add next shift and next shift date to the worker status form

I needed to add shift_shift_ids to res.partner to trigger the computation. I hope it does not impact performance, maybe @tfrancoi knows.

  • Question to @remytms : should I make my function depend on today field ?
  • I could not write tests for is_subscribed_to_shift computation since the `next_alert_date" is computed in overriding modules.

task

@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch 3 times, most recently from 80c3cad to a6cf19d Compare February 18, 2022 10:16
def _compute_next_shift(self):
now = fields.Datetime.now()
for rec in self:
rec.next_shift_id = self.env["beesdoo.shift.shift"].search(
Copy link
Contributor

Choose a reason for hiding this comment

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

The search should be outside the loop, this kind of code is the root of performance issue
search worker_id in self.mapped('cooperator_id').ids and store the result in a dict
You will send only one query for instead of one for each record.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks. Something like this then ?

    def _compute_next_shift(self):
        now = fields.Datetime.now()
        cooperator_ids = self.mapped("cooperator_id").ids
        # avoid searching for shift in loop
        next_shifts = self.env["beesdoo.shift.shift"].search(
            [("start_time", ">=", now), ("worker_id", "in", cooperator_ids)],
            order="worker_id, start_time",
        )

        next_shift_by_worker = {}
        for shift in next_shifts:
            #  take fist shift for each worker
            if shift.worker_id not in next_shift_by_worker:
                next_shift_by_worker[shift.worker_id] = shift

        for rec in self:
            rec.next_shift_id = next_shift_by_worker[rec.cooperator_id]

Thanks for the review

Copy link
Member

@victor-champonnois victor-champonnois left a comment

Choose a reason for hiding this comment

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

FYI Read the code (more for learning than a proper review :) )

beesdoo_shift/models/cooperative_status.py Show resolved Hide resolved
@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch 2 times, most recently from e0ac974 to e54d6d9 Compare February 18, 2022 15:46
@robinkeunen robinkeunen requested review from tfrancoi and victor-champonnois and removed request for victor-champonnois February 18, 2022 16:40
@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch 2 times, most recently from bb605b6 to 2aef4b6 Compare February 21, 2022 11:00
@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch 2 times, most recently from d54907c to 49ac63b Compare March 4, 2022 14:12
Copy link
Collaborator

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

Some comments. Looks good overall. Tests are 👌

beesdoo_shift/models/cooperative_status.py Outdated Show resolved Hide resolved
beesdoo_shift/models/cooperative_status.py Outdated Show resolved Hide resolved
beesdoo_shift/models/res_partner.py Show resolved Hide resolved
@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch 2 times, most recently from 903767d to 32f927c Compare March 4, 2022 15:47
@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch from 32f927c to f78d3a1 Compare March 15, 2022 14:01
@robinkeunen robinkeunen force-pushed the 12.0-IC003-irregular-email-alert branch 3 times, most recently from c6b4c08 to 07e2fd6 Compare March 25, 2022 15:38
@remytms remytms force-pushed the 12.0-IC003-irregular-email-alert branch from 07e2fd6 to 18cba2e Compare April 14, 2022 17:08
Copy link
Collaborator

@remytms remytms left a comment

Choose a reason for hiding this comment

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

LGTM

@remytms remytms force-pushed the 12.0-IC003-irregular-email-alert branch from 18cba2e to 104279b Compare April 14, 2022 17:13
@remytms remytms merged commit ab3d910 into 12.0 Apr 14, 2022
@remytms remytms deleted the 12.0-IC003-irregular-email-alert branch April 14, 2022 17:20
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.

5 participants