Skip to content

Comments

Updated the logic to return 404 instead of 200 when wrong taskRequest Id is sent#2296

Merged
iamitprakash merged 20 commits intoRealDevSquad:developfrom
Akshat187:develop
Dec 29, 2024
Merged

Updated the logic to return 404 instead of 200 when wrong taskRequest Id is sent#2296
iamitprakash merged 20 commits intoRealDevSquad:developfrom
Akshat187:develop

Conversation

@Akshat187
Copy link
Contributor

@Akshat187 Akshat187 commented Dec 14, 2024 •
edited by Achintya-Chatterjee
Loading

Date: 14-12-24

Developer Name: akshat

Issue Ticket Number

Description

When queried with taskRequestId which doesn't exist, fetchTaskRequestById was returning Tasks returned successfully instead of Task request not found, have changed the code to return the proper error message, currently there are no tests for this case, so it didn't get highlighted.

Documentation Updated?

  • Yes
  • No

Under Feature Flag

  • Yes
  • No

Database Changes

  • Yes
  • No

Breaking Changes

  • Yes
  • No

Development Tested?

  • Yes
  • No

Screenshots

Current implementation returning 200 OK, even when the id is incorrect image Updated implementation to return `Task request not found` when wrong id is passed image

Test Coverage

Screenshot 1

Changed the response returning Task Request not found when wrong taskRequestId is passed · Real-Dev-Squad_website-backend@6cef89d - Google Chrome 20-12-2024 21_25_07

unit test coverage
{E8259E77-9D93-48A3-B464-9B4841C5B693}

Additional Notes

pankajjs
pankajjs suggested changes Dec 14, 2024
View reviewed changes
Copy link
Contributor

@pankajjs pankajjs left a comment

Choose a reason for hiding this comment

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

Add screenshot

@AnujChhikara
Copy link
Contributor

AnujChhikara commented Dec 15, 2024

Can you please check if we have tets written for this, if yes then how are they passing before

@AnujChhikara
Copy link
Contributor

AnujChhikara commented Dec 15, 2024

Also can we please improve the title and PR description

@Akshat187 Akshat187 changed the title corrected the response of task request for given task request id Changed the response returning Task Request not found when wrong taskRequestId is passed. Dec 16, 2024
@prakashchoudhary07
Copy link
Contributor

prakashchoudhary07 commented Dec 17, 2024

Also can we please improve the title and PR description

+1

AnujChhikara

This comment was marked as resolved.

Sign in to view

@vikasosmium
Copy link
Contributor

vikasosmium commented Dec 20, 2024

Update the API Contract and link it here for the error codes and other changes.

@vikasosmium
Copy link
Contributor

vikasosmium commented Dec 20, 2024

add tests SS

pankajjs
pankajjs reviewed Dec 20, 2024
View reviewed changes
pankajjs
pankajjs reviewed Dec 20, 2024
View reviewed changes
@pankajjs
Copy link
Contributor

pankajjs commented Dec 20, 2024

Add test coverage

shubhdevelop
shubhdevelop reviewed Dec 20, 2024
View reviewed changes
Copy link
Contributor

@shubhdevelop shubhdevelop left a comment

Choose a reason for hiding this comment

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

catch block error message in the findTaskRequestById is not correct, can you please update it as well

@Akshat187
Copy link
Contributor Author

Akshat187 commented Dec 22, 2024

catch block error message in the findTaskRequestById is not correct, can you please update it as well

updated the error message

@Akshat187
Copy link
Contributor Author

Akshat187 commented Dec 22, 2024

Add test coverage

I have kept test coverage screenshot in the Description

@Akshat187
Copy link
Contributor Author

Akshat187 commented Dec 22, 2024

RealDevSquad/website-api-contracts#207 API contract changes

@vikhyat187 vikhyat187 changed the title Changed the response returning Task Request not found when wrong taskRequestId is passed. Updated the logic to return 404 instead of 200 when wrong taskRequest Id is sent Dec 22, 2024
pankajjs
pankajjs approved these changes Dec 22, 2024
View reviewed changes
Copy link
Contributor

@pankajjs pankajjs left a comment

Choose a reason for hiding this comment

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

LGTM

shubhdevelop
shubhdevelop reviewed Dec 22, 2024
View reviewed changes
Copy link
Contributor

@shubhdevelop shubhdevelop left a comment

Choose a reason for hiding this comment

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

please add unit test coverage for the fetchTaskRequestById()

@Akshat187
Copy link
Contributor Author

Akshat187 commented Dec 28, 2024

Can you please attach test coverage and changes working fine screenshot

I have added the unit test screenshot

shubhdevelop
shubhdevelop reviewed Dec 28, 2024
View reviewed changes
@vikasosmium vikasosmium self-requested a review December 29, 2024 05:55
@iamitprakash iamitprakash merged commit 8101a05 into RealDevSquad:develop Dec 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AnujChhikara AnujChhikara AnujChhikara approved these changes

+4 more reviewers

@shubhdevelop shubhdevelop shubhdevelop left review comments

@Achintya-Chatterjee Achintya-Chatterjee Achintya-Chatterjee approved these changes

@vikasosmium vikasosmium vikasosmium approved these changes

@pankajjs pankajjs pankajjs approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Akshat187 Akshat187

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

API Returns 200 OK Status for Non-Existent Task Instead of 404 Not Found

9 participants

@Akshat187 @AnujChhikara @prakashchoudhary07 @vikasosmium @pankajjs @shubhdevelop @Achintya-Chatterjee @iamitprakash @vikhyat187