Repository navigation
fix(admin): preserve auth path application names - #7080
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix matches the reported behavior and includes effective regression coverage.
Pull request overview
Fixes #6539 by preserving each authorization path’s application name during updates.
Changes:
- Rebuilds paths using each DTO’s
appName. - Adds regression coverage for paths with different application names.
File summaries
| File | Description |
|---|---|
AppAuthServiceImpl.java |
Removes stale app-name lookup and preserves per-path values. |
AppAuthServiceTest.java |
Verifies distinct app names are persisted. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Aias00
left a comment
There was a problem hiding this comment.
Approved as PMC (Aias00). Small, well-scoped fix with regression tests; green CI, mergeable. Reviewed the diff.
Aias00
left a comment
There was a problem hiding this comment.
Re-reviewed: preserving auth path application names instead of dropping them is a small, correct fix with a test. Approving.
|
Ping on state: this PR is approved on the code, but the required checks are red and it cannot be merged yet. The failing jobs are No code change is expected from you. Once the shared e2e problem is resolved, please rerun the failed jobs (or push a trivial rebase) so the required checks go green, and this can be merged. |
|
@dengliming This PR is approved, but 5 checks are red: I inspected these lanes today and they fail for infrastructure reasons rather than on the diff — the Please rebase onto |
|
@dengliming — a correction to the note I left here yesterday. The failures on this branch all predate the fix, so nothing in your change is implicated.
That means the red checks recorded here are stale. A push — an empty commit or a rebase, whichever is easier — re-runs them against the current baseline. I would not spend time reading the old logs. For the |
|
Hi @dengliming, this PR is already approved, but it currently cannot be merged. Here is the blocker and what is needed: Blocker What we need Once the branch is up to date and CI is green, we will squash-merge it. Thanks! |
Summary
Testing
JAVA_TOOL_OPTIONS=-javaagent:<byte-buddy-agent> mvn -pl shenyu-admin -am -DskipTests=false -Dcheckstyle.skip=false -Dtest=AppAuthServiceTest -DfailIfNoTests=false testFixes #6539