ES6: Migrated "announce" #899
Conversation
| constructor(announce_service) { | ||
| this.logger = new z.util.Logger('z.announce.AnnounceRepository', z.config.LOGGER.OPTIONS); | ||
| this.announce_service = announce_service; | ||
| return this; |
There was a problem hiding this comment.
think we dont need to return this or is it chained?
There was a problem hiding this comment.
It is needed in TestAPI I believe
There was a problem hiding this comment.
I removed the this. It was there because otherwise CoffeeScript would return the last statement from the constructor (instead the instance of the class).
| get_announcements() { | ||
| return new Promise((resolve, reject) => { | ||
| $.get(this.url).done((data) => { | ||
| resolve(data['result']); |
There was a problem hiding this comment.
do we want to use fetch for this???
return fetch(this.url).then((response) => response.json()).then((json) => json.result)
There was a problem hiding this comment.
One thing at a time :D
There was a problem hiding this comment.
means touching it twice
There was a problem hiding this comment.
Tests fail if I use return fetch(this.url).then((data) => data.result).
There was a problem hiding this comment.
If there "easy" fixes we should do them in one go.. but of course if the tests fail and stuff then maybe have somewhere a list that we can track of the things that we spotted..
There was a problem hiding this comment.
It's a good time to have a complete review of the codebase!
There was a problem hiding this comment.
Ok, I will see what I can do here...
There was a problem hiding this comment.
window.fetch returns a ReadableStream in data.body. So the code needs some adjustments.
There was a problem hiding this comment.
Looks like it needs a reader: var reader = response.body.getReader(); (https://jakearchibald.com/2015/thats-so-fetch/)
Maybe we should use a wrapper which wraps the fetch API. Like: https://github.com/typicode/fetchival
There was a problem hiding this comment.
This seems to do the trick:
get_announcements() {
return window.fetch(this.url).then((response) => response.json()).then((data) => data.result);
}
get_version() {
return window.fetch('version/').then((response) => response.json()).then((data) => data.version);
}
| done() | ||
| .catch done.fail | ||
|
|
||
|
|
There was a problem hiding this comment.
tsk tsk.. double empty line :)
Type of change
Screenshot