[14.0][IMP] hr_timesheet_purchase_order: create PO automatically #597

dessanhemrayev · 2023-07-06T00:40:24Z

Create PO automatically

hr_timesheet_purchase_order/models/hr_employee_base.py

geomer198

I can't understand for what you use compute method in all fields.
Readme need added.
For functions need add comments and docstrings.
Test coverage not cover complex methods need fixed it.

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

hr_timesheet_purchase_order/models/hr_employee_base.py

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

geomer198

Please check comment

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

geomer198

LGTM!

[FIX]hr_timesheet_purchase_order:change field type Change field type `selection` to `integer` Add new method border min max and chech month `february` Fix and update tests [FIX] hr_timesheet_purchase_order: add method

OCA-git-bot · 2024-02-13T11:00:02Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

geomer198

Please fix your code.

geomer198 · 2024-02-15T17:12:48Z

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

+
+    @api.constrains("repeat_day", "repeat_month")
+    def _check_repeat_day_or_month(self):
+        if 0 > self.repeat_day or self.repeat_day > 31:


Not all months have 31 days. Please add condition for this.

okay, I'll fix it

geomer198 · 2024-02-15T17:16:05Z

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

+        return [DAYS.get(self.repeat_weekday)(n)]
+
+    @api.model
+    def _get_next_recurring_dates(


This method have so many arguments. Please use dict or recurrence record for this. For example:

def _get_next_recurring_dates(self, repeat_struct):

repeat_struct is a dict or recurrence record.
This solution is easier and compact.

Thanks, I'll fix it

geomer198 · 2024-02-15T17:18:38Z

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

+        repeat_week,
+        repeat_month,
+        **kwargs
+    ):


Please add docstring for arguments.

okay, I'll add it

geomer198 · 2024-02-15T17:20:42Z

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

+}
+
+
+class HRTimeSheetRecurrence(models.Model):


Please add docstring for each method with describe arguments.

okay, I'll add it

geomer198 · 2024-02-15T17:32:49Z

hr_timesheet_purchase_order/models/res_partner.py

