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

Repair "Norwegian Bokmål" string #21813

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

christianwach
Copy link
Member

Overview

Fixes this issue on Lab so that "Norwegian Bokmål" renders correctly.

@civibot
Copy link

civibot bot commented Oct 12, 2021

(Standard links)

@civibot civibot bot added the master label Oct 12, 2021
@eileenmcnaughton
Copy link
Contributor

I guesss we should also add an upgrade script for existing dbs

@christianwach
Copy link
Member Author

@eileenmcnaughton Ah, yes. Hmm, I have never looked at that process before. Any docs or a bare-bones example I could look at?

@demeritcowboy
Copy link
Contributor

Also the string is in sql/civicrm_generated.mysql - you don't need to regenerate the whole file just a direct edit would be ok here.

The upgrade part is a bit tricky because (a) you don't want to update it if they've customized it, and (b) if they installed civi in non-english then the string you're looking to verify in (a) might be translated. But it doesn't have to be perfect - just don't update if no match. So I think something like

$sql = CRM_Utils_SQL::interpolate('UPDATE civicrm_option_value SET label = @newLabel WHERE option_group_id = #group AND name = @name AND label IN (@oldLabels)', [
'name' => 'nl_NL',
'newLabel' => ts('Dutch (Netherlands)'),
// Adding check against old label in case they've customized it, in which
// case we don't want to overwrite that. The ts() part is tricky since
// it depends if they installed it in English first.
'oldLabels' => ['Dutch', ts('Dutch')],
'group' => CRM_Core_DAO::singleValueQuery('SELECT id FROM civicrm_option_group WHERE name = "languages"'),
]);
CRM_Core_DAO::executeQuery($sql);

@colemanw
Copy link
Member

@christianwach - can you take a stab at ^

@christianwach
Copy link
Member Author

Sure thing... I'll try when I get a moment.

@eileenmcnaughton
Copy link
Contributor

@christianwach not letting you off the hook for ^^ but I'm gonna merge this in the meantime. It does improve 'something' and it means when you put up the upgrade script it will be on the front page. Given our current PR overload I think that's a good thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants