convert fetch api call to rtk mutation#529
convert fetch api call to rtk mutation#529mayurlalwani wants to merge 3 commits intoRealDevSquad:developfrom
Conversation
|
@mayurlalwani is attempting to deploy a commit to the RDS-Team Team on Vercel. A member of the Team first needs to authorize it. |
bhtibrewal
left a comment
bhtibrewal
left a comment
There was a problem hiding this comment.
Please write tests for the functionality, and description for the task.
src/app/services/tasksApi.ts
Outdated
| query: () => '/tasks/mine', | ||
| providesTags: ['Tasks'], | ||
| }), | ||
| updateCardContent: builder.mutation({ |
There was a problem hiding this comment.
What do you think will this file would be the right place to write this API?
There was a problem hiding this comment.
As this component was related to tasks, so I added this API in this file. Should I create a new file?
There was a problem hiding this comment.
Also, the unit test for the funcitonality already exists. Should I write unit test for this helper function - "updateCardContent"?
There was a problem hiding this comment.
the file is okay but the naming doesn't clearly indicate what this function does, read what it is doing here
url: `"${TASKS_URL}/${id}"`,
method: 'PATCH',
There was a problem hiding this comment.
yes, we are writing tests for this RTK hook also "updateCardContent" and the unit tests will also change because of the change in the implementation of the API call.
mayurlalwani
force-pushed
the
512-write-rtk-mutation-for-updatecardcontent-api-call
branch
from
47d7e04 to
e9a18d3
Compare
Pratiyushkumar
left a comment
Pratiyushkumar
left a comment
There was a problem hiding this comment.
Please address the change request mentioned by bhavika and also write the test cases for it
mayurlalwani
force-pushed
the
512-write-rtk-mutation-for-updatecardcontent-api-call
branch
from
e9a18d3 to
0942865
Compare
mayurlalwani
force-pushed
the
512-write-rtk-mutation-for-updatecardcontent-api-call
branch
from
0942865 to
b834d29
Compare
Pratiyushkumar
left a comment
Pratiyushkumar
left a comment
There was a problem hiding this comment.
Everything looks good to me
| expect(setTasks).toHaveBeenLastCalledWith(groupedTasks); | ||
| }); | ||
|
|
||
| test('Should update task details by id', async () => { |
There was a problem hiding this comment.
please make a separate file for this , it should not go in this file
| id: '1', | ||
| cardDetails: { | ||
| id: '123', | ||
| lossRate: { | ||
| dinero: 10, | ||
| neelam: 5, | ||
| }, | ||
| links: [''], | ||
| completionAward: { | ||
| dinero: 110, | ||
| neelam: 10, | ||
| }, | ||
| dependsOn: [], | ||
| assignee: 'john', | ||
| startedOn: '1618790400', | ||
| isNoteworthy: true, | ||
| title: 'Test', | ||
| purpose: '', | ||
| percentCompleted: 0, | ||
| endsOn: '1618790400', | ||
| status: 'progress', | ||
| featureUrl: 'progress', | ||
| type: 'feature', | ||
| createdBy: 'sam', |
There was a problem hiding this comment.
Taking out this data into a variable will make the test easily readable
| async function onContentChangeHandler(id: string, cardDetails: any) { | ||
| if (!isEditable || !updateCardContent) return; | ||
| updateCardContent(id, cardDetails); | ||
| if (!isEditable || !updateTaskDetailsById) return; |
There was a problem hiding this comment.
please check is !updateTaskDetailsById required here
There was a problem hiding this comment.
It is not required, I will remove it.
mayurlalwani
force-pushed
the
512-write-rtk-mutation-for-updatecardcontent-api-call
branch
from
f3cc656 to
8888916
Compare
Pratiyushkumar
left a comment
Pratiyushkumar
left a comment
There was a problem hiding this comment.
Looks good to me, ask Bhavika to approve, I will approve after she approves
mayurlalwani
force-pushed
the
512-write-rtk-mutation-for-updatecardcontent-api-call
branch
from
8888916 to
e581013
Compare
mayurlalwani
force-pushed
the
512-write-rtk-mutation-for-updatecardcontent-api-call
branch
from
e581013 to
1462973
Compare
3 participants
Uh oh!
There was an error while loading. Please reload this page.