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

After update from 3.9.8 to 3.9.9 template styles list on multilingual site shows duplicate items #25482

Closed
richard67 opened this issue Jul 9, 2019 · 29 comments

Comments

Projects
None yet
8 participants
@richard67
Copy link
Contributor

commented Jul 9, 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.

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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.

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

@infograf768 Can you reproduce this?


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

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

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

image

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 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

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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.

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 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.

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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.

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

@alikon check what you have done here

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

Yes reverting to char(7) fixes the issue

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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 ;-)

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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.

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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).

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 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.

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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)

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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.

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

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

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

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

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

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

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 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.

@HLeithner

This comment has been minimized.

Copy link
Member

commented Jul 9, 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.

@brianteeman

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

I still managed to do an update 2 minutes ago

@mbabker

This comment has been minimized.

Copy link
Member

commented Jul 9, 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 😈)

@HLeithner

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

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

@Milglius

This comment has been minimized.

Copy link

commented Jul 9, 2019

its ok now you can't update

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

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

@richard67

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

Closing as having PR.

@infograf768

This comment has been minimized.

Copy link
Member

commented Jul 10, 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.