api boomark shows 1962 #1988
api boomark shows 1962 #1988
Conversation
|
Reviewing |
| // Get current user bookmark (should be only one API Bookmarks result available) | ||
| const userBookmarks = ApiBookmarks.findOne({ userId: Meteor.user()._id, apiIds: apiId }); | ||
| // Get API ID from instance data context | ||
| const apiId = instance.api._id; |
| // Get current user bookmarks, only if this API is bookmarked | ||
| const userBookmarks = ApiBookmarks.findOne({ | ||
| userId: Meteor.user()._id, | ||
| apiIds: apiId, |
There was a problem hiding this comment.
Also make sure to check case "undefined" by using if (apiId) structure before using apiId.
There was a problem hiding this comment.
Yes even i got it while testing , but didnt know how to fix it .. will try what you have suggested.
There was a problem hiding this comment.
This is resolved by fixing line 36. Change line 36 to instance.data.api._id
| @@ -6,16 +6,28 @@ import { FlowRouter } from 'meteor/kadira:flow-router'; | |||
| import { Apis } from '/apis/collection'; | |||
| import { ApiBookmarks } from '/bookmarks/collection'; | |||
|
|
|||
| import { $ } from 'jquery'; | |||
There was a problem hiding this comment.
This import is incorrect and results error later on code.
TypeError: $ is not a function
at Blaze.TemplateInstance.<anonymous> (catalogue.js:127)
at blaze.js?hash=891d5a1…:3341
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=891d5a1…:3687)
at fireCallbacks (blaze.js?hash=891d5a1…:3337)
at Blaze.View.<anonymous> (blaze.js?hash=891d5a1…:3430)
at blaze.js?hash=891d5a1…:1783
at Object.Blaze._withCurrentView (blaze.js?hash=891d5a1…:2214)
at blaze.js?hash=891d5a1…:1782
at Object.Tracker._runFlush (tracker.js?hash=e52e5fe…:539)
at onGlobalMessage (meteor.js?hash=f9ccb2f…:401)
Changing to "import $ from jquery" should fix issue. Or possibly better to use local jquery http://blazejs.org/api/templates.html#Blaze-TemplateInstance-\$
eg.
Template.catalogue.onRendered(function () {
// Activate tooltips on all relevant items
this.$('.toolbar-tooltip').tooltip({ placement: 'bottom' });
});
Here this points to Template instance, you could also save it to variable to make it more clear, "const instance = this", and then use "instance.$"
This is safer, efficient & does not require importing jquery.
So import can be removed if you decide to use local jquery on line 127.
There was a problem hiding this comment.
Hi Ville ,
Thank you for comments and suggestions. Will implement them , though i did not get the above error will look into this error also.Glad that you tested it.
There was a problem hiding this comment.
As @jykae mentioned, change line 9 to import $ from jquery
There was a problem hiding this comment.
@brylie : Done , changed the code . Please review.
|
|
||
| // Get current user bookmarks, only if this API is bookmarked | ||
| const userBookmarks = ApiBookmarks.findOne({ | ||
| userId: Meteor.user()._id, | ||
| apiIds: apiId, | ||
| }); | ||
|
|
||
| console.log("User Bookmarks",userBookmarks); |
There was a problem hiding this comment.
Remove console logs. We try not to leave these logging messages.
|
@snittoor this looks great. One last request is that you remove the |
Closes #1962
Apibookmarks where not showing . This is because instance.subscription('myApibookmarks') was not ready. This instance.subscription is now initialized in cataloge.js (parent) instead of bookmark.js