catalyst / moodle-local_cohortauto

Automatically add users into cohorts. (Previously moodle-auth_mcae.)
https://moodle.org/plugins/local_cohortauto
11 stars 14 forks source link

Translations are not available in AMOS #1

Closed germanvaleroelizondo closed 4 years ago

germanvaleroelizondo commented 4 years ago

Hi, This plugin can not be translated in AMOS. String concatenation is not allowed. Please check the plugins checklist for strings at https://docs.moodle.org/dev/index.php?title=Plugin_contribution_checklist&redirect=no#Strings .

Thanks in advance.

danmarsden commented 4 years ago

Whoops! - missed that in my review! Thanks for the report - we'll get that fixed next week!

DrCuriosity commented 4 years ago

My apologies for not catching more opportunities to internationalise the strings earlier. I removed some string hard-coding from the original plugin, but I appear to have added some of my own. I will be more careful about that in future.

From an initial look-through I have found these two things to fix, and I will work on them in the morning:

I will also conduct a more careful check once that is done.

If you can see any other places where correction is obviously needed, do please let me know and I will address them as promptly as I can. Thank you!

danmarsden commented 4 years ago

@DrCuriosity - Amos doesn't cope with string concatenation- eg 'line1'.'line2'

danmarsden commented 4 years ago

Eg: https://github.com/catalyst/moodle-local_cohortauto/blob/a4b0f464f1c2c9e8063c60e02acf544dc9a250aa/lang/en/local_cohortauto.php#L43

DrCuriosity commented 4 years ago

Understood. Will make that conversion, and do it as a PR for review.

DrCuriosity commented 4 years ago

Code updated, and version reuploaded to moodle.org. Is there anything further that needs to be done in order for AMOS to pick up the change?

danmarsden commented 4 years ago

nope that should be it - thanks David! - @germanvaleroelizondo - let us know if you have any further issues!

germanvaleroelizondo commented 4 years ago

Hi, Strings are now available in AMOS for translation. Thanks.