api handles chart access authorization only, the rest is done by plugins #155
Conversation
src/routes/charts.js
Outdated
| } | ||
| } | ||
| // user is authorized to access chart | ||
| // further authoritzation is handled by plugins | ||
|
|
||
| Object.assign(payload, params); | ||
| try { | ||
| const result = await events.emit( |
There was a problem hiding this comment.
After looking at the changes in export-{pdf|zip} we need to change the way results are handled here.
Before the changes, the mentioned plugins threw errors when they can't handle a format. That caused results to look something like this:
const results = [
{ status; 'error', error: 'cant handle format' }, // export-zip
{ status: 'success', data: ... }, // export-pdf
]Then the successful event was used further.
Now plugins just return undefined and the result looks like this:
const results = [
{ status: 'success', data: undefined }, // export-zip
{ status; 'success', data: ... } // export-pdf
]Again the first successful event is used. Plugin results appear in the order they are configured and called. With format pdf this would be a problem in my example.
By throwing in the plugins there was always only 1 successful result, the result of the plugin that implemented the corresponding format.
Now without throwing we need to filter based on the returned data.
I implemented it that way because it's common in Hapi to throw Errors in certain conditions and I'm aware that this might be controversial.
There was a problem hiding this comment.
A further improvement could be that the ApiEventEmitter automatically filters results with data: undefined.
There was a problem hiding this comment.
I see. I changed the filter to success and use .find() to look for a result with data in 8b2d63b
There was a problem hiding this comment.
Cool. I think that was the only thing left. Going to approve the PRs.
will open PRs for
export-pdfandexport-zipas well...