-
Notifications
You must be signed in to change notification settings - Fork 13
Implemented deleteImage API, state and tests #945
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: main
Are you sure you want to change the base?
Conversation
3ab10ff to
4121c7f
Compare
0788148 to
cf49a7c
Compare
cf49a7c to
e8c1f0d
Compare
| const formData = JSON.stringify({ | ||
| authorized_tasks_statuses: [ProgressStatus.NOT_STARTED], | ||
| }); | ||
| const kyOptions = getDefaultOptions(config); | ||
| kyOptions.headers = { | ||
| ...kyOptions.headers, | ||
| 'Content-Type': 'application/json', | ||
| }; | ||
| try { | ||
| const response = await ky.delete(`inspections/${options.id}/images/${options.imageId}`, { | ||
| ...kyOptions, | ||
| body: formData, | ||
| }); |
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 don't think that you need this 'content-type', you can skip using it.
And use json property of ky function passing
{ authorized_tasks_statuses: [ProgressStatus.NOT_STARTED]}
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.
Using the json property instead of body allowed me to get rid of content-type, otherwise the API would respond with 415 Code, saying "Did not attempt to load JSON data because the request Content-Type was not '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.
Do you still have this error?
4af7b00 to
b6d4572
Compare
packages/network/README.md
Outdated
| Delete an image from an inspection. The resulting action of this request will contain the details of the image that has | ||
| been created in the API. |
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.
Change created text to deleted
| await deleteImage(options, apiConfig, dispatch); | ||
|
|
||
| expect(getDefaultOptions).toHaveBeenCalledWith(apiConfig); | ||
| expect(ky.delete).toHaveBeenCalledWith( | ||
| `inspections/${options.id}/images/${options.imageId}`, | ||
| expect.objectContaining(getDefaultOptions(apiConfig)), | ||
| ); | ||
| expect(dispatch).toHaveBeenCalledWith({ | ||
| type: MonkActionType.DELETED_ONE_IMAGE, | ||
| payload: { | ||
| inspectionId: options.id, | ||
| imageId: undefined, |
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.
Mock the response of deleteImage and get the body.
Don't forget to change the expected imageId of dispatch function
d864b1c to
6e0b9b2
Compare
| }, | ||
| }); | ||
| expect(result).toEqual({ | ||
| id: 'delete-test-fake-id', |
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.
Here it shouldn't be a hardcoded string, but body.id
| type: MonkActionType.DELETED_ONE_IMAGE, | ||
| payload: { | ||
| inspectionId: options.id, | ||
| imageId: result.id, |
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.
Actually you want to get the real response of ky.delete, so instead of result.id. It should be body.id
| }); | ||
|
|
||
| it('should display an error if deletion fails', async () => { | ||
| const err = new Error('test delete error'); |
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.
Could you change it to kebab-case convention please
Reset state objects containing the deleted image Updated objects to be deleted in State Updated API export and added more tests Added docs Updated Tests
6e0b9b2 to
aaec99e
Compare
Overview
Jira Ticket Reference : MN-780
Added a new
deleteImagefunction to handle image deletion of an inspection.Checklist before requesting a review