fix(GKO-2863): return 400 when primary owner is a group member#17292
fix(GKO-2863): return 400 when primary owner is a group member#17292benoitgravitee wants to merge 1 commit into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
benoitgravitee
force-pushed
the
fix/GKO-2863-group-po-member-4xx
branch
2 times, most recently
from
fa34ac9 to
700fd65
Compare
|
ImportGroupCRDUseCase threw TechnicalDomainException on validation failures, which the Automation API mapped to HTTP 500. Align with other CRD import use cases and surface ValidationDomainException instead. # Conflicts: # gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/test/java/io/gravitee/apim/core/group/use_case/ImportGroupCRDUseCaseTest.java
benoitgravitee
force-pushed
the
fix/GKO-2863-group-po-member-4xx
branch
from
700fd65 to
fef9289
Compare
| var sanitizedInput = validationResult.value().orElseThrow(() -> new TechnicalDomainException("Unable to sanitize CRD spec")); | ||
| var sanitizedInput = validationResult.value().orElseThrow(() -> new ValidationDomainException("Unable to sanitize CRD spec")); | ||
|
|
||
| var status = queryService |
There was a problem hiding this comment.
The update branch here (findById(...).map(existing -> update(...))) — the primary success path when the group already exists — has no test coverage. Both new tests exercise the validation/rejection paths only. A regression that broke group updates (e.g. creating a duplicate instead of updating the existing group) wouldn't be caught by CI.
This is a pre-existing gap rather than something this PR introduced, so feel free to treat it as out of scope — flagging it since the test class is already being touched here.
| import static org.assertj.core.api.SoftAssertions.*; | ||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.when; |
There was a problem hiding this comment.
Nit: import static org.mockito.Mockito.when; is added here but the same import already exists on the next line, so this one is redundant. It compiles, but most spotless/checkstyle no-duplicate-import rules flag it.
2 participants
Issue
https://gravitee.atlassian.net/browse/GKO-2863
Summary
ImportGroupCRDUseCasenow throwsValidationDomainExceptionon severe validation errors (same pattern as other CRD import use cases), so the Automation API returns HTTP 400 instead of 500 when the org primary owner is listed as a group member.memory/adminmember case from the ticket.Test plan
mvn -f gravitee-apim-rest-api/gravitee-apim-rest-api-service/pom.xml test -Dtest=ImportGroupCRDUseCaseTestPUT /automation/organizations/DEFAULT/environments/DEFAULT/groupswith primary owner inmembersreturns 400 and a message containingcan not change the role of primary owner