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

Fix user params after Joomla 3.9.7 with selected french help site #25177

Merged
merged 3 commits into from Jun 11, 2019

Conversation

Projects
None yet
6 participants
@HLeithner
Copy link
Member

commented Jun 11, 2019

Pull Request for Issue #25176

Summary of Changes

Tries to fix user table if json got corrupted.

@alikon

@richard67

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

The old schema updates 3.9.7-2019-05-16.sql should be corrected, too, so we in future don't create the error which new schema updates provided by this PR shall fix on other sites. Problem is that I don't know if the helpsite parameter always is placed somewhere in the middle of the json, or if it also can be first or last parameter in the list.

@mbabker

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

I still say the param shouldn't be touched. Empty the 3.9.7-2019-05-16 changeset and have this change to fix the broken database fields. It was getting rid of the param that broke things in the first place and it was an unnecessary action.

@richard67

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

@mbabker

... have this change to fix the broken database fields.

You mean like in this PR, with new schema update files? Or just as howto in a forum?

Is the change in this PR here safe? Or can it be that the helpsite parameter is not in the middle but at the beginning or the end of the json list so we need a more sophisticated replacement?

@HLeithner

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

I removed the content from the old sql file as michael suggested.

@alikon

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

yes make sense to remove the buggy replace imo

@mbabker

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Should be good now.

@richard67

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

If helpsite can also be at beginning or end of the json, we also have to replace {, with {and ,} with }.

@SniperSister

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

I tested the patch on an updated 3.9.7, works fine and restores the JSON

@HLeithner

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

normally the helpsite is between 2 other parameters I don't think that it's possible that the ,, are at the beginning or the end...

@richard67

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

Should the obsolete schema updates which are deleted with this PR also be deleted on already updated sites, i.e. added somehwre here https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_admin/script.php#L1989 ?

@HLeithner

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

It will be overridden anyway, not sure if it makes sense to remove it.

@mbabker

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

It's fine as an empty file. Or change the file contents to have a SQL comment that way it's clear why there are no queries in it.

# Query removed, see https://github.com/joomla/joomla-cms/pull/25177
@richard67

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

I like Michael's suggestion in his previous comment, have a comment in the wrong sql file.

@alikon

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

lgtm

@HLeithner

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

I added the comment.

@alikon

alikon approved these changes Jun 11, 2019

@richard67

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

I have tested this item successfully on 872c9ce

Tested with 3.9.7 site with user having parameters with error caused by wrong helspite removal.


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

@richard67
Copy link
Contributor

left a comment

Lgtm

@alikon

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2019

I have tested this item successfully on 872c9ce


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

@HLeithner HLeithner merged commit 941631d into joomla:staging Jun 11, 2019

4 of 5 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
Hound No violations found. Woof!
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/drone/pr Build is passing
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@HLeithner HLeithner added this to the Joomla 3.9.8 milestone Jun 11, 2019

@HLeithner

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

thx for the help, release is out.

@HLeithner HLeithner deleted the HLeithner:fix-french-helpsite-removal branch Jun 11, 2019

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.