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

[FW][FIX] website: prevent cow if editing generic page with diverged URL #163162

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Apr 24, 2024

In the case of a generic page and its specific page: When the specific page change its URL, its view will still have the sam key as the generic page despite not being served on the same URL anymore and not really being the same ressource anymore. This is a bit weird conceptually.
Indeed, the generic page is not shadowed by the specific one anymore.

This is an issue because when going on the generic page and editing it, the save will actually "bug": it will not write on the generic view that you edited but the save will instead be "redirected" (through the COW mechanism) to the specific view.
The editor will look like it did not save your change, while in fact it actually erased, behind the scene, the specific page content and replaced it by the one you tried to save on the generic page. You really just lost your specific page content without knowing it.

This is a corner case of the COW mechanism and the holy grail rule of multi website (editing a website should only impact itself and not other websites): we serve both the generic content and its specific content on the website, which should never be possible except for this particular case of a view being linked to a page and the specific view's page having its URL changed.

Note that an apparently good solution would be to adapt the view key to reflect its new page URL (as when you create a new page), but it's not possible. We don't do that for a simple reason: the page view's key could be xpath'd. And it is even the case for the contactus page. Even if it is not a very legit flow to xpath a page view, because it makes (most of the time) that page not possible to edit, we have to support it.

Also note that a generic page is not something supposed to exist, even if we support it (and use it for /contactus), it's best to avoid it:

  1. You can't do that through the UI unless going into debug mode
  2. It does not make a lot of sense to share a whole page content on multiple websites, as it would duplicated content (bad for SEO).

Steps to reproduce:

  • Go to /contactus and enter edit mode
  • Change something like add "WEBSITE1" somewhere in the page
  • Open the page properties and change the url from /contactus to /contactuswebsite1
  • Go to /contactus, which will still be available and now show the generic page
  • You will not see the "WEBSITE1" you added, which is what is expected as this is the generic page, the "WEBSITE1" is on the /contactuswebsite1 page
  • Enter edit mode and add something, like "GENERIC", save
  • The page will reload (as you saved) but your "GENERIC" will disappear, looking like it did not save your change
  • Now go to /contactuswebsite1, you will see that the "WEBSITE1" word is gone, and the "GENERIC" word is there

On top of the following OPWs, the bug was also reported internally on discord.
opw-3760257
opw-3473923
task-3476840

Forward-Port-Of: #163038
Forward-Port-Of: #159297

In the case of a generic page and its specific page:
When the specific page change its URL, its view will still have the sam
key as the generic page despite not being served on the same URL anymore
and not really being the same ressource anymore. This is a bit weird
conceptually.
Indeed, the generic page is not shadowed by the specific one anymore.

This is an issue because when going on the generic page and editing it,
the save will actually "bug": it will not write on the generic view that
you edited but the save will instead be "redirected" (through the COW
mechanism) to the specific view.
The editor will look like it did not save your change, while in fact it
actually erased, behind the scene, the specific page content and
replaced it by the one you tried to save on the generic page.
You really just lost your specific page content without knowing it.

This is a corner case of the COW mechanism and the holy grail rule of
multi website (editing a website should only impact itself and not other
websites): we serve both the generic content and its specific content on
the website, which should never be possible except for this particular
case of a view being linked to a page and the specific view's page
having its URL changed.

Note that an apparently good solution would be to adapt the view key to
reflect its new page URL (as when you create a new page), but it's not
possible. We don't do that for a simple reason: the page view's key
could be t-called'd.

Also note that a generic page is not something supposed to exist, even
if we support it (and use it for /contactus), it's best to avoid it:
1. You can't do that through the UI unless going into debug mode
2. It does not make a lot of sense to share a whole page content on
   multiple websites, as it would be duplicated content (bad for SEO).

Steps to reproduce:
- Go to /contactus and enter edit mode
- Change something like add "WEBSITE1" somewhere in the page
- Open the page properties and change the url from /contactus to
  /contactuswebsite1
- Go to /contactus, which will still be available and now show the
  generic page
- You will not see the "WEBSITE1" you added, which is what is expected
  as this is the generic page, the "WEBSITE1" is on the
  /contactuswebsite1 page
- Enter edit mode and add something, like "GENERIC", save
- The page will reload (as you saved) but your "GENERIC" will disappear,
  looking like it did not save your change
- Now go to /contactuswebsite1, you will see that the "WEBSITE1" word
  is gone, and the "GENERIC" word is there

This commit is making it so when you edit a generic page, it will write
on it, bypassing the COW mechanism and the holy grail rule. It's a very
niche case which will be handled as an exception.
In Odoo 17 (last stable), we will replace this fix by another one which
will prevent the access of a generic page when there is a specific page
with a different URL.

On top of the following OPWs, the bug was also reported internally on
discord.
opw-3760257
opw-3473923
task-3476840

X-original-commit: 4e90722
@robodoo
Copy link
Contributor

robodoo commented Apr 24, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented Apr 24, 2024

This PR targets saas-16.4 and is part of the forward-port chain. Further PRs will be created up to master.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added the forwardport This PR was created by @fw-bot label Apr 24, 2024
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Apr 24, 2024
@rdeodoo
Copy link
Contributor

rdeodoo commented Apr 25, 2024

@fw-bot r+

@fw-bot
Copy link
Contributor Author

fw-bot commented Apr 25, 2024

@rdeodoo @qsm-odoo child PR #163179 was modified / updated and has become a normal PR. This PR (and any of its parents) will need to be merged independently as approvals won't cross.

robodoo pushed a commit that referenced this pull request Apr 25, 2024
In the case of a generic page and its specific page:
When the specific page change its URL, its view will still have the sam
key as the generic page despite not being served on the same URL anymore
and not really being the same ressource anymore. This is a bit weird
conceptually.
Indeed, the generic page is not shadowed by the specific one anymore.

This is an issue because when going on the generic page and editing it,
the save will actually "bug": it will not write on the generic view that
you edited but the save will instead be "redirected" (through the COW
mechanism) to the specific view.
The editor will look like it did not save your change, while in fact it
actually erased, behind the scene, the specific page content and
replaced it by the one you tried to save on the generic page.
You really just lost your specific page content without knowing it.

This is a corner case of the COW mechanism and the holy grail rule of
multi website (editing a website should only impact itself and not other
websites): we serve both the generic content and its specific content on
the website, which should never be possible except for this particular
case of a view being linked to a page and the specific view's page
having its URL changed.

Note that an apparently good solution would be to adapt the view key to
reflect its new page URL (as when you create a new page), but it's not
possible. We don't do that for a simple reason: the page view's key
could be t-called'd.

Also note that a generic page is not something supposed to exist, even
if we support it (and use it for /contactus), it's best to avoid it:
1. You can't do that through the UI unless going into debug mode
2. It does not make a lot of sense to share a whole page content on
   multiple websites, as it would be duplicated content (bad for SEO).

Steps to reproduce:
- Go to /contactus and enter edit mode
- Change something like add "WEBSITE1" somewhere in the page
- Open the page properties and change the url from /contactus to
  /contactuswebsite1
- Go to /contactus, which will still be available and now show the
  generic page
- You will not see the "WEBSITE1" you added, which is what is expected
  as this is the generic page, the "WEBSITE1" is on the
  /contactuswebsite1 page
- Enter edit mode and add something, like "GENERIC", save
- The page will reload (as you saved) but your "GENERIC" will disappear,
  looking like it did not save your change
- Now go to /contactuswebsite1, you will see that the "WEBSITE1" word
  is gone, and the "GENERIC" word is there

This commit is making it so when you edit a generic page, it will write
on it, bypassing the COW mechanism and the holy grail rule. It's a very
niche case which will be handled as an exception.
In Odoo 17 (last stable), we will replace this fix by another one which
will prevent the access of a generic page when there is a specific page
with a different URL.

On top of the following OPWs, the bug was also reported internally on
discord.
opw-3760257
opw-3473923
task-3476840

closes #163162

X-original-commit: 4e90722
Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
@robodoo robodoo closed this Apr 25, 2024
@qsm-odoo qsm-odoo deleted the saas-16.4-17.0-fix-generic-page-url-diverse-rde-lYcf-fw branch April 25, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants