[14.0][ADD] project_forecast_line_priority #14
Conversation
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
4 times, most recently
from
433d8e2 to
b2438cc
Compare
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
2 times, most recently
from
a2f6eb4 to
e481c98
Compare
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
from
e481c98 to
da08e38
Compare
ntsirintanis
force-pushed
the
14.0-project_forecast_line_deadline
branch
from
08df2f4 to
9f4bde8
Compare
gjotten
changed the title
| if this.date_deadline: | ||
| continue | ||
| # if priority is not set, do nothing | ||
| if int(priority) < 0: |
There was a problem hiding this comment.
less then, or less then equal <= as a task with priority zero has basically no priority, and zero is the default?
There was a problem hiding this comment.
Actually priority 0 means no priority, but the module should also set forecast_date_planned_end in this case too
There was a problem hiding this comment.
@NL66278 functionally what @ntsirintanis says here is correct, the module can set a planned end date for priority 0 as well so we should do only nothing if even that is not the case.
There was a problem hiding this comment.
Actually there is a corner case that priority field can be false. This happens when a portal user is creating the task; the priority field accepts falsy values on that case, i.e., its default value is False.
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
from
6626b20 to
da02886
Compare
| return super(ProjectTask, self - to_write_forecast_date).write(vals) | ||
|
|
||
| def _update_forecast_lines(self): | ||
| """Override cron method and inject forecast date recomputation""" |
There was a problem hiding this comment.
Can we simplify this code by just:
- Searching for all tasks that have no deadline, but do have a priority set
- Retriggering a write of the priority field for those records
- Which will trigger the recomputation
That way, the recomputation logic is in one place only, and the code becomes more maintainable.
There was a problem hiding this comment.
The recordset that this method runs on is set here:
and the cron that summons it is here:
So I thought to just use this project.task recordset, which is already fetched by the cron. Which is simpler than performing another search.
Either way, we can keep it that way, or I can inject another method here
that does what you suggested.
There was a problem hiding this comment.
Hm, reusing that same recordset makes sense, but maybe then still, the recomputation can be triggered by dummy-writing to one of the @api.depends fields, instead of explicitly reiterating the computation logic here, which would save some duplicate code.
@NL66278 What do you think?
ntsirintanis
force-pushed
the
14.0-project_forecast_line_deadline
branch
from
6817b01 to
5517f64
Compare
|
@NL66278 Could you check the two remaining open comments |
ntsirintanis
force-pushed
the
14.0-project_forecast_line_deadline
branch
from
5517f64 to
dff7328
Compare
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
from
2b37667 to
8b140e3
Compare
ntsirintanis
changed the base branch from
14.0-project_forecast_line_deadline
to
14.0
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
2 times, most recently
from
73d5778 to
3e1b2ec
Compare
| @@ -0,0 +1,19 @@ | |||
| # Copyright 2024 Therp BV | |||
There was a problem hiding this comment.
What happened to the <https://therp.nl>. part? Here and elsewhere?
| # is creating a task on portal | ||
| priority = "0" | ||
| selection = self.company_id["forecast_line_priority_%s_selection" % priority] | ||
| if selection == "none": |
There was a problem hiding this comment.
These two lines can be left out, as you return False when selection is not "date" or "delta" anyway.
| selection = self.company_id["forecast_line_priority_%s_selection" % priority] | ||
| if selection == "none": | ||
| return False | ||
| elif selection == "delta": |
There was a problem hiding this comment.
Simple if is enough here, also on the selection == "date" line.
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
2 times, most recently
from
a2d10fb to
ba93d37
Compare
ntsirintanis
force-pushed
the
14.0-project_forecast_line_priority
branch
from
ba93d37 to
97d85cb
Compare
No description provided.