Skip to content

upcoming: [DI-20284] - Added support for multiple services in the dashboards list#10805

Merged
jaalah-akamai merged 8 commits intolinode:developfrom
nikhagra-akamai:missing-api-params
Aug 23, 2024
Merged

upcoming: [DI-20284] - Added support for multiple services in the dashboards list#10805
jaalah-akamai merged 8 commits intolinode:developfrom
nikhagra-akamai:missing-api-params

Conversation

@nikhagra-akamai
Copy link
Contributor

@nikhagra-akamai nikhagra-akamai commented Aug 20, 2024

Description 📝

Modified dashboard select component to support multiple service types in the list. Also modified api request params based on API spec.

Changes 🔄

  1. Added a api call to fetch list of available service types.
  2. Modified request params and headers based on API spec change.
  3. Added util functions to convert the api response data into appropriate format for autocomplete component.
  4. Fixed the CSS issue in resource select component as highlighted by @coliu-akamai (Added screenshots)

Target release date 🗓️

30 August 2024

Preview 📷

Include a screenshot or screen recording of the change

💡 Use <video src="" /> tag when including recordings in table.

Before After
Screenshot 2024-08-20 at 5 07 21 PM Screenshot 2024-08-20 at 5 07 51 PM
Screenshot 2024-08-20 at 7 46 29 PM Screenshot 2024-08-20 at 7 47 39 PM
Screenshot 2024-08-21 at 6 41 07 PM Screenshot 2024-08-21 at 6 41 13 PM

How to test 🧪

  1. Enable mock services as some endpoints are not available.
  2. No much visual change in this PR as only new api call is added
  3. Few error messages are added that can be generated by modifying the code.

Note: Some re-rendering is there and its fix is going on but this PR is raised to add some API changes urgently.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@nikhagra-akamai nikhagra-akamai requested a review from a team as a code owner August 20, 2024 14:21
@nikhagra-akamai nikhagra-akamai requested review from AzureLatte and harsh-akamai and removed request for a team August 20, 2024 14:21
@nikhagra-akamai nikhagra-akamai requested a review from a team as a code owner August 21, 2024 05:50
@nikhagra-akamai nikhagra-akamai requested review from jdamore-linode and removed request for a team August 21, 2024 05:50
Copy link
Contributor

@venkymano-akamai venkymano-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jcallahan-akamai jcallahan-akamai requested review from bnussman-akamai and removed request for AzureLatte and harsh-akamai August 21, 2024 13:12
@nikhagra-akamai
Copy link
Contributor Author

Thanks @bnussman-akamai for approval. @jdamore-linode please review and provide other approval if all good

@github-actions
Copy link

github-actions bot commented Aug 21, 2024

Coverage Report:
Base Coverage: 82.63%
Current Coverage: 82.63%

Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 🔥


// Returns the list of all the dashboards available
export const getDashboards = () =>
export const getDashboards = (serviceType: string) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a paginated response, should we add setParams(params) and also setFilters(filters)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for our beta release pagination is not required. Later we will update it because some other endpoints are also paginated

}

export interface ServiceTypesList {
data: ServiceTypes[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Something to consider, is there a more descriptive name we can use rather than data as to avoid data.data in places?

Copy link
Contributor Author

@nikhagra-akamai nikhagra-akamai Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are receiving the "data" as response key from API itself because it is a paginated response

"data": [ { "service_type": "linode" } ],

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Aug 22, 2024
@jaalah-akamai jaalah-akamai merged commit 15e3506 into linode:develop Aug 23, 2024
@nikhagra-akamai nikhagra-akamai deleted the missing-api-params branch August 23, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Cloud Pulse

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants