-
Notifications
You must be signed in to change notification settings - Fork 38
OU-1004: List perses dashbaords #666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: customizable-dashboards
Are you sure you want to change the base?
OU-1004: List perses dashbaords #666
Conversation
|
@zhuje: This pull request references OU-1004 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
36881db to
be8bd6c
Compare
27f7498 to
2648502
Compare
02614bc to
fe7041f
Compare
6c5da8d to
fa6da57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we have 2 files called datasource-api.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed one of the files to differentiate them. /web/src/components/dashboards/perses/perses/datasource-cache-api.ts
| name: entity.metadata.name, | ||
| }); | ||
|
|
||
| const response = await consoleFetch(url, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The console fetch has a put method, so we don't have to set the method manually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use consoleFetchJSON.put(...).
| mutationFn: (dashboard) => { | ||
| return updateDashboard(dashboard); | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| mutationFn: (dashboard) => { | |
| return updateDashboard(dashboard); | |
| }, | |
| mutationFn: updateDashboard, |
| const HTTPMethodPUT = 'PUT'; | ||
| const HTTPHeader: Record<string, string> = { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should use the consoleFetchJSON, please explore the SDK types this will allow you to see what is available and how to use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use consoleFetchJSON.put(...).
| defaultOptions: { | ||
| queries: { | ||
| refetchOnWindowFocus: false, | ||
| retry: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be retry: false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this can be false instead of 0. I updated it in the latest commit.
| sortBy && direction | ||
| ? [...data].sort((a, b) => { | ||
| let aValue: any; | ||
| let bValue: any; | ||
|
|
||
| if (sortBy === 'name') { | ||
| aValue = a.name.label; | ||
| bValue = b.name.label; | ||
| } else if (sortBy === 'created') { | ||
| aValue = a.createdAt; | ||
| bValue = b.createdAt; | ||
| } else if (sortBy === 'modified') { | ||
| aValue = a.updatedAt; | ||
| bValue = b.updatedAt; | ||
| } else { | ||
| aValue = a[sortBy]; | ||
| bValue = b[sortBy]; | ||
| } | ||
|
|
||
| if (direction === 'asc') { | ||
| return aValue < bValue ? -1 : aValue > bValue ? 1 : 0; | ||
| } else { | ||
| return aValue > bValue ? -1 : aValue < bValue ? 1 : 0; | ||
| } | ||
| }) | ||
| : data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function will be more readable and maintainable with an early exit:
if(!sortBy || !direction) return data;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this makes it much more readable. Updated in the latest commit.
610f670 to
9a9d158
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgbernalp, zhuje The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |

JIRA
https://issues.redhat.com/browse/OU-1004
ou1004-list-perses-dashboards.mov