Skip to content
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

🐛 Bug Report: Novu SDK layouts response DTO is not correct #3446

Open
2 tasks done
underfisk opened this issue May 17, 2023 · 6 comments
Open
2 tasks done

🐛 Bug Report: Novu SDK layouts response DTO is not correct #3446

underfisk opened this issue May 17, 2023 · 6 comments
Assignees
Labels

Comments

@underfisk
Copy link
Contributor

underfisk commented May 17, 2023

📜 Description

Noticed while using the Novu Nodejs SDK that when attempting to fetch a list of layouts, the response DTO does not match the API response

👟 Reproduction steps

  1. Install Novu nodejs SDK
  2. Retrieve a list of layouts novu.layouts.list({ your params})
  3. Inspect the API response

👍 Expected behavior

The interface should reflect the actual API response

👎 Actual Behavior with Screenshots

const axiosResponse = await novu.layouts.list({ page: 0, pageSize: 100 })
const list = axiosResponse.data

You'll observe that list will have the following content:

{
    page: 0,
    totalCount: 2,
    pageSize: 100,
    data: [ [Object], [Object] ]
}

When inspecting with my IDE, this is what I see:
Screenshot 2023-05-17 at 6 01 27 PM

📃 Provide any additional context for the Bug.

No response

👀 Have you spent some time to check if this bug has been raised before?

  • I checked and didn't find similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

Yes I am willing to submit a PR!

@underfisk
Copy link
Contributor Author

Bumping

@p-fernandez
Copy link
Contributor

@underfisk sorry but could you clarify what you are receiving that doesn't match the interface ILayoutPaginationResponse when using the Node SDK functionality novu.layouts.list?

export interface ILayoutPaginationResponse {

@marvinjude
Copy link
Contributor

marvinjude commented Aug 9, 2023

@p-fernandez @scopsy, Seem to me like @underfisk is explicitly typing fff(possibly inferred by the editor for whatever reason) as ILayoutPaginationResponse in the example above and referencing fff.data where fff is actually an AxiosResponse with data type -> ILayoutPaginationResponse

Whereas, Correct use on the user's end should look like so:

import { AxiosResponse } from "axios";

//@ts-ignore
const fff: AxiosResponse<ILayoutPaginationResponse> = await novu.layouts.list({
  page: 0,
  pageSize: 100,
});

fff.data.data // now you can see the right hint as you type

@p-fernandez Is there any documentation on how to use types like ILayoutPaginationResponse?

export interface ILayoutPaginationResponse {
I can see it's just a public type as it isn't used internally. Or was this written for future internal use?

This seems to be all tied to the fact that most methods don't have explicit return types.

Happy to make a PR

Also see #3928 (comment)

@scopsy
Copy link
Contributor

scopsy commented Aug 13, 2023

@jainpawan21 I think that you have made some adjustments based on that recently, any thoughts on this one?

@jainpawan21
Copy link
Member

@jainpawan21 I think that you have made some adjustments based on that recently, any thoughts on this one?

This should not happen
i will check this

Assigning this to myself

@jainpawan21 jainpawan21 self-assigned this Sep 7, 2023
@peoray
Copy link
Contributor

peoray commented Jan 3, 2024

What's the status on this @jainpawan21 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants