Make moderation states translatable

I don't think there is a clear way to do that currently. Patches or ideas for approaches welcome.

Here's a patch.

i tried the patch in #2 and currently i have this: http://screencast.com/t/8pZA6L8pz0

i think the patch added translations for the select list (but odd i didn't need to translate these; they just appeared). the Oui was already there from earlier, the word Draft is still not translatable.

also, original post suggest translating "status labels" - i am not quite sure what that refers to. do you mean States?

Thanks for testing it! I seem to have missed one indeed, I've added it to the patch. And indeed this is translating the Moderation States. I will update the title so it is more clear.

I need to translate the states as well!

But I was thinking a bit differently than this patch:
1- How about adding a context to the strings in the t() function. Something like workbench_moderation:state:label:[state_machine_name]. Because the words used for labels are very likely to be used elsewhere, but with different translations.
2- I see in the patch that all calls to workbench_moderation_state_label() are being passed to t(). Why not just modify the workbench_moderation_state_labels() and the t() call inside it? I think that would be much cleaner and more manageable.

Here's a patch :)

Ok I just renamed the context. I figured since the module is part of the workbench package, then it would follow the 'workbench' text group as well.

I don't think translating the labels inside workbench_moderation_state_labels() is the right approach. It is an API change that could break existing implementations and it would make it impossible to retrieve the label in the original language. On API level I prefer to work with original language strings, and perform the translation at the end, before outputting to the page.

I do like the idea of providing a context, but then not going as far as providing a separate context for each state, one global "workbench moderation states" context should suffice.

For the context name, I'm not sure I fully understood your suggestion. The way I thought about it, is that workbench_moderation is really part of the workbench package, so it would make sense to start with text group 'workbench'. Each state has a label and a description, so I kinda think it makes sense to go with this degree of specification. So far, we would have:

- workbench:moderation_state:label:[state_machine_name]
- workbench:moderation_state:description:[state_machine_name]

And if something else gets added in the future, it would be a headache then changing the contexts. This would actually save effort later, and doesn't take much (if any) extra effort now.

As for the translating in workbench_moderation_state_labels(), you're right, it could break existing implementations. How about we add an optional argument $localize? Defaults to FALSE, but when set to TRUE, it will translate the labels. This would leave any existing implementation unaffected. And I think having the translation inside the function would save us a big chunk of redundant code (e.g. the context).

What do you think?

Well my guideline is always "How does Drupal core do it?", and passing a $localize argument is not something I think happens much in core. Calling t() when outputting strings is hardly redundant code, but actually is good Drupal coding practice in my eyes. It would be nice to get someone else's opinion on this.

What would you think of a "workbench:moderation_state" context? Isn't this enough? I'm not familiar with how translation contexts are usually implemented.

Yeah I guess you're right... not Drupal Core-ish at all! What I actually meant with the redundant code was the the context itself, rather than the use of t(). But I guess it wouldn't really be worth deviating from the Drupal way.

Regarding the context, I was too influenced by the way i18nstrings worked in D6 (full-fledged textgroups and contexts...etc). But I just had a look at http://drupal.org/node/1369936

....and it seems I was over-complicating things! A simple 'Workbench moderation states' would be enough. If at a later state, a more-complex context is needed, it can be handled in hook_update_N(). (But I don't see that happening anytime soon)

@pfrenssen
Thank you for your patch! It works excellent.

Marked #1350218: Add multilanguage support for states as duplicate.

Moderation state labels can be user defined, therefore using t() is not the ideal solution. Usage of i18n_string is recommended.

Added i18n_string support.

#15 seems not to integrate the previous patches

@hass: exactly, because previous patches followed the approach with t(), which is not the right one as state label and description can be user defined strings. t() should only be used on strings defined in code.

People who do not run i18n require the t().

@hass, do you see a t() around field labels, content type names, vocabulary names, etc. in Drupal core? :)

Excellent patch @iSoLate - works great, thanks.

Re-roll of the patch because of patch conflicts in the end of the file.

Moved the workbench_moderation_i18n_string_info function implementation close to the other changes in the file, because when the method is in the end of the file, there are conflicts with other patches that I am using at the moment.

In general, code added in the end of the file is more likely to get conflicted.

Using the patch #22 in production for almost half an year without any problems, therefore changing the status as reviewed.
Also hiding the patch files except #15 and #22 as they are working and equivalent in implementation.

BR,
Nikolay Dobromirov.

Missed to hide patch #2.

1. +++ b/workbench_moderation.admin.inc
   @@ -150,6 +150,9 @@ function workbench_moderation_admin_states_form_submit($form, &$form_state) {
   +      if (module_exists('i18n_string')) {
   +        i18n_string_update(array('workbench_moderation', 'moderation_state', 'label', $info['name']), $info['label']);
   +      }
   

   It seems like the order of the string context components is wrong. If you look at i18n_string_object::get_name() it says that the order is as follows:

       return $this->textgroup . ':' . $this->type . ':' . $this->objectid . ':' . $this->property;
   

   Seems like the last two values in the array should be swapped so it becomes:

   +        i18n_string_update(array('workbench_moderation', 'moderation_state', $info['name'], 'label'), $info['label']);
   

2. +++ b/workbench_moderation.module
   @@ -1248,7 +1248,12 @@ function workbench_moderation_state_labels() {
   +      if (module_exists('i18n_string')) {
   +        $labels[$machine_name] = i18n_string_translate(array('workbench_moderation', 'moderation_state', 'label', $machine_name), $state->label);
   +      }
   

   Same here, the two last array values should be swapped, so it becomes:

   array('workbench_moderation', 'moderation_state', $machine_name, 'label')
   

3. +++ b/workbench_moderation.module
   @@ -1256,6 +1261,19 @@ function workbench_moderation_state_labels() {
   +    'format' => FALSE, // This group doesn't have strings with format
   +    'list' => TRUE, // This group can list all strings
   +  );
   

   Put these inline comments above the corresponding lines and end them in a period.

Addressed my remarks from #25. While doing this I noticed that this patch is still missing something: it can implement hook_i18n_string_list() to make all strings available for translation at once.

Implemented hook_i18n_string_list(), so that the moderation states are imported when translators press the "Refresh" button. And added support for translating the moderation state descriptions while I was at it. These are currently not shown anywhere in vanilla Workbench Moderation, but it is nice to have this available for projects that might want to display the descriptions somewhere.

Thank you for this, in my case it seems that this patch makes the translation work for people with the permission 'bypass workbench moderation'. Other see the current state translated but not the other states. I think it must be in this part of the code:

if (user_access('bypass workbench moderation', $account)) {
    // Some functions expect an array of $state => $state pairs.
    $states = workbench_moderation_state_labels();
    unset($states[$current_state]);
  }
  else {
    // Get a list of possible transitions.
    $select = db_select('workbench_moderation_transitions', 'transitions')
      ->condition('transitions.from_name', $current_state)
      ->fields('transitions', array('to_name'))
      ->fields('states', array('label'));
    $select->join('workbench_moderation_states', 'states', 'transitions.to_name = states.name');

    $states = $select->execute()->fetchAllKeyed();

    // Checks whether the user has permission to make each transition. The
    // 'bypass workbench moderation' permission is accounted for in
    // workbench_moderation_state_allowed().
    if ($states) {
      foreach ($states as $machine_name => $label) {
        if (!workbench_moderation_state_allowed($account, $current_state, $machine_name, $node->type)) {
          unset($states[$machine_name]);
        }

But I can't see what I should adapt...

Hi, I removed a few lines from the patch because they were'nt doing anything, and failed to apply on version 1.3

Removed lines:

@@ -670,7 +670,7 @@ function workbench_moderation_node_update($node) {
 
   // Don't proceed if moderation is not enabled on this content, or if
   // we're replacing an already-published revision.
-  if (!workbench_moderation_node_moderated($node) || 
+  if (!workbench_moderation_node_moderated($node) ||
       !empty($node->workbench_moderation['updating_live_revision'])) {
     return;
   }

Thanks! That was a line ending fix that ended up in the patch and is indeed out of scope for this issue.

The patch works fine. A "translate" tab would facilitate the work, but such a feature could also be requested in a new issue, so this one could be committed and closed...

It doesn't work for me. Maybe I'm doing something wrong? I added patch and after that I tried to find this "Draft" or "Needs review" on page admin/config/regional/translate/translate and translate it, but there is no this string.

But this simple patch works for me

The t-string approach is incorrect - the patch in #29 does it correctly with i18n_string. Did you refresh the workbench moderation strings after applying the patch?

@joel_osc, yes I've refreshed it but it doesn't helped.

@eugene.ilyin, that approach is wrong, you should never run dynamic strings through t().

I just re-tested the patch from #29 and it works fine if you add the patch, flush caches (class registry) to bring in the workbench_moderation text group and the re-save your workbench moderation states (admin/config/workbench/moderation).

Uploading the patch from #29 again, since many people use the last available patch without reading the comments.

@joel_osc, did you happen to review the patch too? Could you set it to RTBC? This issue has been open for years, it's time to get it committed :)

re-save your workbench moderation states (admin/config/workbench/moderation).

@joel_osc, thank you for tip, I'll try to do it in Monday

Patch is working well... setting to RTBC.

Patch works for me!
Thank you @joel_osc and @pfrenssen.
I use russian language by default on my site and should name statuses on russian and translate them on english. Then it works. But it's really was no clear, because I always used t() for translation of the texts and the text always was on english language.

Please commit this patch

P.S. Can somebody give me a little info or link to page, where described why we should use i18n_string instead of t()?

@eugene.ilyin: it is explained in the documentation for t() why you shouldn't use dynamic strings :)

https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/t/7

Bumping to say thanks and a reminder to the maintainers to get this committed please :)

Thanks everyone!

I am having the same issue as the one yichalal mentioned in comment 28.

Everything displays fine for users with the permission "Bypass Moderation Restrictions", however for those without it, the drop down selection fields for the moderation state in node creation and moderation pages omit the translated labels for any state non-current.

I am using a patched 1.4 and tried the most recent dev (1 branch) without any luck.

function workbench_moderation_states_next($current_state, $account = NULL, $node) {

  if (user_access('bypass workbench moderation', $account)) {
    // Some functions expect an array of $state => $state pairs.
    $states = workbench_moderation_state_labels();
    unset($states[$current_state]);
  }
  else {

The patch translates labels in that function only if the user has the bypass permission. I added another if statement after the unset to translate the labels, and although not beautiful, it worked:

if ($states) {
      foreach ($states as $machine_name => $label) {
        if (!workbench_moderation_state_allowed($account, $current_state, $machine_name, $node->type)) {
          unset($states[$machine_name]);
        }else{
            if (module_exists('i18n_string')) {
              $states[$machine_name] = i18n_string_translate(array('workbench_moderation', 'moderation_state', $machine_name, 'label'), $label);
            }
        }
      }

Any help to tidy that is appreciated.