Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR’s picture

Title: Please declare your variables » Add support for Variable module

Seems like a lot more trouble than it's worth, and not necessary if multi-lingual variables is all you need since Ubercart already defines i18n_variables for the Internationalization module.

You could always write your own hooks for the Variable module - this doesn't have to be in Ubercart core, and my inclination is to not put it in unless there's a good use-case for doing so.

mgifford’s picture

I'm pretty sure that this is how user defined custom variables are being handled in D7. It's very much related to:
#582044: Checkout messages not available as multi-lingual values

TR’s picture

The Variable module is 9 years old - it's certainly not the new, D7 way of doing things. D7 core doesn't address internationalization of variables or database values. There was an aborted effort to make this happen in core, but it wasn't implemented and may not even get into D8. At least, I haven't seen anyone working on it. Ubercart uses a lot of variables, unlike core, so this is an issue that affects Ubercart more than others. We rely on the i18n module to handle our multi-lingual variable needs because there's no sense in trying to implement our own solution for what really should be a core Drupal capability.

mgifford’s picture

Status: Active » Postponed

I overlooked the start date & just saw that it only had a D7 version.

In D7 this would be done with $conf['i18n_variables'] - and ya, you're absolutely right that there was an effort to bring variables into D7 that wasn't successful. Think there's bigger thinking afoot for D8. Absolutely, gotta rely on other modules and not build a UC specific solution. That being said, i18n has a lot of modules within it or associated with it. It's a pretty complex beast. I just see Variables as a module associated with i18n that you need if you want to have control of user defined variables like say the maintenance module's messages (from core).

I do acknowledge that I need to dig into this a bit further so will push to see if we can get a best practice clarified & then get back here when this is better defined.

mgifford’s picture

Status: Postponed » Active
Issue tags: +i18n

Just adding 2 links for the use of the Variable module:

I'd still like to see something in the handbook, but in the interim the project page of i18n is pretty definitive - http://drupal.org/project/i18n

Requires the Variable module (Drupal 7)

longwave’s picture

Status: Active » Needs review
FileSize
3.9 KB

This patch is a proof of concept that will let you translate the six checkout completion message variables using i18n_variable.

You will need to apply the patch, download i18n and variable modules, and enable i18n_variable. Then at /admin/config/regional/i18n/variable select the variables you want to translate such as "Completion message header", and finally you will be able to change these variables per language at /admin/store/settings/checkout

longwave’s picture

Closed #582044: Checkout messages not available as multi-lingual values as duplicate as I think this can be solved here.

longwave’s picture

#834290-47: Address fields are no longer multilingual variables and cannot be translated has a patch to add address field support for the Variable module as well.

longwave’s picture

Status: Needs review » Active

Committed #6.

Back to active, as all instances of i18n_variables in 7.x are obsolete and should be replaced with Variable hooks.

mandreato’s picture

Status: Active » Needs review
FileSize
1.81 KB

Here's a patch to manage COD and checks policies of uc_payment_pack via i18n variables.
I've successfully tested COD.

longwave’s picture

Status: Needs review » Active

Thanks for working on this. I committed this with some minor changes; I set the actual defaults in uc_payment_pack.variable.inc, and removed uc_payment_pack_init() entirely as it is obsolete now.

Back to active to deal with the remaining cases.

mandreato’s picture

Status: Active » Needs review
FileSize
1.93 KB
longwave’s picture

Status: Needs review » Active

Thanks. Committed, with the addition of the other checkout instructions message.

I wonder if we should remove the alternative "Continue shopping" text option entirely, as you can translate that in the normal way already.

mandreato’s picture

To which "Continue shopping" text do you refer ?

There is the custom continue shopping text in admin/store/settings/cart (continue shopping element) which is the label used in the cart page to let the user return to the product list after adding one product.
This is not translatable.

There is the just patched Continue shopping message in admin/store/settings/checkout (completion messages) which is the text shown after the checkout is complete.

I think both are useful.

longwave’s picture

I meant the "Custom continue shopping text". If you leave this field blank, the default text "Continue shopping" is used, and this is then translatable through the interface translation UI at /admin/config/regional/translate/translate (or through String Overrides, if you are only using English) - so it seems like we don't need to allow custom text here.

mandreato’s picture

I see. Well, it depends if someone needs to put there a text different from "Continue shopping", maybe "Go back to catalog" or something like that. In that situation it would not be correct to translate "Continue shopping" to a localized text with different meaning.

But this is not my case... so feel free to remove the "Custom continue shopping text"...

longwave’s picture

The better supported way of changing any module-supplied text is String Overrides, that works without having to provide individual settings as Ubercart does here.

Looking through git history, it seems this feature was added in D5, along with custom text fields for various other snippets such as the Next and Submit buttons that have since been removed.

mandreato’s picture

Status: Active » Needs review
FileSize
1.87 KB

Here's a patch to manage two strings of uc_stock.

longwave’s picture

Status: Needs review » Active

Thanks again. Committed, with an additional description for the notification subject variable, and the complete removal of uc_stock_init().

nerdcore’s picture

Same, for two string in shipping/uc_quote

nerdcore’s picture

Status: Active » Needs review
rfay’s picture

IMO these should be rolled into a single patch and just get on with it. Without hook_variable_* we don't have any multilingual support.

Related patch to remove all the hook_init() cruft from D6: #2577915: Remove D6 cruft i18n_variables references in D7 version

  • longwave committed 7432ffb on 7.x-3.x authored by nerdcore
    Issue #1533286 by nerdcore: Add support for Variable module
    
longwave’s picture

Status: Needs review » Active

Committed #20. This only leaves:

uc_store/uc_store.module:229:  $conf['i18n_variables'][] = 'uc_store_name';
payment/uc_2checkout/uc_2checkout.module:43:  $conf['i18n_variables'][] = 'uc_2checkout_method_title';
payment/uc_credit/uc_credit.module:109:  $conf['i18n_variables'][] = 'uc_credit_fail_message';
payment/uc_credit/uc_credit.module:110:  $conf['i18n_variables'][] = 'uc_credit_policy';
uc_cart/uc_cart.module:215:  $conf['i18n_variables'][] = 'uc_cart_breadcrumb_text';