feat: add sorting for alerts #1097
Conversation
…ite-sdk-js into vebjorn/add-sort-alerts
| @@ -50,10 +52,14 @@ export class AlertsAPI extends BaseResourceAPI<Alert> { | |||
| return this.createEndpoint(items); | |||
| }; | |||
|
|
|||
| public list = async (filter?: AlertFilterQuery) => { | |||
| return this.listEndpoint<AlertFilterQuery>( | |||
| public list = async (filter?: AlertFilterQuery, sort?: AlertSortQuery) => { | |||
There was a problem hiding this comment.
why not just use a single query property for this same as others?
export interface AlertFilterQuery extends FilterQuery {
filter?: AlertFilter;
sort?: WhateverSortTypeHere
}
There was a problem hiding this comment.
That's what i tried initially, but that didn't work for me because listEndpoint() expected two parameters, and not three, so i combined sort and filter so that i avoided changing listEndpoint()
|
@VebjornG can you disable failing subscriber tests? We will fix them in another PR :) |
| public sortAlerts = async (sort: AlertSortQuery) => { | ||
| return this.list(undefined, sort); | ||
| }; |
There was a problem hiding this comment.
what is the meaning of this one? sort without the filter, or?
There was a problem hiding this comment.
Is such a function unnecessary if sorting is to be added? @polomani
If so I'm happy to remove it. I tried to follow the structure of the file.
There was a problem hiding this comment.
sort and list are normally the same endpoint, hence it should be a single method as well.
I pushed a change
…ite-sdk-js into vebjorn/add-sort-alerts
…ite-sdk-js into vebjorn/add-sort-alerts
| }, | ||
| ]); | ||
| } catch (error) { | ||
| expect((error as AxiosError).response?.status).toBe(400); |
There was a problem hiding this comment.
the SDK never throws axios error directly. you should use CogniteError
Make sorting alerts available in the frontend. We are going to fix the tests for creating a subscriber and subscription, as well as listing them, in a different PR.
Ticket