?
avatar richard67
richard67
9 Jul 2019

Steps to reproduce the issue

Update multilingual Joomla 3.9.8 site to 3.9.9.

Expected result

Same as before the update, see following screen shot:

screen shot 2019-07-09 at 18 29 20

Actual result

Each template style is shown 3 times, I guess because the site has 3 languages.

The language-specific default is lost, i.e. no template style is default for a particular language, only the default for all languages is not lost.

See following screen shot from after the update:

screen shot 2019-07-09 at 18 32 28

When editing a template style and setting it to default for a particular language, saving and closing, there is no change in the "Default" value for that template style, i.e. the information that it should be default e.g. for German is not saved.

System information (as much as possible)

Multilingual Joomla 3.9.8 site updated to 3.9.9

PHP 7.2, MySQL 5.7.25-log

For every site language one default template style plus another style as default for all languages, but I think that's not really required to reproduce the issue.

avatar richard67 richard67 - open - 9 Jul 2019
avatar joomla-cms-bot joomla-cms-bot - labeled - 9 Jul 2019
avatar richard67 richard67 - change - 9 Jul 2019
The description was changed
avatar richard67 richard67 - edited - 9 Jul 2019
avatar richard67
richard67 - comment - 9 Jul 2019

I will see if I can find the reason, but if someone is faster he/she is welcome 😄


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25482.

avatar richard67
richard67 - comment - 9 Jul 2019

@infograf768 Can you reproduce this?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25482.

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

Issue confirmed, but not only on multilingual websites... on every website!

image

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

I guess this needs an immediate patch to push a 3.9.10 just now... probably it has something to do with an SQL JOIN query

avatar richard67
richard67 - comment - 9 Jul 2019

I am almost sure it has to do with a join, some recent "optimization". Not sure if it needs an immediate patch, I can live with it so far with my small little, rarely edited web site. Only thing is I don't have language specific template style default anymore and so no language specific site motto. Not nice, but frontend is still ok.

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

Yes but having the same style listed multiple times and poiting to the same record is not acceptable for a production version. I think it would be better to fix it immediately.

avatar richard67
richard67 - comment - 9 Jul 2019

Hmm, it is not that easy, the join here has not changed since 3 years: https://github.com/joomla/joomla-cms/blame/staging/administrator/components/com_templates/models/styles.php#L126.

So maybe it is related to data type changes of database columns in recent times, e.g. things like home from string, e.g. '1', to numeric, e.g. 1.

avatar richard67
richard67 - comment - 9 Jul 2019

The data type of column home of table #__template_styles, which is used in the join I've linked above, has been recently changed from varchar(7) DEFAULT '0' NOT NULL to tinyint(1) unsigned NOT NULL DEFAULT 0 with PR #24595 .

=> Ping @alikon

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019
avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

@alikon check what you have done here

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

Yes reverting to char(7) fixes the issue

avatar richard67
richard67 - comment - 9 Jul 2019

Yes, the problem is that the column home of template style table can either hold things like ' 0' or '1' or - when made default for a language, the language code for that language, e.g. 'de-DE'.

So issue #23188 was wrong, and therefore also PR #24595 was wrong.

@alikon Just for info, not to blame.

@infograf768 Regarding your comment in issue 23188: now you know the answer ;-)

avatar richard67
richard67 - comment - 9 Jul 2019

I mean if even Jean-Marie did not know about that, then it was really a very hidden thing to have also language codes in that column.

avatar richard67
richard67 - comment - 9 Jul 2019

@alikon @wilsonge now we might have a problem: When we make PR to change data type of that column back to before, we have 2 schema updates, one with the wrong change from PR #24595 , and a new one with changing data type back. This will make the database schema checker toggle with complaining that the column with this and this format is missing: First it will complain new column definition is missing, then after fix it will complain old column definition is missing, after fix again new is missing and so on and so on. I will check that, but I am almost sure this will happen. Only way out would be either to rename that column, which means change code, too, or to betray the schema check somehow (no idea yet, how).

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

Yes @richard67 i just checked the code of the com_templates model, you are right this is the problem and need to be reverted.

avatar richard67
richard67 - comment - 9 Jul 2019

@joeforjoomla As I wrote in previous comment, it could be a problem to simply revert with schema update script.
To be honest, it is also my fault: I did not test 3.9.9 RC well enough. If I had done that, I could have reported this error here before release. (blush)

avatar richard67
richard67 - comment - 9 Jul 2019

I will prepare PR to change back and will test here if it will be a problem with schema update like I suppose, or if we are lucky and it will work.

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

Not to blame anyone @richard67 i've even tested the RC for 1 week but didn't notice this issue.

avatar richard67
richard67 - comment - 9 Jul 2019

Sure, but I could bite myself in my ass now, as we say here in Germany.

avatar joeforjoomla
joeforjoomla - comment - 9 Jul 2019

Ahahah... however i think that it's very confusing for users so probably needs to be fixed now

avatar richard67
richard67 - comment - 9 Jul 2019

We have a problem because there is an index on columns client_id, home.

Due to how the database schema checker (Extensions -> Manage -> Database) works, we can't just drop the index and add it back. That problem was discussed between me and @alikon in PR #24595 for old index idx_home, too, and result was that we decided not to add back the old index but the new combined one instead. But the new one makes sense. Either we add it back with a new name which does not fit to our index naming scheme, or we don't add it back and it can never be added an index on that table with that index name unless the databa schema checker is at some future day enabled to handle such stuff. @alikon Please advise what to do.

Regarding column data type we can handle by making new schema update for chaning it back plus removing the SQL in the old schema update from PR #24595 which changed it in the wrong way, so the schema schecker does not find that old SQL.

But with the index it is not that easy, we can only add it back with a new name, e.g. idx_client_id_home_2 or idx_client_id_home_new.

avatar HLeithner
HLeithner - comment - 9 Jul 2019

@richard67 new name is not a problem. I already removed Joomla! 3.9.9 from the update server and I'm waiting for you PR.

avatar brianteeman
brianteeman - comment - 9 Jul 2019

I still managed to do an update 2 minutes ago

avatar mbabker
mbabker - comment - 9 Jul 2019

Your "remove from update server" pull request isn't merged, or deployed, unless you're manually making changes on the server (in which case I know who to blame for one of the multilingual .org sites having an incorrectly restored 3.7 database snapshot 😈)

avatar HLeithner
HLeithner - comment - 9 Jul 2019

Sorry didn't merged it.... now it's merged...

avatar Milglius
Milglius - comment - 9 Jul 2019

its ok now you can't update

avatar richard67
richard67 - comment - 9 Jul 2019

PR is #25484 . Haven't tested it myself yet. Will do soon.

avatar richard67
richard67 - comment - 9 Jul 2019

Closing as having PR.

avatar richard67 richard67 - change - 9 Jul 2019
Status New Closed
Closed_Date 0000-00-00 00:00:00 2019-07-09 20:20:14
Closed_By richard67
avatar richard67 richard67 - close - 9 Jul 2019
avatar infograf768
infograf768 - comment - 10 Jul 2019

@infograf768 Regarding your comment in issue 23188: now you know the answer ;-)

Indeed. And, unhappily I never tested the following PR and fixed database after this was merged. Sorry.
#24595

Add a Comment

Login with GitHub to post a comment