[SHARE-938][Improvement] Only display sources with data on sources page#193
Conversation
aaxelb
left a comment
aaxelb
left a comment
There was a problem hiding this comment.
Looks pretty good 📽
What happens if elastic is down? Does this page show an error, or does it hang?
| getQueryBody() { | ||
| let query = { | ||
| query_string: { | ||
| query: this.get('q') || '*' |
There was a problem hiding this comment.
Is there ever going to be a q on this page? You can just leave out the query part of the request, it'll match everything by default.
There was a problem hiding this comment.
Oops missed that 👍
| }).then((json) => { | ||
| this.set('numberOfSources', json.meta.pagination.count); | ||
| this.get('sources').addObjects(json.data); | ||
| let tmpSources = json.data.filter(source => sourcesWithData.includes(source.attributes.longTitle)); |
There was a problem hiding this comment.
This would be nicer if sourcesWithData were an object with titles as property names, so you could treat it like a set (longTitle in sourcesWithData) instead of iterating through the array over and over.
| this.loadElasticAggregations(); | ||
| }, | ||
|
|
||
| searchUrl: Ember.computed(function() { |
There was a problem hiding this comment.
I know this is from the discover page, but there's no reason for this to be a computed property. loadElasticAggregations should just call buildElasticCall (which probably doesn't even deserve to be a util... but there's no point trying to fix everything right now)
|
It will hang...should probs handle that 🐒 |
Use object instead of array for faster checking
3 participants
Purpose
The number of sources on the sources page differ from the number on the discover page.
Changes
Filter API results based on an elastic aggregation.
Side effects
Only sources with data will display on the sources page. On staging and locally there might be only a few sources listed.
Ticket
https://openscience.atlassian.net/browse/SHARE-938