These are chat archives for reactioncommerce/reaction

9th
Oct 2015
Bryan Wu
@cooloney
Oct 09 2015 00:46
@aaronjudd and @mikemurray , I still can't add a tag when I add a new product as admin
Exception while invoking method 'products/updateProductTags' Error: Match error: Failed Match.OneOf or Match.Optional validation
    at checkSubtree (packages/check/packages/check.js:252:1)
    at check (packages/check/packages/check.js:40:1)
    at [object Object].Meteor.methods.products/updateProductTags (server/methods/products.js:569:5)
    at packages/check/packages/check.js:117:1
    at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
    at Object.Match._failIfArgumentsAreNotAllChecked (packages/check/packages/check.js:116:1)
    at maybeAuditArgumentChecks (livedata_server.js:1689:18)
    at livedata_server.js:708:19
    at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
    at livedata_server.js:706:40
Sanitized and reported to the client as: Match failed [400]
this is my fix
diff --git a/client/templates/products/productDetail/tags/tags.js b/client/templates/products/productDetail/tags/tags.js
index 2ba6a4b..3df3b1e 100644
--- a/client/templates/products/productDetail/tags/tags.js
+++ b/client/templates/products/productDetail/tags/tags.js
@@ -62,7 +62,7 @@ Template.productTagInputForm.events({
   "focusout .tags-input-select": function (event, template) {
     let val = $(event.currentTarget).val();
     if (val) {
-      let currentTag = Session.get("currentTag");
+      let currentTag = Session.get("currentTag") || '';
       return Meteor.call("products/updateProductTags", selectedProductId(),
         val, this._id, currentTag,
         function (error) {
Bryan Wu
@cooloney
Oct 09 2015 01:35
@aaronjudd @mikemurray looks like currentTagId is not used at all in updateProductTags method function
can we just simply remove the check() of it?
Aaron Judd
@aaronjudd
Oct 09 2015 01:36
I’m not able to replicate that. I just added a new product and added a tag without any issue. I also edited an existing product. As well as nav tags, just to be sure.
Bryan Wu
@cooloney
Oct 09 2015 01:37
Another issue is if I input a tag in Chinese character it will also fail like this.
Aaron Judd
@aaronjudd
Oct 09 2015 01:37
anything else that might be happening? such as a session refresh, code refresh, in between?
also, your fix won’t pass linting. ;-)
Bryan Wu
@cooloney
Oct 09 2015 01:37
Exception while invoking method 'products/updateProductTags' Error: Slug is required
    at getErrorObject (packages/aldeed_collection2/packages/aldeed_collection2.js:425:1)
    at [object Object].doValidate (packages/aldeed_collection2/packages/aldeed_collection2.js:408:1)
    at [object Object].Mongo.Collection.(anonymous function) [as insert] (packages/aldeed_collection2/packages/aldeed_collection2.js:177:1)
    at [object Object].Meteor.methods.products/updateProductTags (server/methods/products.js:612:25)
    at packages/check/packages/check.js:117:1
    at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
    at Object.Match._failIfArgumentsAreNotAllChecked (packages/check/packages/check.js:116:1)
    at maybeAuditArgumentChecks (livedata_server.js:1689:18)
    at livedata_server.js:708:19
    at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
Sanitized and reported to the client as: Slug is required [400]
or maybe tag has to be English words, -:)
Aaron Judd
@aaronjudd
Oct 09 2015 01:38
ok, this is dumb, but how do I add a chinese word to test? lol
Bryan Wu
@cooloney
Oct 09 2015 01:38
sure, that will be lower priority
back to the original tag issue, i can reproduce every time
Aaron Judd
@aaronjudd
Oct 09 2015 01:47
it’s ok, I was just looking for ways to replicate the tag issue. I’m able to repeat this by copying a word from weibo, but it fails to let me add anything (presumably because the input is not recognized), I’m wondering if your keyboard/locale,etc when typing english (or chinese), is some how creating a input that’s not understood. If you use “” you’re bypassing a required field in the product schema. I’ll take a look at it right now - but, but we might need to create an issue to document the steps to replicate this, as the answer might not be easy as it seems.
Bryan Wu
@cooloney
Oct 09 2015 01:50
sure, i failed to find a place to file a issue against core package. but it send out a pull request. and @mikemurray replied to me and suggested me to change the check in server side.
diff --git a/server/methods/products.js b/server/methods/products.js
index 53598c6..10f4de4 100644
--- a/server/methods/products.js
+++ b/server/methods/products.js
@@ -566,7 +566,8 @@ Meteor.methods({
     check(productId, String);
     check(tagName, String);
     check(tagId, Match.OneOf(String, null));
-    check(currentTagId, Match.Optional(String));
+    check(currentTagId, Match.OneOf(String, null, undefined));
+
     // must have createProduct permission
     if (!ReactionCore.hasPermission("createProduct")) {
       throw new Meteor.Error(403, "Access Denied");
something like this.
actually currentTagId is not used in that function
Aaron Judd
@aaronjudd
Oct 09 2015 01:59
I think you’re right, that’s not even used. The error is in the schema validation failing, again I think because of an input failure.
Mike Murray
@mikemurray
Oct 09 2015 02:21
@cooloney hmm, you're right. we should remove it if its not used
Aaron Judd
@aaronjudd
Oct 09 2015 02:22
I’m doing that, let’s not duplicate efforts. the issue is in autocomplete and in the jquery-tags-input methods (line 62, of tags.js).
Mike Murray
@mikemurray
Oct 09 2015 02:23
ok
Aaron Judd
@aaronjudd
Oct 09 2015 02:24
@cooloney - I’m going to close the PR. I”ll move the comments into an issue,so that we can track - this might be more of a @mikemurray thing to resolve anyways. but I’d like to make sure that we’re not “fixing" the wrong things. (i.e.: assigned to me until further notice. )
Aaron Judd
@aaronjudd
Oct 09 2015 02:40
@cooloney the existing issue for this is reactioncommerce/reaction#384. I’d be happy to take a patch that replaces speakingUrl with “limax” or some other Chinese/Japanese compatible slug library.
Bryan Wu
@cooloney
Oct 09 2015 05:04
@aaronjudd and @mikemurray sure, I will try to find a way to solve the Chinese issue, since our customer also needs a Chinese version.
Aaron Judd
@aaronjudd
Oct 09 2015 05:38
I’m testing a solution right now - I’d like to get a patch up if I can get a working solution quickly.
perpi
@perpi
Oct 09 2015 06:38
hrath2015
@hrath2015
Oct 09 2015 08:59
@aaronjudd Found 2 issues logged one as #482. Not able to create second. Not sure what is wrong.
Aaron Judd
@aaronjudd
Oct 09 2015 09:03
@perpi updated :clap:
hrath2015
@hrath2015
Oct 09 2015 09:04
Second one is where on a fresh build while checkout, not able to configure payment by clicking link. Before click router is http://localhost:3000/checkout and post it changes to http://localhost:3000/dashboard but no screen to Login nor any message.
Aaron Judd
@aaronjudd
Oct 09 2015 09:05
#482 is a quick fix, I’ll grab it first thing tomorrow
hrath2015
@hrath2015
Oct 09 2015 09:06
There is third one as well. I will see if I can log as issue in Github else will report here. It is already pretty late. Take care and GN
Aaron Judd
@aaronjudd
Oct 09 2015 09:07
on the second one, an issue would be awesome, should also be a quick fix, but not sure. - we’ll catch them and repair them really quick in the am. (2am here as well)
hrath2015
@hrath2015
Oct 09 2015 09:13
sure will do it.
Aaron Judd
@aaronjudd
Oct 09 2015 09:14
:thumbsup: thanks!
perpi
@perpi
Oct 09 2015 10:57
@aaronjudd It's needed to run a find&sed to solve url os all moved docs, as in: https://github.com/reactioncommerce/reaction-core/blob/development/docs/themes.md and
Aleksei Mironov
@alexmironof
Oct 09 2015 11:39
Hi folks! How do u think, is it possible to make variants behave like "color options" in this kind of shop http://www.mansurgavriel.com/products/large-wallet-calf-coated/rosa-rosa
Aaron Judd
@aaronjudd
Oct 09 2015 15:38
@perpi good catch, sort of got missed in all the updates. taken care of now.
@kefirchik do you mean having “color” swatches for the options?
Aleksei Mironov
@alexmironof
Oct 09 2015 18:04
@aaronjudd I mean I want to replace variants with this swatches
sorry i think this is dumb question
It is about global modularity, I should ask like this: is it possible to make exact copy of this shop with reaction
Aleksei Mironov
@alexmironof
Oct 09 2015 18:10
And i have another shop example I'll show it a bit later
sorry it is bad example ))) it is for printing on shirts
Spencer Norman
@spencern
Oct 09 2015 18:38
@aaronjudd is there any kind of changelog for 0.7/0.8 -> 0.9 in terms of core objects/methods? Just figured out ReactionCore.Events is now ReactionCore.Log but if there is any documentation on this, that would be extremely helpful :)
is that enough? I can add to it, if I missed anything (highly likely)
Aleksei Mironov
@alexmironof
Oct 09 2015 18:40
I need nearly the same thing but without editing the product. So I can only choose from existing "options" and mabe have an interface with steps like:
step one : choose a t-shirt
step two: choose print from options (refresh image and add price to total)
step three: choose option (with or without diamonds from swarovsky)
step four: choose option (add leather or silk belt)
etc...
Aaron Judd
@aaronjudd
Oct 09 2015 18:40
@kefirchik yes, possible - as each element on the product page is a “metacomponent” - i.e. , a template
Aleksei Mironov
@alexmironof
Oct 09 2015 18:41
@aaronjudd Thanks! I'll try it
Aaron Judd
@aaronjudd
Oct 09 2015 18:42
however I want to apply the new Package.layout templates to products in the very near future as well - which would be a combination of configuration in the Shops.layout + some Product layouts. just to bring a higher scope modularity to the product page
Aleksei Mironov
@alexmironof
Oct 09 2015 18:43
Wow, nice! I start with example with color swatches and then Ill try to convince my colleague to make shop with custom product designer using reactioncommerce :smile:
Aaron Judd
@aaronjudd
Oct 09 2015 18:44
the product page was one of the very first pages I wrote, so it’s high time to get some love and additional product types to serve as an example for these kinds of mods - but really should be pretty straight forward once you dig in
the color swatches is something we’ve been thinking about as well, we never implemented “metadata” object at the variant level (it’s there in the schema, and you can see it being used elsewhere). That’s where I would store data for the option presentation, if the label /option value wasn’t enough
Aleksei Mironov
@alexmironof
Oct 09 2015 18:53
If i can add different images to options i think it will be easy to make composable product out of the box
Aaron Judd
@aaronjudd
Oct 09 2015 18:54
yes, each variant option has it’s own images (works that way now)
Aleksei Mironov
@alexmironof
Oct 09 2015 18:55
Thanks! Nice I'll try it
Aaron Judd
@aaronjudd
Oct 09 2015 18:55
there is metadata on the images themselves as well, you could just filter them if you wanted the variant option button themselves to show an image. could be an easy fun mod ;-)
Bryan Wu
@cooloney
Oct 09 2015 18:56
@aaronjudd you branch fixed the chinese tag input issue, works as expected, thanks a lot. will you pull in this fix?
Aaron Judd
@aaronjudd
Oct 09 2015 19:11
the url output is ok?
Bryan Wu
@cooloney
Oct 09 2015 19:12
hmmm, what's the url output?
Aaron Judd
@aaronjudd
Oct 09 2015 19:12
(when you click the “bookmark” icon in the tag it sets the product url, want to make sure that’s acceptable) - I did have a solution that would actually use the chinese characters in the url, but failed with the regex lookup we do on the url string)
Bryan Wu
@cooloney
Oct 09 2015 19:12
oh, let me check
Aaron Judd
@aaronjudd
Oct 09 2015 19:13
so the output now is “transliterated” I guess a phonetic english url?
REACTION___登場。.png
the issue we’re running into is that there are no client libraries that handle this as well as a server package like Limax- and we need to check the slug on typeahead -> which if it was a server call might be slower, heavier than I’d like
Bryan Wu
@cooloney
Oct 09 2015 19:17
yes, it works perfect
Aaron Judd
@aaronjudd
Oct 09 2015 19:18
ok, great
Bryan Wu
@cooloney
Oct 09 2015 19:18
if i created a Chinese tag
then a transliterated english URL will be created
and I clicked on that icon, it will go to the URL
Aaron Judd
@aaronjudd
Oct 09 2015 19:18
I tested with Arabic, Hebrew, Chinese and Japanese -> all work, I’m just not sure what that english output really means, lol
Bryan Wu
@cooloney
Oct 09 2015 19:19
actually the transiliterated english is quite good.
Aaron Judd
@aaronjudd
Oct 09 2015 19:19
ah, that’s awesome, glad to hear. I’ll merge it then :thumbsup:
Bryan Wu
@cooloney
Oct 09 2015 19:19
it's the use the some chinese input method I guess.
like we input chinese characters in computer but literally we is typing in English letters.
Aaron Judd
@aaronjudd
Oct 09 2015 19:20
all I could tell, is that the hebrew one looked like hebrew sounds :smile:
I didn’t know that, so the url output is similar to what you’d type in?
Bryan Wu
@cooloney
Oct 09 2015 19:21
let me learn som hebrew tonight and to some test for you heh
Aaron Judd
@aaronjudd
Oct 09 2015 19:21
lol, good plan!
Bryan Wu
@cooloney
Oct 09 2015 19:21
for example, if i input 苹果, which means apple in Chinese
the url will be product/ping-guo
Bryan Wu
@cooloney
Oct 09 2015 19:22
which is transliterated but not translated.
looks very smart .hah
Aaron Judd
@aaronjudd
Oct 09 2015 19:23
and this is acceptable for SEO?
Bryan Wu
@cooloney
Oct 09 2015 19:23
i think so,
SEO will use that to search
if search in Chinese '苹果', it will search pingguo as well as Apple i guess
Aaron Judd
@aaronjudd
Oct 09 2015 19:24
what about product in that url, should that be translated to chinese, and then transliterated? if so, let’s create an issue for that - it’d require more though, but I’m guessing we can get away with this for now?
Bryan Wu
@cooloney
Oct 09 2015 19:25
because pingguo is quite standard transliterated, we type pingguo in some chinese input method then we get chinese word '苹果'
Aaron Judd
@aaronjudd
Oct 09 2015 19:25
ok this sound like it’s working better than I expected, nice surprise
Bryan Wu
@cooloney
Oct 09 2015 19:25
product looks very normal to me
i don't think we need to transliterated that.
Aaron Judd
@aaronjudd
Oct 09 2015 19:26
This message was deleted
Bryan Wu
@cooloney
Oct 09 2015 19:27
thanks for fixing this so quick.
Aaron Judd
@aaronjudd
Oct 09 2015 19:29
:thumbsup: it’s a several month old ticket, and I want to make sure we’re supporting all the languages correctly - so I’m happy there was a decent solution. I’ll keep an eye out for a Limax client solution as well - as I still think it’s a better option, but this should be good for now.
Michael Jenny
@prinzdezibel
Oct 09 2015 20:02
@aaronjudd Were there any database schema changes since the alpha release?
I'd like to copy over my old database
but I'm experiencing strange things which I have not nailed down yet. (Products.find().fetch() returns not all products, only some of them)
Aaron Judd
@aaronjudd
Oct 09 2015 20:22
not to products, that I remember, just to orders / cart
maybe visibility is filtering?
Bryan Wu
@cooloney
Oct 09 2015 22:01
@aaronjudd one quick quesion, I normally use single quote for strings in JavaScript, but double quote for HTML. looks like in Reaction code, we should us doube quote
is the correct?
Spencer Norman
@spencern
Oct 09 2015 22:30
Inside of the ReactionCore.MethodHooks.beforeMethods is there a way to prevent the original method from running other than throwing an error?
or perhaps a better way to change the order processing workflow from a package than using hooks?
Aaron Judd
@aaronjudd
Oct 09 2015 22:31
@cooloney yes, (see eslintrc) I guess that’s a preference and I’m old school - multi -languages where real “double” quotes mean string - not apostrophe..
Spencer Norman
@spencern
Oct 09 2015 22:32
e.g. I need to change logic for how orders/inventoryAdjust works
Aaron Judd
@aaronjudd
Oct 09 2015 22:32
@spencern hmmmmmmm
Spencer Norman
@spencern
Oct 09 2015 22:36
@aaronjudd i’ve looked through the MethodHooks docs here: https://github.com/workpop/meteor-method-hooks/ and couldn’t find anything.
I might be able to mutate the orderId that’s passed in so that it doesn’t work the second time, but that feels kinda hacky
Aaron Judd
@aaronjudd
Oct 09 2015 22:36
I’m thinking that we might need to add something along the lines of “this.done” or something to the hooks themselves
Spencer Norman
@spencern
Oct 09 2015 22:37
where this.done would circumvent the original method call in some way?
Aaron Judd
@aaronjudd
Oct 09 2015 22:41
exactly - need to look into it some more @jshimko and I have discussed enhancing this, seems like a valid requirement
sorry I don’t have good answer, need to dig into it some more
happy to take a PR to improve the hooks, and there are about 10 of these hooks scripts floating around, I’m most curious about the way telescope has added hooks, maybe we can find a slightly more elegant implementation
Spencer Norman
@spencern
Oct 09 2015 22:45
I’ll start digging around and see if I can find anything that might help