Skip to content
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

[16.0][OU-FIX] website: rename group_website_publisher in custom templates #4216

Conversation

chienandalu
Copy link
Member

Reviewing #4023 I detected that this can be an issue in custom views that make use of that group.

cc @Tecnativa TT45244

@chienandalu chienandalu force-pushed the 16.0-fix-website-group_website_restricted_editor branch from 3c87a24 to 7e6d449 Compare November 7, 2023 08:12
Locate group directives in custom views with the former `group_website_publisher`
group which is now renamed to `group_website_restricted_editor`.

TT45244
@chienandalu chienandalu force-pushed the 16.0-fix-website-group_website_restricted_editor branch from 7e6d449 to f639767 Compare November 7, 2023 08:40
@chienandalu
Copy link
Member Author

fixed 🤦

@pedrobaeza pedrobaeza added this to the 16.0 milestone Nov 7, 2023
@pedrobaeza
Copy link
Member

Although it's a very distant possibility, you may have an string website.group_website_publisher<whatever> that is not for this use, so is it a way to directly check the string with the delimiters (are they single ' or double " quotes?) and maybe the while definition with the groups thing?

@chienandalu
Copy link
Member Author

so is it a way to directly check the string with the delimiters (are they single ' or double " quotes?) and maybe the while definition with the groups thing?

Normally it's double but it could be both. And also there could be commas (e.g: groups='base.group_none,website.group_website_publisher').

To consider that possibility we could maybe use openupgradelib.convert_html_fragment or hook this change into the general list of replacements (https://github.com/OCA/openupgradelib/blob/c00a6662cbc9470cca8e0192cdda58ecfd805144/openupgradelib/openupgrade_160.py#L283-L295)

@pedrobaeza
Copy link
Member

The second one sounds good then.

@chienandalu
Copy link
Member Author

The second one sounds good then.

I'm seeing that is going to be a little bit harder than expected. We can match by xpath the group but only remove it and add it later. That would spoil those cases which had several more groups along with the one we want to replace...

@pedrobaeza
Copy link
Member

So fragment conversions is not handling correctly the removal and addition of elements in list separated by commas?

@chienandalu
Copy link
Member Author

So fragment conversions is not handling correctly the removal and addition of elements in list separated by commas?

It simply isn't supported and that case is hardcoded for classes and styles

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK, it's very limiting IMO.

Let's merge this patch here then.

@pedrobaeza pedrobaeza merged commit 2fe40e9 into OCA:16.0 Nov 7, 2023
@pedrobaeza pedrobaeza deleted the 16.0-fix-website-group_website_restricted_editor branch November 7, 2023 11:57
@chienandalu
Copy link
Member Author

Yeah, maybe for a more important future migration we'll need to refactor that part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants