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

EZP-28950 Correct legacy instructions, site.ini is the best place for DB charset #2337

Merged
merged 1 commit into from
May 24, 2018

Conversation

glye
Copy link
Member

@glye glye commented May 23, 2018

Question Answer
JIRA issue EZP-28950
Bug/Improvement bug
New feature no
Target version 7.2
BC breaks no
Tests pass doc only
Doc needed yes

The original PR #2277 had this change in i18n.ini, for reasons unknown/forgotten. It should work fine according to docs, and no problem was found in testing. But @emodric reported failures due to the mysql utf8mb4 charset being expected in ini-files and templates. As he pointed out, site.ini is the right/best place for the DB charset, since you may want to have different charset in db and elsewhere.

Refs:
https://github.com/ezsystems/ezpublish-legacy/blob/master/settings/site.ini#L82
https://github.com/ezsystems/ezpublish-legacy/blob/master/settings/i18n.ini#L23
https://doc.ez.no/eZ-Publish/Technical-manual/4.x/Reference/Configuration-files/site.ini/DatabaseSettings/Charset
https://doc.ez.no/eZ-Publish/Technical-manual/4.x/Reference/Configuration-files/i18n.ini/CharacterSettings/Charset

@glye
Copy link
Member Author

glye commented May 23, 2018

Seems that for legacy to fully work, we have to add utf8mb4 to the encoding table, here: https://github.com/ezsystems/ezpublish-legacy/blob/d76ce9824dabaceeefa77530c8c611e447d1b109/lib/ezi18n/classes/ezcharsetinfo.php#L153

@gggeek
Copy link
Contributor

gggeek commented May 23, 2018

I'd say that db charset and app charset are two separate things (not recommended of course, but it could happen a lot in far-east countries). So keep separate settings in i18n and Database confi8g.

@emodric
Copy link
Contributor

emodric commented May 23, 2018

@gggeek We have to keep separate config anyway, because config from i18n.ini is used for templates, INI files and what not.

@emodric
Copy link
Contributor

emodric commented May 23, 2018

PR on legacy at ezsystems/ezpublish-legacy#1361

@glye glye requested review from alongosz and andrerom May 23, 2018 11:32
@glye
Copy link
Member Author

glye commented May 23, 2018

@gggeek Yes, we'll keep the settings separate for sure. This PR is just about fixing upgrade instructions.

@glye glye removed the request for review from alongosz May 24, 2018 12:20
@glye glye merged commit b2a1ffd into master May 24, 2018
@glye glye deleted the ezp28950_correct_legacy_doc branch May 24, 2018 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants