These are chat archives for reactioncommerce/reaction

28th
Apr 2015
Spencer Norman
@spencern
Apr 28 2015 02:29
@aaronjudd Running into some snags trying to finish up the Foundation port. Biggest issue right now is that many packages are responsible for structure and styling in addition to content. Payment packages are what is bringing this up, but I think it's going to be an issue with any non-core package.
can you think of a better way to deal with this than rewriting each of these packages with a Foundation version?
Spencer Norman
@spencern
Apr 28 2015 02:44
A makeshift temporary solution that I'm currently using is to extend some of the essential bootstrap classes that are used (block grid, buttons, etc). Will add a decent chunk of unnecessary CSS though.
Bogi
@boboci9
Apr 28 2015 12:51
Hi @aaronjudd , I have been experimenting with the transformed Cart collection to find a way to personalize and extend the Cart calculations withouth having to overwrite the functions defined in the collections.coffee. Do you have any propositions how to do that?
I tried transforming again the collection but it will loose the original functions
I have some ideas with using object prototypes but it would mean some changes to the RC as well but if you have some better ideas I would try your way or if not I can PR my ideas to see what you think
Spencer Norman
@spencern
Apr 28 2015 16:18
@aaronjudd, do you have a preference for how things are namespaced within ReactionCommerce? snake_case or camelCase or something else? (e.g. is one of these better: foundation_layout vs foundationLayout)
Aaron Judd
@aaronjudd
Apr 28 2015 16:19
@spencern added response in #370 - short answer is “not really, but maybe we can do something to make it manageable"
Spencer Norman
@spencern
Apr 28 2015 16:21
Saw that, thanks for the thoughtful response.
Aaron Judd
@aaronjudd
Apr 28 2015 16:22
I prefer camelCase. we can’t use ‘-‘ dash (meteor limitation). I’ve avoided ‘_’ generally.
Spencer Norman
@spencern
Apr 28 2015 16:22
thanks
Aaron Judd
@aaronjudd
Apr 28 2015 16:23
I suppose if we wanted to do theme specific templates we could do paymentMethod_foundation as a case where maybe “_” would be acceptable
(or the other way around in camelCase foundationPaymentMethod) which is the style I follow now)
Spencer Norman
@spencern
Apr 28 2015 16:24
yeah, that's what I'm getting at. If you still prefer foundationPaymentMethod that's fine too
Aaron Judd
@aaronjudd
Apr 28 2015 16:26
well I was thinking we should rename all core templates to be coreTemplate - and this is how I’m namespacing now - so we should follow that - I’m not sure if _ would bite us again (like - did).
Spencer Norman
@spencern
Apr 28 2015 16:27
let's go with camelcase then. I'll use foundationTemplateand we will avoid future troubles
Aaron Judd
@aaronjudd
Apr 28 2015 16:27
:thumbsup:
Aaron Judd
@aaronjudd
Apr 28 2015 16:35
@boboci9 what have you tried? I’m looking at it now
@boboci9 I like your proposed change, that works
Bogi
@boboci9
Apr 28 2015 16:41
great :)
:+1:
Aaron Judd
@aaronjudd
Apr 28 2015 16:43
:clap: merged
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 17:00
@boboci9 @aaronjudd I did not get what the PR is doing. Like can it be used for #362 or #160 ?
Bogi
@boboci9
Apr 28 2015 17:03
It can be used to extend the Cart collection if you want to add additional functions or for example change the shipping calculations for the cart
Aaron Judd
@aaronjudd
Apr 28 2015 17:03
@Gouthamve just controlling the cart calculations - so you could override those. You could add the misc charges of #362 there I suppose
@spencern I’ve been thinking about testing https://atmospherejs.com/useraccounts what do you think? it’s either this or at some point we need to rewrite the accounts package for our own use
Spencer Norman
@spencern
Apr 28 2015 17:13
hadn't seen those yet, but I'll look into it. Definitely need something that's more extensible than the current meteor accounts package
Will this help solve #348 ?
Aaron Judd
@aaronjudd
Apr 28 2015 17:24
honestly, I’m not sure - I just took a quick look at the bootstrap demo, and at first glance it might have the same issue (when there are multiple instances of the template)
but at least there are versions for each ui framework
Spencer Norman
@spencern
Apr 28 2015 17:25
Yeah, that's definitely positive.
Aaron Judd
@aaronjudd
Apr 28 2015 17:25
we can always contribute back fixes needed for multiple instance - probably easier than getting the meteor core package updated ;-)
Spencer Norman
@spencern
Apr 28 2015 17:25
yep
Aaron Judd
@aaronjudd
Apr 28 2015 17:28
yay! I didn’t notice this in 1.0.4
<ul>
{{#each person in people}}
  <li>{{person.name}} from {{people.groupName}}</li>
{{/each}}
</ul>
Spencer Norman
@spencern
Apr 28 2015 17:29
wooo! so nice.
Bogi
@boboci9
Apr 28 2015 20:28
I still have no luck in starting the reaction-core tests my velocity html reporter page is stuck in Loading though the reaction core tests files are loaded I can see them in the show files log, does anyone have any ideas about what my issue could be?
Aaron Judd
@aaronjudd
Apr 28 2015 20:32
I’ve not experienced that, but there was a recent update to the velocity packages - maybe try a meteor update?
Aaron Judd
@aaronjudd
Apr 28 2015 20:42
random: food for thought: meteor/meteor#4247
Spencer Norman
@spencern
Apr 28 2015 20:49
That would be nice. Looks like it's off in the not-on-the-roadmap-yet future, but would make some of the stuff we are dealing with right now a lot easier.
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 21:21
I think I know why it was there. The call removes the Cart session. Causing this to give an error: https://github.com/reactioncommerce/reaction-core/blob/c6116d3ad4c39f575cc02d5375fecbfa55fb2f67/client/workflows/cart/workflow.coffee#L93
Uncaught TypeError: Cannot read property '_id' of undefined
Well, after that line was removed, my COD package throws the new error. I am not sure if it is just my package or if it is affecting every payment package
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 21:44
I take it back. I think the error is elsewhere
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 21:54
Ah. Finally found the error. There is an error in the Mongo query. Patch on its way.
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 22:00
Sent the PR but I added the same sessionId querying to paymentAuth also. It made sense that both use the same.
Aaron Judd
@aaronjudd
Apr 28 2015 22:37
@Gouthamve ok so cart = Cart.findOne(sessionId: sessionId) would break now that we have multiple sessions, but we probably don't need cart = Cart.findOne(sessions: sessionId) as that logic is handled in the server publication
meaning it could probably just be cart = Cart.findOne()
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 22:42
Should I revert to Cart.findOne()? Like I saw that you changed it to use sessions in your previous edit of the file. So, I thought you might have a good reason to do it
And also is there a way I can extend the Schema of a collection in a package? If #362 doesn’t make sense to be put in Reaction-core, I could extend it in the package.
If that is not possible, I will probably create another collection and link to each COD order. I just love @boboci ’s PR. It makes cart handling damn simple.
Aaron Judd
@aaronjudd
Apr 28 2015 22:49
I’m thinking that I left that in while refactoring the sessions, but probably shouldn’t have
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 22:50
Hmm, I will revert and send a new PR fixing it.
Aaron Judd
@aaronjudd
Apr 28 2015 22:50
to extend a schema for another
ReactionCore.Schemas.myConfig = new SimpleSchema([
  ReactionCore.Schemas.PackageConfig
  {
    “myField":
      type: Boolean
      defaultValue: false
  }
])
no need to revert, just test the update and commit it
it will go in the same pr
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 22:51
Sorry, didnt know that
Aaron Judd
@aaronjudd
Apr 28 2015 22:52
for the schema, reference the paypal payment package, I think that’s the best example at the moment
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 22:52
I meant for Cart, we dont have Misc charges. I dont want to create Cart2 but instead extend Cart.
Sent the update. I successfully created a couple orders. :)
Aaron Judd
@aaronjudd
Apr 28 2015 22:55
the collection will allow anything, so it’s only the schema that enforces. in this example, there’s a core ReactionCore.Schemas.PackageConfig, but I’m extending the schema with the addition of myField, which will allow the collection that the schema is attached to (in this case Packages) to accept additional fields
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 22:57
But that extension is for ReactionCore.Schemas.myConfig I want to extend the schema of ReactionCore.Schemas.PackageConfig itself before it is attached to the collection
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 23:07
Ahh feeling much better. I force pushed a neat commit which resolves the issue.
Aaron Judd
@aaronjudd
Apr 28 2015 23:07
@Gouthamve I think what I’m suggesting should work for you see: aldeed/meteor-simple-schema#257
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 23:17
Ohh. Nice. Just to be clear: I extend the schema and then attach it to the collection. This will ensure that the collection now has the new schema.
I think I can finally fix: gouthamve/reaction-cod#3
Aaron Judd
@aaronjudd
Apr 28 2015 23:18
I don’t think you need to ‘re-attach’ it … I’m pretty sure that just doing the extend works
actually you might, not really sure - you’re not using autoform, and in the payment packages I’m setting the schema in the autoform.
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 23:27
I looked through collection2 code. We need to attach the additional part. Also we could completely replace the old schema by passing option replace = true. This is nice as it gives us more freedom.
Aaron Judd
@aaronjudd
Apr 28 2015 23:30
I do wonder if ‘misc’ should just go in the core payment schema anyways…. seems like a useful field ;-)
Goutham Veeramachaneni
@gouthamve
Apr 28 2015 23:33
:smile: