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

fix(Rest): Adding vcs field in component update api #2383

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

Conversation

FarooqAftab
Copy link
Contributor

@FarooqAftab FarooqAftab commented Mar 27, 2024

Please provide a summary of your changes here.

Depends-on: #2408

Issue: #2376

Suggest Reviewer

@ag4ums

How To Test?

Test by passing vcs field in the request body of the patch api to update component using http://localhost:8080/resource/api/components/{id}
Have you implemented any additional tests?

Checklist

Must:

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

@akshitjoshii
Copy link
Contributor

Testing this PR

@akshitjoshii
Copy link
Contributor

Testing done. Currently user can put any and every string in the VCS field as their is no validation being performed. Maybe a check can be added.
image

@afsahsyeda
Copy link
Contributor

@FarooqAftab you can implement a regex check for VCS URL before updating the component either in the service or controller class.

@manik-sh manik-sh force-pushed the fix/vcsFieldUpdateInRestComponent branch from fa1e318 to eac3d45 Compare April 5, 2024 05:19
@FarooqAftab
Copy link
Contributor Author

@FarooqAftab you can implement a regex check for VCS URL before updating the component either in the service or controller class.

@akshitjoshii @afsahsyeda URL Validation has been implemented , request to please test and review it once.

@afsahsyeda afsahsyeda self-assigned this Apr 5, 2024
@afsahsyeda afsahsyeda added needs general test This is general testing, meaning that there is no org specific issue to check for needs code review labels Apr 5, 2024
@rajaraajeshwari
Copy link

Hi @afsahsyeda
Could you please let us know when this PR will be merged.

@afsahsyeda
Copy link
Contributor

@FarooqAftab Please resolve the conflicts.

@GMishx
Copy link
Member

GMishx commented Nov 5, 2024

@afsahsyeda the conflicts have been resolved. Can you please test/review this PR?

@akshitjoshii
Copy link
Contributor

PR still has liferay folder and not rebased with latest main causing build to fail.

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.

@FarooqAftab , please rebase the branch for testing and reviewing.

@afsahsyeda afsahsyeda added the has merge conflicts The PR has merge conflicts label Nov 18, 2024
@afsahsyeda
Copy link
Contributor

@FarooqAftab Kindly resolve the merge conflicts

@FarooqAftab
Copy link
Contributor Author

@FarooqAftab Kindly resolve the merge conflicts

sure , will resolve it.

@GMishx GMishx added this to the Release - 20.0.0 milestone Nov 27, 2024
@@ -405,6 +405,7 @@ struct ComponentDTO {
51: optional string mailinglist,
52: optional string wiki,
53: optional string blog,
56: optional string vcs,
Copy link
Member

Choose a reason for hiding this comment

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

Please add the appropriate json ignore for this new field (like setVcs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has merge conflicts The PR has merge conflicts 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.

Can't able to update vcs field in the component via rest api of the component.
6 participants