+        readonly=False,
+    )
+    repeat_month = fields.Selection(
+        [


The same selection in prev model, would you create variable for this selection and use it in fields?

I can't do this because all these fields are calculated or modified by the user

Selection argument is static data. Just create variable MONTH_SELECTION

MONTH_SELECTION = [ ("january", "January"), ("february", "February"), .... ] ... repeat_month = fields.Selection(selection=MONTH_SELECTION)

The same for week day field

geomer198 · 2024-02-15T17:33:08Z

hr_timesheet_purchase_order/models/res_partner.py

+    )
+    repeat_weekday = fields.Selection(
+        [
+            ("mon", "Monday"),


The same selection in prev model, would you create variable for this selection and use it in fields?

I can't do this because all these fields are calculated or modified by the user

geomer198 · 2024-02-15T17:33:26Z

hr_timesheet_purchase_order/models/res_partner.py

+        if self.repeat_month == "february" and self.repeat_day > 29:
+            self.repeat_day = 28
+
+    repeat_week = fields.Selection(


The same selection in prev model, would you create variable for this selection and use it in fields?

I can't do this because all these fields are calculated or modified by the user

geomer198 · 2024-02-15T17:36:58Z

hr_timesheet_purchase_order/tests/test_create_not_product.py

+from .common_po_recurrence import TestTimesheetPOrecurrenceCommon
+
+
+class TestTimesheetPORecurrenceNotProduct(TestTimesheetPOrecurrenceCommon):


Only one test on the all model? Would you please upped test coverage and add docstrings and assertions messages.

I understand that it is unnecessary to use an entire class for one test, but this requires the test to cover other functions of this module. And what kind of testing options can there be when the most important thing of this module is not selected, what is it intended for?)

I'll add docstrings and assertions messages.

geomer198 · 2024-02-15T17:38:20Z

hr_timesheet_purchase_order/tests/test_create_timesheet_po_recurrence.py

+from .common_po_recurrence import TestTimesheetPOrecurrenceCommon
+
+
+class TestTimesheetPOrecurrence(TestTimesheetPOrecurrenceCommon):


Would you add docstrings and assertions messages

okay, I'll add it

geomer198 · 2024-02-17T07:05:19Z

hr_timesheet_purchase_order/models/res_partner.py

+        readonly=False,
+    )
+    repeat_month = fields.Selection(
+        [


Selection argument is static data. Just create variable MONTH_SELECTION

MONTH_SELECTION = [ ("january", "January"), ("february", "February"), .... ] ... repeat_month = fields.Selection(selection=MONTH_SELECTION)

The same for week day field

alexey-pelykh

Can't agree with amount of hardcoded calendar info

alexey-pelykh · 2024-02-17T07:38:48Z

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

+from odoo import _, api, fields, models
+from odoo.exceptions import ValidationError
+
+MONTHS = {


This hardcoded part should be used from corresponding library, calendar.monthrange

Thanks, for feedback, I'll fix it

alexey-pelykh · 2024-02-17T07:39:01Z

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py

+    )
+    repeat_weekday = fields.Selection(
+        [
+            ("mon", "Monday"),


import locale locale.nl_langinfo(locale.DAY_1)

don't multiply static text without need.

… values

geomer198

LGTM!

…ners

OCA-git-bot · 2024-03-18T10:32:52Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

francesco-ooops · 2024-03-18T10:33:59Z

@alexey-pelykh shall we merge? thanks!

alexey-pelykh · 2024-03-18T10:34:57Z

/ocabot merge minor

OCA-git-bot · 2024-03-18T10:35:04Z

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-597-by-alexey-pelykh-bump-minor, awaiting test results.

OCA-git-bot · 2024-03-18T10:46:55Z

Congratulations, your PR was merged at 017f2f2. Thanks a lot for contributing to OCA. ❤️

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch 4 times, most recently from 0645995 to ab44ec4 Compare July 20, 2023 00:10

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch 10 times, most recently from e051a34 to b1e89ad Compare August 3, 2023 20:12

dessanhemrayev marked this pull request as ready for review August 3, 2023 20:13

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch 3 times, most recently from b5ee6ce to c825a10 Compare August 4, 2023 00:20

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch 3 times, most recently from a74f9e8 to 26c69a3 Compare August 15, 2023 18:30

geomer198 reviewed Aug 20, 2023

View reviewed changes

hr_timesheet_purchase_order/models/hr_employee_base.py Outdated Show resolved Hide resolved

geomer198 reviewed Aug 20, 2023

View reviewed changes

hr_timesheet_purchase_order/models/hr_employee_base.py Outdated Show resolved Hide resolved

geomer198 reviewed Aug 20, 2023

View reviewed changes

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch 3 times, most recently from 8d75845 to 751566c Compare October 17, 2023 20:39

geomer198 reviewed Oct 18, 2023

View reviewed changes

hr_timesheet_purchase_order/models/hr_timesheet_recurrence.py Show resolved Hide resolved

geomer198 approved these changes Oct 20, 2023

View reviewed changes

[IMP] hr_timesheet_purchase_order: create PO automatically

1242191

[FIX]hr_timesheet_purchase_order:change field type Change field type `selection` to `integer` Add new method border min max and chech month `february` Fix and update tests [FIX] hr_timesheet_purchase_order: add method

OCA-git-bot added approved ready to merge labels Feb 13, 2024

geomer198 suggested changes Feb 15, 2024

View reviewed changes

OCA-git-bot removed approved ready to merge labels Feb 15, 2024

dessanhemrayev added 2 commits February 16, 2024 23:08

[FIX] hr_timesheet_purchase_order: add maintainers

c1aab2a

[FIX] hr_timesheet_purchase_order: refactoring of the code

3fc3d08

geomer198 suggested changes Feb 17, 2024

View reviewed changes

alexey-pelykh requested changes Feb 17, 2024

View reviewed changes

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch 2 times, most recently from c200073 to 0aa1fbe Compare February 18, 2024 14:38

[FIX] hr_timesheet_purchase_order: Placed in a separate file constant…

590052d

… values

dessanhemrayev force-pushed the 14.0-t2676-hr_timesheet_purchase_order-auto_create_po branch from 0aa1fbe to 590052d Compare February 18, 2024 14:47

dessanhemrayev requested review from geomer198 and alexey-pelykh February 20, 2024 18:41

geomer198 approved these changes Feb 21, 2024

View reviewed changes

[FIX] hr_timesheet_purchase_order: remove cetmixgitdrone from maintai…

5fbec0c

…ners

alexey-pelykh approved these changes Mar 18, 2024

View reviewed changes

OCA-git-bot added approved ready to merge labels Mar 18, 2024

OCA-git-bot added the bot is merging ⏳ label Mar 18, 2024

OCA-git-bot merged commit d602ed5 into OCA:14.0 Mar 18, 2024
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Mar 18, 2024

		from .common_po_recurrence import TestTimesheetPOrecurrenceCommon


		class TestTimesheetPORecurrenceNotProduct(TestTimesheetPOrecurrenceCommon):

		from .common_po_recurrence import TestTimesheetPOrecurrenceCommon


		class TestTimesheetPOrecurrence(TestTimesheetPOrecurrenceCommon):

[14.0][IMP] hr_timesheet_purchase_order: create PO automatically #597

[14.0][IMP] hr_timesheet_purchase_order: create PO automatically #597

Conversation

dessanhemrayev commented Jul 6, 2023 • edited Loading

geomer198 left a comment

Choose a reason for hiding this comment

geomer198 left a comment

Choose a reason for hiding this comment

geomer198 left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Feb 13, 2024

geomer198 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexey-pelykh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geomer198 left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Mar 18, 2024

francesco-ooops commented Mar 18, 2024

alexey-pelykh commented Mar 18, 2024

OCA-git-bot commented Mar 18, 2024

OCA-git-bot commented Mar 18, 2024

dessanhemrayev commented Jul 6, 2023 •

edited

Loading