-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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: Provide a new open APl to return the organization list(#5349) #5365
Conversation
WalkthroughThis pull request introduces new API functionality for managing organization data. A service class and a REST controller are added to retrieve organization information. Utility methods are also provided to transform organization entities into DTOs, and redundant import statements are removed from the utility class. Additionally, the dependency version for Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant OC as OrganizationController
participant SO as ServerOrganizationOpenApiService
participant PC as PortalConfig
participant U as OpenApiBeanUtils
C->>OC: GET /openapi/v1/organizations
OC->>SO: getOrganizations()
SO->>PC: organizations()
PC-->>SO: List<Organization>
SO->>U: transformFromOrganizations(List<Organization>)
U-->>SO: List<OpenOrganizationDto>
SO-->>OC: List<OpenOrganizationDto>
OC-->>C: List<OpenOrganizationDto>
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationController.java (1)
35-38
: Method can be improved for RESTful clarity.The method name is singular (
getOrganization
) but returns a list of organizations. Consider these improvements:-@RequestMapping("/organizations") -public List<OpenOrganizationDto> getOrganization() { +@GetMapping("/organizations") +public List<OpenOrganizationDto> getOrganizations() { return organizationOpenApiService.getOrganizations(); }This change:
- Uses
@GetMapping
to explicitly specify HTTP GET method- Renames the method to plural form to match the return type
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerOrganizationOpenApiService.java
(1 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java
(2 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationController.java
(1 hunks)pom.xml
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerOrganizationOpenApiService.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java (1)
OpenApiBeanUtils
(54-204)
🔇 Additional comments (5)
pom.xml (1)
66-66
:✅ Verification successful
Version dependency updated to match new features.
The apollo-java.version has been updated to 2.5.0-SNAPSHOT to align with the new organization API functionality being introduced. Using a SNAPSHOT version implies this is development code that may change.
🏁 Script executed:
#!/bin/bash # Check if this version is consistent with the project version grep -n "<revision>" pom.xmlLength of output: 71
Consistent Version Update in pom.xml
The
<apollo-java.version>
has been correctly updated to2.5.0-SNAPSHOT
in line 66, which matches the project<revision>
at line 63. This confirms that the dependency update aligns with the new organization API functionality requirements. No further changes are required.apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerOrganizationOpenApiService.java (1)
25-37
: Implementation looks good.The service correctly implements the
OrganizationOpenApiService
interface, uses constructor injection for dependencies, and properly delegates to the utility method for transforming organizations to DTOs.apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationController.java (1)
25-32
: Controller implementation follows best practices.The controller uses constructor injection and provides a descriptive bean name to avoid conflicts.
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java (2)
27-37
: Improved import organization.The imports have been consolidated by removing redundant individual imports for DTO classes.
194-203
: Well-implemented utility methods.The new transformation methods:
- Follow the same pattern as existing transformation methods in the class
- Properly handle null/empty collections
- Use precondition checks to validate inputs
- Leverage Java 8 streams for efficient transformation
The implementation is consistent with the coding style of the rest of the class.
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also update the CHANGES.md.
...java/com/ctrip/framework/apollo/openapi/server/service/ServerOrganizationOpenApiService.java
Show resolved
Hide resolved
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java
Show resolved
Hide resolved
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java
Show resolved
Hide resolved
...l/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationController.java
Outdated
Show resolved
Hide resolved
…/server/service/ServerOrganizationOpenApiService.java Co-authored-by: Jason Song <nobodyiam@gmail.com>
…/util/OpenApiBeanUtils.java Co-authored-by: Jason Song <nobodyiam@gmail.com>
…/v1/controller/OrganizationController.java Co-authored-by: Jason Song <nobodyiam@gmail.com>
…/util/OpenApiBeanUtils.java Co-authored-by: Jason Song <nobodyiam@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGES.md (1)
10-10
: Fix Typographical Error in Feature DescriptionThe new feature entry on line 10 incorrectly spells "API" as "APl". Please update the entry to correct this typographical error for clarity and consistency with the intended messaging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGES.md
(1 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerOrganizationOpenApiService.java
(1 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java
(2 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationController.java
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/server/service/ServerOrganizationOpenApiService.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/OrganizationController.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiBeanUtils.java
Thank you for your review and suggestions. I have updated the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What's the purpose of this PR
feat: Provide a new open APl to return the organization list(#5349)
Which issue(s) this PR fixes:
#5349
Brief changelog
After updating the project, we can use this method like this way:
Summary by CodeRabbit
New Features
Chores