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): Endpoint to add comment on a clearing request. #2549

Merged
merged 1 commit into from
Oct 22, 2024
Merged

feat(rest): Endpoint to add comment on a clearing request. #2549

merged 1 commit into from
Oct 22, 2024

Conversation

akshitjoshii
Copy link
Contributor

Endpoint to update comment on a clearing request.

Response contains a list of all the comments present on the CR.

image
Closes : #2524

@GMishx GMishx added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for labels Aug 12, 2024
@akshitjoshii akshitjoshii force-pushed the feat/updateCommentsOnClearingRequest-2524 branch 5 times, most recently from 93e57c2 to 0c1210d Compare August 16, 2024 11:41
@akshitjoshii akshitjoshii force-pushed the feat/updateCommentsOnClearingRequest-2524 branch from 0c1210d to ca0f747 Compare August 19, 2024 11:58
@amritkv
Copy link

amritkv commented Aug 27, 2024 •
edited
Loading

Hi @akshitjoshii !

Can we make this response sorted based on update date ?

Need extra fields :

  • User who updated a comment (full name)
  • Comment update date & time

@akshitjoshii akshitjoshii force-pushed the feat/updateCommentsOnClearingRequest-2524 branch from ca0f747 to 469ccd8 Compare August 29, 2024 07:41
@akshitjoshii
Copy link
Contributor Author

akshitjoshii commented Aug 29, 2024 •
edited
Loading

Hi @amritkv
As per above requirement :

  1. Sorted the comments in response based on update date .
  2. Added the extra fields commentedOn and commentingUser in the response. Pls review.

image

@amritkv
Copy link

amritkv commented Sep 2, 2024

Hi @amritkv As per above requirement :

  1. Sorted the comments in response based on update date .
  2. Added the extra fields commentedOn and commentingUser in the response. Pls review.

image

Hi @akshitjoshii !
I have tested this PR. The sorted response is working fine.
But, the name part is not working as I'm not receiving user name. Although, the DB contains the users' full name, given name, and last name.

image

@akshitjoshii
Copy link
Contributor Author

akshitjoshii commented Sep 9, 2024 •
edited
Loading

Hi @amritkv , users in couchdb do not follow the camel casing for full name field. You can change the fullName to fullname in couchdb for you admin user and it will start working.

image

@amritkv
Copy link

amritkv commented Sep 9, 2024

Hi @amritkv , users in couchdb do not follow the camel casing for full name field. You can change the fullName to fullname in couchdb for you admin user and it will start working.

image

Hi @akshitjoshii !
Yes, tested it as per your description and it is working as expected. 👍🏻

@amritkv
Copy link

amritkv commented Sep 13, 2024

Hi @amritkv As per above requirement :

  1. Sorted the comments in response based on update date .
  2. Added the extra fields commentedOn and commentingUser in the response. Pls review.

image

Hi @akshitjoshii !
We need autoGenerated key in the response here, like you added in get comments api.

@akshitjoshii akshitjoshii force-pushed the feat/updateCommentsOnClearingRequest-2524 branch from 469ccd8 to d03ecbd Compare September 13, 2024 07:03
@akshitjoshii
Copy link
Contributor Author

Resolved merge conflicts.

Hi @amritkv, autoGenerated key has been added in the comment response in PR #2603 and is appearing here as well now.

image

@amritkv
Copy link

amritkv commented Sep 17, 2024

Resolved merge conflicts.

Hi @amritkv, autoGenerated key has been added in the comment response in PR #2603 and is appearing here as well now.

image

@akshitjoshii Looks good to me.

@smrutis1 smrutis1 removed the needs general test This is general testing, meaning that there is no org specific issue to check for label Sep 30, 2024
@akshitjoshii akshitjoshii mentioned this pull request Oct 21, 2024
Merged
@akshitjoshii
feat(rest): Endpoint to add comment on a clearing request
2c26f7c 
Signed-off-by: Akshit Joshi <akshit.joshi@siemens-healthineers.com>
@akshitjoshii akshitjoshii force-pushed the feat/updateCommentsOnClearingRequest-2524 branch from d03ecbd to 2c26f7c Compare October 22, 2024 08:05
GMishx
GMishx approved these changes Oct 22, 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.

@GMishx GMishx added the ready ready to merge label Oct 22, 2024
@GMishx GMishx merged commit c167bcc into eclipse-sw360:main Oct 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GMishx GMishx GMishx approved these changes

@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

Assignees
No one assigned
Labels
ready ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Need api to update comments on clearing requests
4 participants
@akshitjoshii @amritkv @GMishx @smrutis1