reactioncommerce / reaction

Mailchimp Open Commerce is an API-first, headless commerce platform built using Node.js, React, GraphQL. Deployed via Docker and Kubernetes.
https://mailchimp.com/developer/open-commerce/
GNU General Public License v3.0
12.35k stars 2.17k forks source link

Not able to add tag in languages other than English #384

Closed tradward closed 9 years ago

tradward commented 9 years ago

When I try to add tag in Chinese or Japanese, the input edit text just reset to the default "Add tag". What might be wrong?

gouthamve commented 9 years ago

Looks like another error with the check package.

Error in terminal:

I20150605-22:13:15.079(5.5)? Exception while invoking method 'updateProductTags' Error: Slug is required
I20150605-22:13:15.079(5.5)?     at getErrorObject (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/aldeed_collection2.js:393:15)
I20150605-22:13:15.080(5.5)?     at [object Object].doValidate (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/aldeed_collection2.js:376:13)
I20150605-22:13:15.080(5.5)?     at [object Object].Mongo.Collection.(anonymous function) [as insert] (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/aldeed_collection2.js:178:31)
I20150605-22:13:15.080(5.5)?     at [object Object].Meteor.methods.updateProductTags (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/reactioncommerce_core.js:4827:25)
I20150605-22:13:15.080(5.5)?     at /Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/check.js:127:16
I20150605-22:13:15.080(5.5)?     at [object Object]._.extend.withValue (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/meteor.js:989:17)
I20150605-22:13:15.080(5.5)?     at Object.Match._failIfArgumentsAreNotAllChecked (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/check.js:126:41)
I20150605-22:13:15.081(5.5)?     at maybeAuditArgumentChecks (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/ddp.js:2442:18)
I20150605-22:13:15.081(5.5)?     at /Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/ddp.js:1476:20
I20150605-22:13:15.081(5.5)?     at [object Object]._.extend.withValue (/Users/goutham/reaction-devel/.meteor/local/build/programs/server/packages/meteor.js:989:17)
I20150605-22:13:15.081(5.5)? Sanitized and reported to the client as: Slug is required [400]
gouthamve commented 9 years ago

Actually, its speakingUrl and consequently getSlug doesn't support chinese.

https://github.com/reactioncommerce/reaction-core/blob/7dfe11b2ff710029662e2941fc4807e73f14f889/server/methods/methods.coffee#L101-103

The line produces this: { slug: '', name: '标签' }.

gouthamve commented 9 years ago

We could use this: https://github.com/lovell/limax. Its has more languages support (i18n) and is api compatible with speakingurl. I could wrap it a meteor package like the ongoworks:speakingurl. Looking for feedback.

aaronjudd commented 9 years ago

@Gouthamve let's test it.. but it sounds like a good idea. I'm happy to wrap limax and publish the package as well.

aaronjudd commented 9 years ago

I notice limax is missing Arabic, Thai at this moment, and has some issues with Chinese.

aaronjudd commented 9 years ago

See: https://github.com/reactioncommerce/reaction-core/pull/189 and https://gitter.im/reactioncommerce/reaction?at=56170e8199bbd76f0f306f88

If you attempt to add a chinese character, and perhaps any foreign character the input value is not received in tags.js.

See: https://github.com/cooloney/reaction-core/blob/fix_tag_issue/client/templates/products/productDetail/tags/tags.js#L62

aaronjudd commented 9 years ago

The problem with limax, at least at first glance is no Thai or Arabic support (which we have now with speakingUrl) See: https://github.com/lovell/limax/issues/1

I'm sure @lovell would appreciate a PR that added support as well.

I'm looking at https://github.com/dodo/node-slug so see if this is an alternate that will work.

aaronjudd commented 9 years ago

FYI - steps to replicate, just switch from English to Chinese in Languages menu. Input anything into tag. Doesn't work. Switch back to English. Works.

cooloney commented 9 years ago

Actually, you don't need to switch languages in the menu, just inputing some Chinese in tag will cause this issue.

lovell commented 9 years ago

Hello, limax maintainer here. It uses the excellent speakingurl under the hood (for Latin and Cyrillic scripts) then adds in support for Japanese Kana and Mandarin Chinese.

"Thai or Arabic support (which we have now with speakingUrl) "

It looks like speakingurl has added support for Arabic since I last looked, which means that should now "just work" with limax.

As you point out, support for the transliteration of Thai scripts has previously been requested and would make a most welcome addition, ideally in the form of another module upon which it could depend.

aaronjudd commented 9 years ago

@lovell thanks for the update. The only barrier I see to using Limax is an easy way to add client support to Limax. I've got a meteor package written that we can publish with Limax, but ideally I could do something similar to the https://github.com/ongoworks/meteor-speakingurl package were there is both client and server support. Your package is far better maintained and tested versus the Transliteration package that I'm testing as a replacement, but this package does seem to work ok for both purposes.

lovell commented 9 years ago

@aaronjudd limax ultimately depends on jieba for Chinese word segmentation, a process that extracts the words from a string of characters before they can be "Romanised" via Pinyin. This relies on C++ so, whilst very fast and accurate, won't work client-side.

The transliteration module you mention doesn't appear to split Chinese chars into separate words, something you'd probably want for things like SEO.

If anyone is aware of a well-maintained, pure-JavaScript Pinyin module that handles word separation then please do let me know :)