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.

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

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

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

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

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

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

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.

Signed-off-by: Akshit Joshi <akshit.joshi@siemens-healthineers.com>
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review needs general test This is general testing, meaning that there is no org specific issue to check for
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need api to update comments on clearing requests
3 participants