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 missing 'on behalf of' and honoree labels in multilingual #20482

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

MegaphoneJon
Copy link
Contributor

https://lab.civicrm.org/dev/translation/-/issues/69

Overview

When converting to multilingual, the monolingual strings become the default value in your multilingual installation. However, these labels are stored differently, and the default is to have no label at all. It also generated an "undefined index" notice.

Before

Selection_1109

After

Selection_1108

Technical Details

Previously this code read, "if monolingual, do X. If multilingual and a localized string exists, do Y."

Now it reads, "If multilingual and a localized string exists, do Y. Otherwise, do X."

@civibot
Copy link

civibot bot commented Jun 2, 2021

(Standard links)

@civibot civibot bot added the master label Jun 2, 2021
@demeritcowboy
Copy link
Contributor

To get my head around this and see which situations needed to be tested and which hadn't changed, I made a table. Here, "X" means the code block with the "monolingual" comment, and "Y" means the code block with the "multilingual" comment. Also to make it extra fun I replaced !empty with positives. So there's 2 situations that come up:

  • multilingual where there isn't a local string but there is a default (I think this is the bugfix)
  • single-lingual where there is a local string but no default (now it does nothing - so need to check what that means - I'm not clear at the moment if this situation is even possible)

BEFORE:

  $multilingual not $multilingual
$jsonDecode[$tsLocale] and $jsonDecode[default] Y X
not $jsonDecode[$tsLocale] and $jsonDecode[default]   X
$jsonDecode[$tsLocale] and not $jsonDecode[default] Y Y
not $jsonDecode[$tsLocale] and not $jsonDecode[default]    

AFTER:

  $multilingual not $multilingual
$jsonDecode[$tsLocale] and $jsonDecode[default] Y X
not $jsonDecode[$tsLocale] and $jsonDecode[default] X X
$jsonDecode[$tsLocale] and not $jsonDecode[default] Y  
not $jsonDecode[$tsLocale] and not $jsonDecode[default]    

@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
      • It's preexisting but the way you enter alternate language translations on the contribution page setup is confusing. For some fields it's the "A" popup icon. For other fields you need to reload the page in the other language first.
    • [r-doc] PASS
    • [r-run] PASS
      • Fixes the bug and otherwise seems ok. I didn't see a real-life situation that corresponded to the second changed scenario.
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS
    • [r-maint] ?
      • A nice-to-have might be to pull that code block out into its own function and test it against that table above.
    • [r-test] Pending
      • Passed before but jenkins retest this please since it's been a while.

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Jul 24, 2021
@demeritcowboy demeritcowboy merged commit 3545a81 into civicrm:master Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants