-
Notifications
You must be signed in to change notification settings - Fork 38
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
[MS] Adds API and mocks for custom order requests list #9261
Conversation
Max-7
commented
Dec 24, 2024
- Keep changes in the pull request as small as possible
- Ensure the commit history is sanitized
- Give a meaningful title to your PR
- Describe your changes
return { | ||
status: 200, | ||
isError: false, | ||
requests: [ |
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 think you missed the type
:
data: {
type: DataType.GetCustomOrderRequests,
requests: [
{
...`
users: number; | ||
storage: number; |
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 type coming from Parsec Sign is a string
.
i.e.: From 100 Go to 300 Go
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.
No it's not, https://github.com/Scille/parsec-sign/blob/be56bec136eabc482693ce407562a529ea963121/src/services/bmsApi.ts#L116
And it shouldn't be. You're not displaying the string as it is (you're not doing {{ data.storage }}
, you're also matching the value to the right translation. It becomes very hard to do and to maintain properly if you're trying to match From 100 Go to 300 Go
to display it correctly in French for example.
You're defining the value that is sent yourself in
const storageOptions: MsOptions = new MsOptions([ |
key
, a number, and what you display to the user is the label.
status: CustomOrderRequestStatus; | ||
comment: string; | ||
orderDate: DateTime; | ||
}>; |
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's an array in your mocks:
}[];
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 is defined as an Array. requests: Array<{...}>
. I used the Array<{...}>
syntax instead of {...}[]
.
c2ef71a
to
5f7ebbc
Compare