12.0 mig project_recalculate #540

ernestotejeda · 2019-05-16T19:00:21Z

readme by fragments
adapt python code to use the new methods of the calendar class
adapt tests

oca-clabot · 2019-05-16T19:00:24Z

Hey @ernestotejeda, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

mike <mike@mike.(none)> (no github login found)
Endika Iglesias endikaig@antiun.com (no github login found)

Appreciation of efforts,
OCA CLAbot

pedrobaeza · 2019-05-17T17:14:49Z

project_recalculate/models/project_task.py

@@ -57,14 +60,17 @@ def _dates_onchange(self, vals):
        start = from_string(date_start)
        end = from_string(date_end)
        resource, calendar = self._resource_calendar_select()
+
+        if not start.tzinfo:


For saving a lot of code lines, call directly in all places to _resource_timezone, and there, you check:

if dt.tzinfo: return dt

pedrobaeza · 2019-05-17T17:15:01Z

project_recalculate/models/project_task.py

@@ -82,13 +88,17 @@ def _dates_onchange(self, vals):
                    return vals
                project_date = from_string(self.project_id.date)
                start, end = start, project_date
+


Don't add useless empty lines

pedrobaeza · 2019-05-17T17:16:31Z

project_recalculate/models/project_task.py

@@ -193,39 +202,71 @@ def _interval_context_tz(self, interval):
        start = fields.Datetime.context_timestamp(self, start)
        end = datetime.now().replace(hour=interval[1])
        end = fields.Datetime.context_timestamp(self, end)
-        return (start.hour, end.hour)
+        result = Intervals()
+        result._items.append((start, end))


Should you access to the protected variable _items? Isn't there a way to add elements directly through a public API?

Thinking better I think it does not make much sense to use the _interval_default_get method. Since in v12, the resource.calendar methods no longer receive a default_interval as a parameter.
So, it is not necessary to create an Intervals object

pedrobaeza · 2019-05-17T17:17:26Z

project_recalculate/models/project_task.py

-    def _calendar_schedule_days(self, days, day_date,
-                                resource=None, calendar=None):
+        intervals = self._get_work_intervals(day_date, resource, calendar)
+        return intervals._items and intervals._items[0] or False


Isn't the first element accessible without needing to access protected variable? (intervals[0]?)

pedrobaeza · 2019-05-17T17:17:56Z

project_recalculate/models/project_task.py

+                if end_planned_dt:
+                    date_end = self._last_interval_of_day_get(
+                        end_planned_dt, resource, calendar)[1]
+


Remove useless empty lines inside a method

pedrobaeza · 2019-05-17T17:19:29Z

project_recalculate/models/resource_calendar.py

+
+        # which method to use for retrieving intervals
+        if compute_leaves:
+            get_intervals = partial(self._work_intervals, resource=resource,


Why accessing protected variable _work_intervals?

I have added the plan_days_to_resource method to resource.calendar with the intention that it does the same as plan_days but for a specific resource that is passed by parameter. Therefore, the code of that method is almost the same as that of the existing plan_days method, that's why there is the call to the protected method _work_intervals.
I have done this with the intention that the planning of the days take into account the leaves of the resource

pedrobaeza · 2019-05-17T17:19:58Z

project_recalculate/models/resource_calendar.py

+        if days > 0:
+            found = set()
+            delta = timedelta(days=14)
+            for n in range(100):


This double loop is not very performant. Isn't a better solution?

pedrobaeza · 2019-05-17T17:21:11Z

project_recalculate/tests/test_project_project.py

-            ['pj_0', '2015-08-01', '2015-08-05'],
-            ['pj_1', '2015-08-02', '2015-08-05'],
-            ['pj_2', '2015-08-03', '2015-08-05'],
+            ['pj0', date(2015, 8, 1), date(2015, 8, 5)],


Why changing key names?

because if this module is tested with the project_key module, then the tests do not pass. The project_key module adds the key field at the project level and the value of that field must be unique. What happens is that this field is automatically filled depending on the module name and for these module names, the value generated for the key field is the same.

This must be definitively fixed on project_key which is causing a lot of problems with the rest of the modules. Please do another PR disabling the features if on test mode and not testing the module itself (via a context key). Example: https://github.com/OCA/bank-payment/blob/7ce8bbb88bd8c889347a7550aa973124b847af58/account_payment_transfer_reconcile_batch/models/payment_order.py#L28

pedrobaeza · 2019-05-17T17:21:32Z

project_recalculate/tests/test_project_task.py

-                'pj_0': list(self.task_days['date_begin']),
-                'pj_1': list(self.task_days['date_begin']),
-                'pj_2': list(self.task_days['date_begin']),
+                'pj0': list(self.task_days['date_begin']),


Why changing key names? Better to reduce the diff

pedrobaeza · 2019-05-17T17:21:55Z

project_recalculate/wizard/recalculate_wizard.py

 # See README.rst file on addon root folder for license details

 from odoo import models, fields, api


 class ProjectRecalculateWizard(models.TransientModel):
    _name = 'project.recalculate.wizard'
+    _description = 'project recalculate wizard'


Use upper in first letter

ernestotejeda · 2019-05-21T20:13:26Z

Changes done

pedrobaeza · 2019-05-22T07:34:43Z

You can rebase now and leave test data as it was

…unction "project recalculate".

…rce/calendar

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

ernestotejeda · 2019-05-22T13:27:01Z

Changes done

yajo

Some typos

yajo · 2019-05-23T07:22:10Z

project_recalculate/readme/CONFIGURE.rst

@@ -0,0 +1,19 @@
+You can define working calendar at Setting > Technical > Resource > Working time


Suggested change

You can define working calendar at Setting > Technical > Resource > Working time

You can define working calendar at Setting > Technical > Resource > Working time.

yajo · 2019-05-23T07:22:22Z

project_recalculate/readme/CONFIGURE.rst

@@ -0,0 +1,19 @@
+You can define working calendar at Setting > Technical > Resource > Working time
+Then assign this calendar to a resource (related with an user), a project or


Suggested change

Then assign this calendar to a resource (related with an user), a project or

Then assign this calendar to a resource (related with a user), a project or

yajo · 2019-05-23T07:22:30Z

project_recalculate/readme/CONFIGURE.rst

@@ -0,0 +1,19 @@
+You can define working calendar at Setting > Technical > Resource > Working time
+Then assign this calendar to a resource (related with an user), a project or
+a company


Suggested change

a company

a company.

yajo · 2019-05-23T07:23:41Z

project_recalculate/readme/CONFIGURE.rst

+Also you can define which task stages are included in recalculation when
+'Project recalculate' button is clicked. By default, all are included.
+To change this go to Project > Configuration > Stages > Task Stages and change
+'Include in project recalculate' field


Suggested change

'Include in project recalculate' field

the 'Include in project recalculate' field.

ernestotejeda · 2019-05-23T14:06:17Z

Changes done

Tardo · 2019-05-23T16:25:36Z

project_recalculate/models/project_task.py

+            days = self.from_days * (-1)
+        return increment, project_date, days
+
+    def _resource_timezone(self, dt, resource=None, calendar=None):


Here perhaps can use Odoo Style:

tz = 'UTC' if resource or calendar: tz = (resource or calendar).tz record_user_timestamp = env.user.sudo().with_context(tz=tz) return fields.Datetime.context_timestamp(record_user_timestamp, dt)

What happens is method should not return the converted dt to the timezone of the resource or calendar. The method must return the same dt but aware of the timezone of the resource or calendar.
Example:
for
dt = 2019-06-23 00:00:00
tz = 'Europe / Brussels'
I want it to return
2019-06-03 00:00:00+02:00 instead of 2019-06-23 02:00:00+02:00

I don't understand why need timezone if you like hours in UTC...

2019-06-03 00:00:00+02:00 in UTC is 2019-05-03 22:00:00+00:00

yajo

I have to admit I mostly don't understand this source code, but if tests pass... OK.

However there's one point that totally seems a bug, and there are some suggestions too.

yajo · 2019-05-24T08:22:35Z

project_recalculate/models/resource_calendar.py

+        days = 0
+        current = start_dt
+        while current <= end_dt:
+            if id is None:


⚠️ id is a built-in function that will never be None. I'm not sure what this code was supposed to do, but it's wrong. 😅

yajo · 2019-05-24T08:26:07Z

project_recalculate/models/project_task.py

-                'date_start': date_start and to_string(date_start) or False,
-                'date_end': date_end and to_string(date_end) or False,
-                'date_deadline': date_end and to_string(date_end) or False,
+                'date_start': date_start and date_start or False,


Oops, did you mean this?

Suggested change

'date_start': date_start and date_start or False,

'date_start': date_start or False,

yajo · 2019-05-24T08:29:14Z

project_recalculate/models/project_task.py

+
+            task.with_context(task.env.context, task_recalculate=True).write({
+                'date_start': date_start and date_start or False,
+                'date_end': date_end and date_end or False,


Suggested change

'date_end': date_end and date_end or False,

'date_end': date_end or False,

yajo · 2019-05-24T08:29:21Z

project_recalculate/models/project_task.py

+            task.with_context(task.env.context, task_recalculate=True).write({
+                'date_start': date_start and date_start or False,
+                'date_end': date_end and date_end or False,
+                'date_deadline': date_end and date_end or False,


Suggested change

'date_deadline': date_end and date_end or False,

'date_deadline': date_end or False,

yajo · 2019-05-24T08:30:38Z

project_recalculate/models/project_task.py

+                    _('Estimated days must be greater than 0.')
+                )
+
+    def _dates_onchange(self, vals):


I don't like this method name. It's not an onchange, so it's confusing. Could you change it?

yajo · 2019-05-24T08:34:42Z

project_recalculate/models/project_project.py

+                raise UserError(_("Cannot recalculate project because your "
+                                  "project doesn't have date end."))
+            for task in project.tasks:
+                task.task_recalculate()


Why not just project.tasks.task_recalculate()?

yajo · 2019-05-24T08:36:51Z

project_recalculate/models/project_task.py

+                if not self.project_id.date_start:
+                    # Can't calculate from_days without project date_start
+                    return vals
+                project_date = from_string(self.project_id.date_start)


It should return a datetime object already, isn't it?

Suggested change

project_date = from_string(self.project_id.date_start)

project_date = self.project_id.date_start

yajo · 2019-05-24T08:37:34Z

project_recalculate/models/project_task.py

+                if not self.project_id.date:
+                    # Can't calculate from_days without project date
+                    return vals
+                project_date = from_string(self.project_id.date)


Suggested change

project_date = from_string(self.project_id.date)

project_date = self.project_id.date

yajo · 2019-05-24T08:43:42Z

project_recalculate/models/resource_calendar.py

+                working_intervals = obj._work_intervals(current, end, resource)
+                if len(working_intervals):
+                    days += 1
+            next_dt = current + timedelta(days=1)


Suggested change

next_dt = current + timedelta(days=1)

yajo · 2019-05-24T08:43:55Z

project_recalculate/models/resource_calendar.py

+                if len(working_intervals):
+                    days += 1
+            next_dt = current + timedelta(days=1)
+            current = next_dt


Suggested change

current = next_dt

current += timedelta(days=1)

rafaelbn · 2019-05-27T17:12:57Z

Hello @patrickrwilson maybe you are interested in this one. As commented in #500

patrickrwilson · 2019-05-28T18:09:01Z

@ernestotejeda thanks for working on converting this module. Do you mind squashing all your commits after your next changes please.

rafaelbn

Tested in runbot IMHO

If the task don't have any user assigned the module don't calculate anything
If the task have an user assigned with a working time 30 hour and the project have a working time of 40 hours the task are planned with the working time of the project and not with the working time of the user

This should be fixed or added to know issues as you could have no calculation or a wrong planning calculation based in a working plan thatis diferent of the user. This cases have been never deliberate from v8

pedrobaeza · 2019-06-04T16:33:49Z

I think you should use user working time if present, and project if not.

ernestotejeda · 2019-06-04T20:29:14Z

@rafaelbn , check now, I've already made those changes

rafaelbn · 2019-06-04T21:07:56Z

@ernestotejeda in the project form the fields Start date and Expiration date are duplicated
(tested in http://3373450-540-efe2ea.runbot2-2.odoo-community.org/web?debug=true#id=5&action=231&model=project.project&view_type=form&menu_id=154 )

It looks like project_recalculate and project_timeline add the same fields two times

rafaelbn · 2019-06-04T21:15:16Z

@ernestotejeda even with debug mode (developer mode) activate user cannot see in the TASKS start date or end date in order to modify manually a single task. They should, check please:

rafaelbn · 2019-06-04T21:22:33Z

@ernestotejeda The sequence of tasks is incorrect, "Installation" task starts in July but the the Kanban view the order is by sequence so you cannot see first the task that begins first. IMHO we could set a sequence by start date. The task for example, "Follow-up socios potenciales" starts in June but in the kanban view doesn't appear as is in the lowest place

ernestotejeda · 2019-06-05T03:56:07Z

@ernestotejeda in the project form the fields Start date and Expiration date are duplicated

@rafaelbn These fields are going to be repeated when the project_recalculate and project_timeline modules are installed at the same time. These two modules add to the project form view these two fields after the Working Time field.
A solution could be to set this module to depend on project_timeline, which is not essential for its operation but I don't think it would be bad. Providing a timeline view, for example, would allow to see quickly how a planning was made by this module.
Another solution I can think of is to add a new module to this repository that only aims to show the start and end dates of the project and perhaps those dates for each task in their form view. Then, project_recalculate and project_timeline, will depend on the new module, so there is no need for this project_recalculate to depend on project_timeline. Perhaps there are cases in which the timeline view does not interest or it's prefered another instead of this one. The problem with this solution is that I don't know what repercussion to add a new module has for the repository. Or what repercussion it has to set project_timeline to depend on this module now that it is already migrated and that someone could be using it.

ernestotejeda · 2019-06-05T04:00:04Z

@ernestotejeda even with debug mode (developer mode) activate user cannot see in the TASKS start date or end date in order to modify manually a single task...

@rafaelbn The only way I have found to edit those fields in the form view is by installing the project_timeline module, which have the same problem mentioned in the previous comment..

ernestotejeda · 2019-06-05T04:02:18Z

@ernestotejeda The sequence of tasks is incorrect, "Installation" task starts in July but...

@rafaelbn , yes, I think it could be ordered by start_date, what do you think @pedrobaeza ?

pedrobaeza · 2019-06-05T16:02:13Z

No, the order in the kanban is one different from the "time" order. You have timeline view for this purpose.

chienandalu

@ernestotejeda Please resolve done conversations to ease review of what's left. Also, try to increase test coverage if possible

pedrobaeza · 2019-06-07T13:37:41Z

Let's merge this as is and let improvements for a second phase, as this migration has been very time consuming without customer real uses for most of the cases.

ernestotejeda force-pushed the 12.0-mig-project_recalculate branch from efed02f to 15cb6ef Compare May 16, 2019 20:36

pedrobaeza requested changes May 17, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-project_recalculate branch from 15cb6ef to 2bad184 Compare May 21, 2019 16:42

pedrobaeza added this to the 12.0 milestone May 22, 2019

Endika Iglesias and others added 23 commits May 22, 2019 08:31

[ADD] new addon project_recalculate

785b255

[FIX] Define UTC as tz in get_working_days_of_date method

7c37719

[FIX] frozendict error

dbb4663

OCA Transbot updated translations from Transifex

870a4d9

[MIG] Make modules uninstallable

7c6c082

[FIX] remove en.po that was erroneously created by transbot

f62af9a

[MIG] Rename manifest files

d6b1a12

OCA Transbot updated translations from Transifex

64a35ee

* Fixing Error W503

d8c8cd4

OCA Transbot updated translations from Transifex

e190d48

project_recalculate: Remove condition calculation type! = 'None' in f…

a623450

…unction "project recalculate".

OCA Transbot updated translations from Transifex

01b12b0

[MIG] project_recalculate: Migration to 10.0

9f0f3d2

OCA Transbot updated translations from Transifex

7eb5721

OCA Transbot updated translations from Transifex

ce2579f

OCA Transbot updated translations from Transifex

8d4bbde

[FIX] project_recalculate: Avoid error on no resource nor calendar

6c037d7

OCA Transbot updated translations from Transifex

27725fc

[FIX] project_recalculate: Avoid error when recalculating if no resou…

719ea4c

…rce/calendar

OCA Transbot updated translations from Transifex

06315bb

[UPD] Update project_recalculate.pot

59dcb0b

[UPD] Update project_recalculate.pot

353800b

Update translation files

62459d0

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

ernestotejeda force-pushed the 12.0-mig-project_recalculate branch from 2bad184 to 8c72654 Compare May 22, 2019 12:42

pedrobaeza requested a review from yajo May 23, 2019 07:19

yajo reviewed May 23, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-project_recalculate branch from 8c72654 to 921bd9c Compare May 23, 2019 13:31

Tardo reviewed May 23, 2019

View reviewed changes

yajo mentioned this pull request May 24, 2019

12.0 mig event_project OCA/event#151

Merged

yajo requested changes May 24, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-project_recalculate branch 2 times, most recently from 27f3027 to 484338b Compare May 27, 2019 11:53

rafaelbn reviewed Jun 4, 2019

View reviewed changes

[MIG] project_recalculate: Migration to 12.0

efe2ea2

ernestotejeda force-pushed the 12.0-mig-project_recalculate branch from 484338b to efe2ea2 Compare June 4, 2019 19:45

chienandalu reviewed Jun 7, 2019

View reviewed changes

pedrobaeza merged commit 0e67250 into OCA:12.0 Jun 7, 2019

pedrobaeza deleted the 12.0-mig-project_recalculate branch June 7, 2019 13:37

OCA-git-bot mentioned this pull request Jun 7, 2019

Migration to version 12.0 #435

Closed

23 tasks

		@@ -0,0 +1,19 @@
		You can define working calendar at Setting > Technical > Resource > Working time

		@@ -0,0 +1,19 @@
		You can define working calendar at Setting > Technical > Resource > Working time
		Then assign this calendar to a resource (related with an user), a project or

	'Include in project recalculate' field
	the 'Include in project recalculate' field.

	'date_start': date_start and date_start or False,
	'date_start': date_start or False,

	'date_end': date_end and date_end or False,
	'date_end': date_end or False,

	'date_deadline': date_end and date_end or False,
	'date_deadline': date_end or False,

	project_date = from_string(self.project_id.date_start)
	project_date = self.project_id.date_start

	project_date = from_string(self.project_id.date)
	project_date = self.project_id.date

12.0 mig project_recalculate #540

12.0 mig project_recalculate #540

Conversation

ernestotejeda commented May 16, 2019

oca-clabot commented May 16, 2019

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

ernestotejeda commented May 21, 2019

pedrobaeza commented May 22, 2019

ernestotejeda commented May 22, 2019

yajo 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

ernestotejeda commented May 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yajo 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

rafaelbn commented May 27, 2019

patrickrwilson commented May 28, 2019

rafaelbn left a comment • edited Loading

Choose a reason for hiding this comment

pedrobaeza commented Jun 4, 2019

ernestotejeda commented Jun 4, 2019

rafaelbn commented Jun 4, 2019 • edited Loading

rafaelbn commented Jun 4, 2019 • edited Loading

rafaelbn commented Jun 4, 2019

ernestotejeda commented Jun 5, 2019 • edited Loading

ernestotejeda commented Jun 5, 2019

ernestotejeda commented Jun 5, 2019

pedrobaeza commented Jun 5, 2019

chienandalu left a comment

Choose a reason for hiding this comment

pedrobaeza commented Jun 7, 2019

rafaelbn left a comment •

edited

Loading

rafaelbn commented Jun 4, 2019 •

edited

Loading

rafaelbn commented Jun 4, 2019 •

edited

Loading

ernestotejeda commented Jun 5, 2019 •

edited

Loading