Skip to content

Catalogue not displayed on nightly#1217

Merged
55 merged 6 commits intodevelopfrom
bugfix/catalog-not-displayed
Jun 29, 2016
Merged

Catalogue not displayed on nightly#1217
55 merged 6 commits intodevelopfrom
bugfix/catalog-not-displayed

Conversation

@jykae
Copy link
Copy Markdown
Contributor

@jykae jykae commented Jun 27, 2016

Closes #1214

Proposed changes

  • check bookmarks result, user might not have bookmarks, which results undefined.
  • add reactivevar import
  • paranoid check user query, in case of undefined

@jykae jykae added the WIP label Jun 27, 2016
@jykae jykae added this to the Sprint 25 milestone Jun 27, 2016
@jykae
Copy link
Copy Markdown
Contributor Author

jykae commented Jun 27, 2016

Tried deploying this branch for temporary server to test in production one exception occurring on nightly but not in development.
However I could not catch this error on development and not in test deployment,

TypeError: Cannot read property 'splice' of null
    at https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:107:29128
    at Object.o.nonreactive (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:52:4311)
    at Object.removedAt (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:107:28856)
    at Object.removed (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:103:1831)
    at https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:70:1323
    at Array.forEach (native)
    at Function.A.each.A.forEach (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:1:865)
    at r.diffQueryOrderedChanges (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:70:1281)
    at l (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:103:1417)
    at https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:103:802

Catalogue and each tab opens ok for me in development and test production deployment with these changes. Could someone of @apinf/developers do review?

@jykae jykae changed the title Check bookmarks result, if user has no bookmarks Catalogue not displayed on nightly Jun 27, 2016
@55
Copy link
Copy Markdown
Contributor

55 commented Jun 28, 2016

Getting on nightly:

2a2fb88….js?meteor_js_resource=true:3 Exception from Tracker recompute function:
2a2fb88….js?meteor_js_resource=true:3 TypeError: Cannot read property 'username' of undefined
    at https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:428:4436
    at Array.map (native)
    at Function.A.map.A.collect (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:1:1117)
    at t.e [as getApiManagersByName] (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:428:4386)
    at https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:126:2518
    at s.call (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:126:1929)
    at s.mustacheImpl (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:126:1184)
    at Object.s.mustache (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:126:1243)
    at ._render (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:440:29692)
    at l (https://nightly.apinf.io/2a2fb884adc53dffc6e76ae838b2444f03baafbd.js?meteor_js_resource=true:107:20600)

@55
Copy link
Copy Markdown
Contributor

55 commented Jun 28, 2016

@jykae, if it's working on test deployment, let's merge it and do a manual deployment to nightly, since the catalogue is a big thing.

@jykae
Copy link
Copy Markdown
Contributor Author

jykae commented Jun 28, 2016

@NNN Ok, I think "username" is now checked as much as one can. I could manual deploy on nightly if you merge this.

@jykae
Copy link
Copy Markdown
Contributor Author

jykae commented Jun 28, 2016

@55 55 self-assigned this Jun 28, 2016
@jykae
Copy link
Copy Markdown
Contributor Author

jykae commented Jun 28, 2016

As you notice there's one weird edge case, where this "if-statement" does not happen, https://github.com/apinf/api-umbrella-dashboard/blob/bfa7d9475fc0ea0f7bb02cb97401bff5db91059a/apis/collection/helpers.js#L129

@NNN Is it ok to return nothing like it is now, or should we return there also "admin"? What you think?

@55
Copy link
Copy Markdown
Contributor

55 commented Jun 28, 2016

@jykae, sounds right for me. @brylie, what do you think?

}

// Set up query object to contain bookmarked API IDs
query = {_id: {$in: bookmarkedApiIds}};
Copy link
Copy Markdown
Contributor

@brylie brylie Jun 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be moved inside of the if (userBookmarks) { .. } block.

Also, wait until @NNN merges changes in pr #1221, as some of these changes overlap. Specifically, changes to the publications.js are duplicating changes in PR #1221.

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Jun 28, 2016

This PR looks good. Lets hold off until we get PR #1221 merged, and see how that relates to this PR. I.e. the changes to publication.js may not be necessary after the PR #1221 merge.

elnzv added 2 commits June 29, 2016 13:14
…-displayed

# Conflicts:
#	catalogue/client/catalogue.js
#	catalogue/server/publications.js
@55 55 merged commit 517eeb0 into develop Jun 29, 2016
@55 55 deleted the bugfix/catalog-not-displayed branch June 29, 2016 10:36
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.

3 participants