[16.0][FIX] project_task_description_template: no duplicate description on template onchange#1606
Conversation
|
@ntsirintanis check out: 32a77d6 |
gfcapalbo
left a comment
gfcapalbo
left a comment
There was a problem hiding this comment.
@ntsirintanis two small changes. but in general seems the way to go.
As revealed in testing , it will not cover the case of multiple addition once the user has changed the template. but that's okay, this is an improvement on current.
| if not task.description_template_id: | ||
| continue | ||
| template_text = task.description_template_id.description or "" | ||
| if not template_text: |
There was a problem hiding this comment.
this code is extra. it is strange we allow empty description, but if there is one, it would change nothing. just extra code. not needed.
| continue | ||
| current = task.description or "" | ||
| # Avoid duplicating the same template content | ||
| if current == template_text or current.endswith(template_text): |
There was a problem hiding this comment.
Checkout my regex solution on v14. it covers the template present in any part of the text.
32a77d6
There was a problem hiding this comment.
@gfcapalbo or instead of current.endswith(template_text), do something like current in template_text. The results should be the same exactly as the one
There was a problem hiding this comment.
Works fine too. yes. Simpler as well.
…on on template onchange
ntsirintanis
force-pushed
the
16.0-fix-project_task_description_template_onchange
branch
from
58683f2 to
6424494
Compare
alexey-pelykh
left a comment
alexey-pelykh
left a comment
There was a problem hiding this comment.
Thanks for the fix and for iterating on the feedback.
The updated code addresses both points from the previous review:
- Removed the redundant empty-template guard
- Switched from
endswithtotemplate_text in currentfor full substring detection
The str() casts are the right call here since Odoo 16 HTML fields return markupsafe.Markup objects and the in operator may not behave as expected across Markup/str boundaries.
CI is green (the codecov failures are pre-existing -- this module has no tests directory).
One minor note for awareness: if a user's own description text happens to contain the exact template string, the template won't be re-appended. This was already acknowledged as acceptable in the earlier discussion.
Looks good to merge.
3 participants
No description provided.