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

fix(rest): Added a commit message while create a moderation request. #2405

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

fix(rest): Added a commit message while create a moderation request. #2405

wants to merge 1 commit into from
+219 −98

Conversation

nikkuma7
Copy link
Contributor

Please provide a summary of your changes here.

  • Which issue is this pull request belonging to and how is it solving it? (Refer to issue here)
  • Did you add or update any new dependencies that are required for your change?

Issue: Closes : #2404

Suggest Reviewer

You can suggest reviewers here with an @mention.

How To Test?

How should these changes be tested by the reviewer?
Have you implemented any additional tests?

Checklist

Must:

  • All related issues are referenced in commit messages and in PR

@ag4ums ag4ums added the WIP work in progress label Apr 15, 2024
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch 4 times, most recently from 37f04fa to 798d2a2 Compare April 16, 2024 05:46
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch 2 times, most recently from 92785fb to b7b3c51 Compare May 6, 2024 07:25
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from b7b3c51 to fcd4263 Compare May 13, 2024 10:12
@GMishx
Copy link
Member

GMishx commented Oct 21, 2024

(from discussion) The new front-end should be aware that current action will result in a Moderation Request and should capture comment from user before submitting. This is the current behavior of existing UI with Liferay.

@GMishx GMishx mentioned this pull request Oct 24, 2024
Merged
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from fcd4263 to 655f85c Compare November 14, 2024 08:27
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch 4 times, most recently from d74f7ea to 029d4bd Compare November 25, 2024 09:10
@nikkuma7 nikkuma7 added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for and removed WIP work in progress labels Nov 26, 2024
@GMishx GMishx added this to the Release - 20.0.0 milestone Nov 27, 2024
@GMishx GMishx added the has merge conflicts The PR has merge conflicts label Dec 4, 2024
GMishx
GMishx requested changes Dec 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.

Lot of irrelevant changes make PR hard to review. Please revert them.

...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from 029d4bd to 887b2cf Compare December 6, 2024 08:46
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch 7 times, most recently from 50e9cec to 3c7ca36 Compare December 10, 2024 06:25
GMishx
GMishx requested changes Dec 10, 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.

Response status needs to be changed for bad request.

...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Show resolved Hide resolved
...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
...ce-server/src/main/java/org/eclipse/sw360/rest/resourceserver/project/ProjectController.java Outdated Show resolved Hide resolved
@GMishx GMishx removed the has merge conflicts The PR has merge conflicts label Dec 10, 2024
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from 3c7ca36 to 6a5d415 Compare December 11, 2024 09:32
@nikkuma7
Copy link
Contributor Author

@GMishx , Addressed comment.

GMishx
GMishx approved these changes Dec 11, 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
Copy link
Member

GMishx commented Dec 11, 2024

@nikkuma7 one thing I noticed is for requests where a map object is used in request body, the additional parameter for comment is not used (which is technically not possible).
Can you please add the comment to the documentation of the body to add a comment field?

Something like

@Parameter(description = "Project with `dependencyNetwork` set. Add `comment` field in the body for moderation request.",
           schema = @Schema(implementation = Project.class))
@RequestBody Map<String, Object> reqBodyMap

This will be required for following functions:

  • patchProjectWithNetwork()
  • patchProject()

GMishx
GMishx requested changes Dec 11, 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.

Missed it.

...erver/src/main/java/org/eclipse/sw360/rest/resourceserver/component/ComponentController.java Outdated Show resolved Hide resolved
@GMishx GMishx added the has merge conflicts The PR has merge conflicts label Dec 13, 2024
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from 6a5d415 to 66232d6 Compare December 16, 2024 06:42
GMishx
GMishx previously approved these changes Dec 16, 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.

Needs test.

@GMishx
Copy link
Member

GMishx commented Dec 16, 2024

@nikkuma7 please rebase for testing.

@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from 66232d6 to 3d30bf2 Compare December 16, 2024 08:55
@GMishx GMishx removed the has merge conflicts The PR has merge conflicts label Dec 16, 2024
GMishx
GMishx previously approved these changes Dec 16, 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 has merge conflicts The PR has merge conflicts and removed needs code review labels Dec 16, 2024
@GMishx
Copy link
Member

GMishx commented Jan 6, 2025

@nikkuma7 please rebase for testing.

@nikkuma7
chore(deps): bump org.apache.velocity:velocity-engine-core
3040c25 
Bumps org.apache.velocity:velocity-engine-core from 2.4 to 2.4.1.

---
updated-dependencies:
- dependency-name: org.apache.velocity:velocity-engine-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Nikesh kumar <[email protected]>
@nikkuma7 nikkuma7 force-pushed the feat/commitMessageForuserModerator branch from 3d30bf2 to 3040c25 Compare January 7, 2025 06:27
@nikkuma7 nikkuma7 removed the has merge conflicts The PR has merge conflicts label Jan 7, 2025
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

@nikkuma7 nikkuma7

Labels
needs general test This is general testing, meaning that there is no org specific issue to check for
Projects
None yet
Milestone
Release - 20.0.0
Development

Successfully merging this pull request may close these issues.

feat(rest): User add a commit message while create a moderation request.
4 participants
@nikkuma7 @GMishx @heliocastro @ag4ums