Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(rest): Add endpoint to handle updation of clearing requests. #2480

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(rest): Add endpoint to handle updation of clearing requests. #2480

wants to merge 1 commit into from

Conversation

sameed20
Copy link
Contributor

@sameed20 sameed20 commented Jun 4, 2024 •
edited by GMishx
Loading

Description

This endpoint is to update a clearing request using id.
cr_endpoint

Closes: #2464
Closes: #2523

@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch from 0a1fad6 to e7bc3fe Compare June 4, 2024 05:25
@GMishx GMishx added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for New-UI Level for the API and UI level changes for the new-ui labels Jul 4, 2024
GMishx
GMishx requested changes Jul 4, 2024
View reviewed changes
Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

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

Minor changes needed.

...in/java/org/eclipse/sw360/rest/resourceserver/clearingrequest/ClearingRequestController.java Outdated Show resolved Hide resolved
...er/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ClearingRequestSpecTest.java Outdated Show resolved Hide resolved
...er/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ClearingRequestSpecTest.java Show resolved Hide resolved
@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch from e7bc3fe to 23dff51 Compare July 4, 2024 12:13
GMishx
GMishx previously approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

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

Changes looks good.

@nikkuma7
Copy link
Contributor

nikkuma7 commented Jul 5, 2024

Test Successful.

image

@sameed20 sameed20 mentioned this pull request Jul 24, 2024
Open
@GMishx GMishx added ready ready to merge and removed needs general test This is general testing, meaning that there is no org specific issue to check for labels Jul 24, 2024
@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch from 23dff51 to 95a7408 Compare July 31, 2024 10:20
@akshitjoshii
Copy link
Contributor

Test successful after rebasing it latest main.
image

@amritkv
Copy link

amritkv commented Aug 13, 2024 •
edited
Loading

Hi @sameed20 & @akshitjoshii !

The update CR is accepting only preferredClearingDate in the body. But as per UI is should accept agreedClearingDate. And moreover on update a CR with preferredClearingDate, the db is not reflecting the changes.
Can you please cross check this issue ?

UI Screen :

image

Postman API Response :

image

DB is not reflecting the changes in preferredClearingDate :

image

@GMishx GMishx added needs general test This is general testing, meaning that there is no org specific issue to check for and removed ready ready to merge labels Aug 16, 2024
@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch 3 times, most recently from df95be9 to 186c255 Compare August 16, 2024 19:22
@amritkv
Copy link

amritkv commented Aug 17, 2024

preferredClearingDate

Hi @sameed20 !
Now It is accepting the agreedClearingDate.
But, still the preferredClearingDate is not reflecting in DB although the api response to update the DB with preferredClearingDate is 200 OK.

@sameed20
Copy link
Contributor Author

preferredClearingDate

Hi @sameed20 ! Now It is accepting the agreedClearingDate. But, still the preferredClearingDate is not reflecting in DB although the api response to update the DB with preferredClearingDate is 200 OK.

Hi @amritkv
As we discussed previously that the value of preferredClearingDate cannot be set less than it's current value and can only be updated by the user who has created that CR.

@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch from 186c255 to f7becfc Compare September 18, 2024 06:05
@GMishx
Copy link
Member

GMishx commented Oct 17, 2024

@amritkv @akshitjoshii is the PR good to merge?

@amritkv
Copy link

amritkv commented Oct 17, 2024

@amritkv @akshitjoshii is the PR good to merge?

@GMishx It needs a general test about what @sameed20 has pointed out.

@akshitjoshii
Copy link
Contributor

Hi @sameed20, Endpoint is allowing adding comments on a CR and on adding the comments via PATCH request, it is removing all the previous comments on Clearing Request and overriding them with the comments passed in the request body.

PR for add comments on a CR already exists #2549

@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch 2 times, most recently from 8d7191d to ff60f41 Compare November 4, 2024 14:57
@sameed20 sameed20 force-pushed the feat/update-clearing-request-endpoint-2464 branch from ff60f41 to e394f7d Compare November 4, 2024 15:24
@akshitjoshii
Copy link
Contributor

@sameed20, the lastUpdatedOn date is not changing when an PATCH operation is done on the CR. Pls set that field as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GMishx GMishx GMishx left review comments

@heliocastro heliocastro Awaiting requested review from heliocastro heliocastro is a code owner

@KoukiHama KoukiHama Awaiting requested review from KoukiHama KoukiHama is a code owner

@arunazhakesan arunazhakesan Awaiting requested review from arunazhakesan arunazhakesan is a code owner

@ag4ums ag4ums Awaiting requested review from ag4ums ag4ums is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@sameed20 sameed20

Labels
needs general test This is general testing, meaning that there is no org specific issue to check for New-UI Level for the API and UI level changes for the new-ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Need Edit Clearing Request rest api Create new endpoint for updating a clearing request
5 participants
@sameed20 @nikkuma7 @akshitjoshii @amritkv @GMishx