It would be good to allow the option to choose the country when inserting ASIN codes.
The country code could be printed in the markup, maybe as a CSS class.

CommentFileSizeAuthor
#12 add_support_for-2918260-12.patch17.48 KBmarcelovani
#10 add_support_for-2918260-10.patch17.06 KBmarcelovani
#7 add_support_for_multiple_locales-2918260-7.patch53.03 KBmarcelovani
#6 add_support_for-2918260-6.patch17.06 KBmarcelovani
#4 add_support_for-2918260-4.patch16.81 KBmarcelovani
#2 Screen Shot 2017-10-31 at 17.40.34.png96.77 KBmarcelovani
#2 add_support_for-2918260-2.patch16.81 KBmarcelovani

Comments

marcelovani created an issue. See original summary.

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new add_support_for-2918260-2.patch16.81 KB
new Screen Shot 2017-10-31 at 17.40.34.png96.77 KB

I have done the work on the module, and it should look like the screenshot below

This patch will only work with the latest dev branch of Amazon module that contains support for multiple locales.

Status: Needs review » Needs work

The last submitted patch, 2: add_support_for-2918260-2.patch, failed testing. View results

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
Status: Needs work » Needs review
StatusFileSize
new add_support_for-2918260-4.patch16.81 KB

Fixed php errors

Status: Needs review » Needs work

The last submitted patch, 4: add_support_for-2918260-4.patch, failed testing. View results

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
Status: Needs work » Needs review
StatusFileSize
new add_support_for-2918260-6.patch17.06 KB
marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
StatusFileSize
new add_support_for_multiple_locales-2918260-7.patch53.03 KB

The last submitted patch, 6: add_support_for-2918260-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 7: add_support_for_multiple_locales-2918260-7.patch, failed testing. View results

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
Status: Needs work » Needs review
StatusFileSize
new add_support_for-2918260-10.patch17.06 KB

Uploaded wrong patch on the last comment

Status: Needs review » Needs work

The last submitted patch, 10: add_support_for-2918260-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented
StatusFileSize
new add_support_for-2918260-12.patch17.48 KB
marcelovani’s picture

marcelovani
Primary language Portuguese, Brazil
Location London
commented

Tried to make drupalci patch amazon module as part of the build but apparently its not supported
These tests will keep failing until #2920070: Locale database storage is resolved

dakala’s picture

dakala
he/him
Primary language English
Location Fairford, Glos
commented 
+++ b/amazon_wysiwyg.module
@@ -103,48 +125,29 @@ function amazon_wysiwyg_widget_callback() {
+  $items = amazon_item_lookup_from_web(array($asin), strtoupper($locale));

Why don't we use drupal_strtoupper() here?

Apart from this minor comment, code looks good. Thanks.

  • marcelovani committed 1fd2e05 on 7.x-2.x 
    #2918260 by marcelovani: Added support for multiple locations
dakala’s picture

dakala
he/him
Primary language English
Location Fairford, Glos
commented
Status: Needs work » Fixed

Merged in as part of the latest alpha release. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.