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(openapi): allow user create app via openapi #4954

Merged

Conversation

Anilople
Copy link
Contributor

@Anilople Anilople commented Aug 11, 2023

What's the purpose of this PR

Allow user create app via openapi

Which issue(s) this PR fixes:

#4234

Dependency on apolloconfig/apollo-java#32

Brief changelog

Bind consumer to a role which can create app.

Just a poc code, please don't merge.

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@Anilople
Copy link
Contributor Author

Will add some tests later

@nobodyiam nobodyiam added this to the 2.2.0 milestone Aug 23, 2023
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #4954 (075c7a4) into master (84c1d0b) will increase coverage by 0.48%.
The diff coverage is 57.89%.

@@             Coverage Diff              @@
##             master    #4954      +/-   ##
============================================
+ Coverage     48.48%   48.96%   +0.48%     
- Complexity     1726     1781      +55     
============================================
  Files           346      348       +2     
  Lines         10836    10991     +155     
  Branches       1080     1095      +15     
============================================
+ Hits           5254     5382     +128     
- Misses         5258     5279      +21     
- Partials        324      330       +6     
Files Changed Coverage Δ
...k/apollo/common/exception/BadRequestException.java 85.71% <0.00%> (-14.29%) ⬇️
...ollo/openapi/auth/ConsumerPermissionValidator.java 25.00% <0.00%> (-2.78%) ⬇️
...penapi/server/service/ServerAppOpenApiService.java 22.58% <0.00%> (-12.42%) ⬇️
...rk/apollo/openapi/v1/controller/AppController.java 36.36% <0.00%> (-30.31%) ⬇️
...mework/apollo/portal/controller/AppController.java 29.33% <0.00%> (+2.50%) ⬆️
...mework/apollo/openapi/service/ConsumerService.java 56.84% <50.00%> (-1.62%) ⬇️
...k/apollo/portal/controller/ConsumerController.java 43.85% <76.92%> (+33.33%) ⬆️
...apollo/portal/entity/vo/consumer/ConsumerInfo.java 82.14% <82.14%> (ø)
...ip/framework/apollo/portal/service/AppService.java 39.56% <88.88%> (+23.89%) ⬆️
...al/entity/vo/consumer/ConsumerCreateRequestVO.java 89.47% <89.47%> (ø)
... and 1 more

... and 3 files with indirect coverage changes

…/v1/controller/AppController.java

Co-authored-by: Jason Song <nobodyiam@gmail.com>
@nobodyiam
Copy link
Member

Looks great! One minor suggestion: consider removing the * before the Allow app creation? label since the default setting for this field is false.

image

@nobodyiam
Copy link
Member

BTW, please also help to update the CHANGES.md.

@Anilople
Copy link
Contributor Author

Looks great! One minor suggestion: consider removing the * before the Allow app creation? label since the default setting for this field is false.

image

Done in 20a860f

@Anilople
Copy link
Contributor Author

BTW, please also help to update the CHANGES.md.

Done in 075c7a4

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit a23b3a8 into apolloconfig:master Sep 20, 2023
12 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2023
@nobodyiam
Copy link
Member

@Anilople it appears that the documentation for the new API is not included. Could you kindly assist in incorporating it into the documentation?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants