-
Notifications
You must be signed in to change notification settings - Fork 20
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
Adds Get Endorsement End Point #40
Conversation
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/ApiResponse.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementServiceImpl.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/JsonApiResponseConverter.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementDTO.java
Show resolved
Hide resolved
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.
left few comments and didn't understand why the endorserslist
has a different response than the EndorsementList
model
I didn't get the this part : |
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/ApiResponse.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementServiceImpl.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/JsonApiResponseConverter.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/EndorsementList/EndorsementListModel.java
Show resolved
Hide resolved
skill-tree /src/test/java/com/RDS/skilltree/Endorsement/EndorsementServiceTest.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementModel.java
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementServiceImpl.java
Outdated
Show resolved
Hide resolved
…al-Dev-Squad/skill-tree-backend into feat/get-endorsement-endpoint
Hi @heyrandhir whatever dependency is there on the Auth PR, can you include those files in your PR? |
In your PR we are sending the entire Model object in the response, which shouldn't be done, we can just have the user Id or user Name, @bhtibrewal @prakashchoudhary07 @Ajeyakrishna-k what are your thoughts on this? |
Its this entire PR #22 @vikhyat187. Without this PR, the route would encounter authentication errors. Additionally, I don't believe it's good idea to include those files in this PR, as it could be overwhelming for the reviewers to assess numerous changes within this context just for a GET route. |
sure lets see what others have to say, will modify accordingly |
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
skill-tree /src/test/java/com/RDS/skilltree/Endorsement/EndorsementServiceTest.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementDTO.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementDTO.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
What does the API contract say? |
Its the user Id as per the contracts I've written earlier @prakashchoudhary07 cc : @heyrandhir |
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementDTO.java
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementController.java
Outdated
Show resolved
Hide resolved
skill-tree /src/main/java/com/RDS/skilltree/Endorsement/EndorsementResponse.java
Outdated
Show resolved
Hide resolved
…lass by excluding null fields
Date: 1 Dec 2023
Developer Name: @heyrandhir
Description:
This PR has the code changes for the APIs on the endpoint
/v1/endorsements
Is Under Feature Flag
Database changes
Breaking changes (If your feature is breaking/missing something please mention pending tickets)
Is Development Tested?
Testing Stats :
Proof on local :
Since the complete response was not captured hence posting the complete response below :
GET Request to
v1/endorsements/a9e48c1f-19b8-4c66-9ba2-5d8b7c80b2a6