-
-
Notifications
You must be signed in to change notification settings - Fork 872
[16.0][FIX] project_task_description_template: no duplicate description on template onchange #1606
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
base: 16.0
Are you sure you want to change the base?
[16.0][FIX] project_task_description_template: no duplicate description on template onchange #1606
Conversation
|
@ntsirintanis check out: 32a77d6 |
gfcapalbo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine too. yes. Simpler as well.
…on on template onchange
58683f2 to
6424494
Compare
No description provided.