Skip to content

[15.0][IMP] template_content_swapper: allow manual editing of the name - #1305

Open
smorita7749 wants to merge 1 commit into
OCA:15.0from
qrtl:15.0-imp-template_content_swapper
Open

[15.0][IMP] template_content_swapper: allow manual editing of the name#1305
smorita7749 wants to merge 1 commit into
OCA:15.0from
qrtl:15.0-imp-template_content_swapper

Conversation

@smorita7749

@smorita7749 smorita7749 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

This change allows users to edit the mapping name manually.
@qrtl QT6819

@OCA-git-bot OCA-git-bot added series:15.0 mod:template_content_swapper Module template_content_swapper labels Jun 25, 2026
return self.env["res.lang"].get_installed()

name = fields.Char(compute="_compute_name", store=True, readonly=True)
name = fields.Char()

@AungKoKoLin1997 AungKoKoLin1997 Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible just by changing readonly to False?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to add name before inserting content values? Is that the reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent is to preserve a manually entered name even after content_from / content_to are changed.
I reflected the review for Yoshi-san, so please take a look when you have time.

Comment thread template_content_swapper/models/template_content_mapping.py Outdated
Comment thread template_content_swapper/models/template_content_mapping.py Outdated
@smorita7749
smorita7749 force-pushed the 15.0-imp-template_content_swapper branch from 24e9598 to 623b46b Compare June 30, 2026 05:23
@smorita7749
smorita7749 force-pushed the 15.0-imp-template_content_swapper branch from 623b46b to c98be3d Compare June 30, 2026 23:57

@AungKoKoLin1997 AungKoKoLin1997 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding logic in both create and onchange, I am wondering why can't we keep original approach just by adding readonly=False attribute. The original compute method depends on content_from and content_to. So, user can still change the name after these values are filled. IMO, changing the name when content_from or content_to is make sense and the name should be updated.

def create(self, vals_list):
for vals in vals_list:
if not vals.get("name") and vals.get("content_from"):
vals["name"] = vals["content_from"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we keep original practice f"{record.content_from or ''} -> {record.content_to or ''}"? Is there any reason to use only content_from?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the original computed version the name was always rebuilt from content_from / content_to, so including content_to made sense.

Now the record is created as soon as content_from (a required field) is filled, and content_to is optional and often not set yet at that point. So I guess there's no need to build the name from content_to here.

@AungKoKoLin1997 AungKoKoLin1997 Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I am not comfortable is current improvement breaks the original behavior and always forces us to put the manual name. If we don't do it, the name will always show "content_from" only. I understand the name field in the config is not in important role at the moment but I don't want to break existing behavior and forcing a manual name.

def _onchange_content(self):
"""Auto-fill the name from the contents while empty; keep manual names."""
if not self.name and self.content_from:
self.name = self.content_from

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.
Isn't better to use write method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name is required=True, so a saved record always has a name, and a write override would have nothing to fill. The onchange fills a default in the form while the record's name is still empty before saving.

@smorita7749

Copy link
Copy Markdown
Contributor Author

Instead of adding logic in both create and onchange, I am wondering why can't we keep original approach just by adding readonly=False attribute. The original compute method depends on content_from and content_to. So, user can still change the name after these values are filled. IMO, changing the name when content_from or content_to is make sense and the name should be updated.

The editable-computed way would overwrite the name, but that's what we want to avoid. content_from / content_to can hold long text or markup, so a name built from them is often hard to read. So we want to let users set their own clear name, and keep it even when content_from / content_to are changed later. The auto-fill is just a helper to set a default name while it's empty, and once the user sets a name, we keep it. That's why name is a normal field filled in create / onchange instead of a computed one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:template_content_swapper Module template_content_swapper series:15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants