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

Do not reset languageMain #192

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented Mar 1, 2021

Consider the following scenario, where you set up a page like this:

  • example.org/en (fallback)
  • example.org/de

The site contains a lot of pages in both website roots and the pages in the de have their respective page in the main page tree (en) assigned.

However, later on it is decided to switch to a domain based setup, not using the language parameter in the URL:

  • example.org (en)
  • example.de (de)

But when you do that terminal42/contao-changelanguage currently resets the languageMain assignment for all pages in the example.de website tree, meaning that the painstaking work of assigning all the pages the respective main page of the fallback language is lost.

I do not think the extension should do that at all. Under normal circumstances, there is never a use case where you would want this to happen, is there?

@aschempp
Copy link
Member

aschempp commented Mar 3, 2021

Yes there is, for example if you change the fallback page. If you set en to fallback and de to not, not resetting would cause invalid data. Maybe your best bet would be to setup the new config in the database directly?

@fritzmg
Copy link
Contributor Author

fritzmg commented Mar 3, 2021

The problem is actually the submitOnChange => true for tl_page.fallback that was introduced in Contao 4.9. Before that this was not a problem, because if you enabled the fallback and set the primary domain, no data would be lost. However, this is not possible anymore because as soon as you enable the fallback the form is submitted before you have the chance to change the primary domain and thus the data gets deleted.

@aschempp
Copy link
Member

aschempp commented Mar 3, 2021

the same reset would happen if you submit the changes to the root page:

@fritzmg
Copy link
Contributor Author

fritzmg commented Mar 3, 2021

the same reset would happen if you submit the changes to the root page:

The reset only happens if all these conditions are met:

  • The type of the page is root.
  • The root page is the fallback page.
  • No primary domain is set (tl_page.languageRoot).

Due to the submitOnChange => true on tl_page.fallback the last condition will always be true, because when you enable the fallback option, the form is submitted immediately and the primary domain selection will be empty at this point.

@aschempp
Copy link
Member

aschempp commented Mar 3, 2021

Again, you'll get this condition anyway. You'll not change type root, it's already present. You'll enable fallback, but you would need to add a domain and save the record before you can set a languageRoot

@fritzmg
Copy link
Contributor Author

fritzmg commented Mar 3, 2021

You'll enable fallback, but you would need to add a domain and save the record before you can set a languageRoot

Yes, and in that case no reset will happen - in Contao versions prior to 4.9, because the form is not submitted after fallback has been enabled.

@fritzmg
Copy link
Contributor Author

fritzmg commented Mar 3, 2021

To fix this the database consistency check should probably be moved to another event and the callback should additionally check for act=edit, so that no data is reset as long as you are still editing a root page.

@aschempp
Copy link
Member

How is act=edit related to that?

@fritzmg
Copy link
Contributor Author

fritzmg commented Mar 21, 2021

How is act=edit related to that?

The language assignments should not be reset while you are still editing the tl_page record. Only the final root page settings should be evaluated when deciding whether or not to reset the language assignments. That's the idea at least.

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