Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support both Meteor.Collection.ObjectID() and the default Meteor ID strings #1

Merged
merged 13 commits into from Aug 27, 2014
Merged

Conversation

leoetlino
Copy link
Contributor

Meteor seems to be using two different formats as the item's ID:

  • Meteor.Collection.ObjectID() - this is also what MongoDB uses by default.
  • Meteor's ID strings - this is what Meteor uses by default when inserting elements.

This pull request simply aims to have ngMeteor support both types, since some projects use ObjectID(), and others might use Meteor's default ID strings.

This fixes an issue in ngMeteor-collections.js that would cause
upserting to fail because of an invalid ObjectID being passed to
collection.update() (as first argument).
This fixes an issue in ngMeteor-collections.js that would cause
removing to fail because of an invalid ObjectID being passed to
collection.remove() (as first argument).

Same fix for upserting.
There seems to be two types of ObjectId:
- A simple string
- Meteor's ObjectId.

This commit allows ngMeteor to handle both types.
Merge urigo/ngMeteor v0.2
…r v0.2"

This reverts commit 27382bb, as it
seems to be causing issues on 0.8.3 and on versions prior to 0.8.2.
@Urigo
Copy link
Owner

Urigo commented Aug 25, 2014

Thanks @leoetlino !
Small question - why did you add - api.use('jquery', 'client'); ?

@leoetlino
Copy link
Contributor Author

Sorry, seems that this got in the pull request. Should have been separated.

Well, I added it because angular-ui's sortable requires jQuery to be loaded before AngularJS. As it isn't possible to change the load order easily with Meteor, the easiest way is to have ngMeteor load jQuery, then we can be sure it is loaded before AngularJS.

@Urigo
Copy link
Owner

Urigo commented Aug 26, 2014

@leoetlino Cool. we are testing your fork on our team and will merge it tomorrow or the day after.
Thank you so much!

@leoetlino
Copy link
Contributor Author

Actually, I should really thank you for testing it. It works for me, might
break something for someone else ;)

2014-08-26 12:47 GMT+02:00 Uri Goldshtein notifications@github.com:

@leoetlino https://github.com/leoetlino Cool. we are testing your fork
on our team and will merge it tomorrow or the day after.
Thank you so much!


Reply to this email directly or view it on GitHub
#1 (comment).

$$hashKeys will sometimes cause upserting to fail with the below error:
> Error: key $$hashKey must not start with '$'
@Urigo
Copy link
Owner

Urigo commented Aug 27, 2014

@leoetlino Good job! we tested it and went through the code. I'm going to merge it now.
About the JQuery, We had the same problems and it's a nice solution.
I'm going to open an issue that says that right now we have to use JQuery and we need to add the option to cancel it in case someone will want to use only angular's JQuery lite and not JQuery.
Thanks again

Urigo added a commit that referenced this pull request Aug 27, 2014
Support both Meteor.Collection.ObjectID() and the default Meteor ID strings
@Urigo Urigo merged commit bea4643 into Urigo:v0.2 Aug 27, 2014
Urigo pushed a commit that referenced this pull request Apr 13, 2015
Urigo pushed a commit that referenced this pull request May 8, 2015
Urigo pushed a commit that referenced this pull request May 26, 2015
@Urigo Urigo mentioned this pull request May 26, 2015
33 tasks
Urigo pushed a commit that referenced this pull request Jul 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants