Modularize server, client, and tests into feature packages#1095
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Claudia-Anthropica
approved these changes
Jun 18, 2026
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
Collaborator
There was a problem hiding this comment.
@bensofficial This looks like a clean structural reorganization. I checked the package/import rewrites, config/resource strings, moved test paths, and CI status; approving as-is.
Resolves conflicts from develop (passkey #955, hide-expired-topics #1089, dependency updates, react-router v8) against the feature-package modularization. Resolution summary: - Imports: kept the modularized `@/` (client) and feature-package (server) import style; folded in develop's newly-added imports converted to the same style. - New passkey files added by develop landed in the old flat layout and were relocated into the modularized structure with `@/` imports: - components/PasskeyRegistrationPrompt -> core/components/PasskeyRegistrationPrompt - pages/SettingsPage/components/PasskeySettings -> core/admin/pages/SettingsPage/components/PasskeySettings - utils/passkey.ts (git-relocated) -> core/utils/passkey.ts - CollapsibleTopicElement: took develop's canonical #1089 behavior (`canApply = !!fullTopic && state === OPEN`). - BaseKeycloakIntegrationTest: kept in mock/ (consistent with BaseIntegrationTest) with modularized repository imports; TestContainerImages is same-package. Verified: client build/tsc/lint/tests green; server compiles + 864/864 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
krusche
approved these changes
Jun 20, 2026
The feature-package reorg moved e2e specs into subdirectories, which changed
Playwright's file-execution order and exposed a pre-existing cross-test state
dependency:
- `group/research-group-management.spec.ts` ("admin can create a research group")
selected the shared `admin` user as the group head, permanently assigning admin
to a research group (the head cannot be removed and groups cannot be deleted).
- `/v2/users` (the admin user-search behind account deletion) is scoped to the
current user's research group, so once admin had a group its search no longer
returned the (group-less) seeded students.
- On `develop` this never surfaced because `account-deletion.spec.ts` sorts first
alphabetically and ran before the group test; the subdir reorg flipped the order.
Fix: add a dedicated `e2e_grouphead` realm user and use it as the created group's
head, so the shared `admin` stays group-less and account-deletion's admin tests
are order-independent. No app code changes.
Verified: full local e2e suite passes (236 passed; the lone flaky is a pre-existing
grading-modal timing issue that retries green), including all account-deletion
admin tests and thesis-config-user-search (no collision from the new user).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reorganizes the entire codebase from a layer-based layout (everything grouped by kind —
controller/,service/,entity/,dto/, …) into a feature-based layout where each feature module owns its own controllers, services, repositories, entities, DTOs, etc. The existingdependency/subpackage is the reference — it was already feature-shaped, and the rest of the codebase now follows the same pattern.No behavior changes — purely structural moves plus the import rewrites they imply.
Why
.javafiles and 50+ client components/pages/providers/hooks lived in flat directories. Feature ownership and navigation got harder with each new feature.dependency/package (SBOM/vulnerability scanning) already demonstrated the cleaner shape — extending it to everything else was overdue.@/alias (whichwebpackandvitestwere already configured for but nothing actually used), so cross-feature imports read@/core/hooks/authenticationinstead of../../../../hooks/authentication.Server (
server/src/main/java/de/tum/cit/aet/thesis/)core/config,security,exception,constants,dto,utility,mailvariables. Also holds the merged-feature sub-modules:topic/,application/(+ its cron jobs),user/,group/,notification/,upload/,admin/(Dashboard, DataExport, DataRetention, Calendar, MailingService).thesis/proposal/ThesisProposalentity, repository, mail-variable POJO.interview/presentation/dependency/Each module has its own
controller/(withpayload/),service/,repository/,entity/(withkey/andjsonb/),dto/,constants/,mailvariables/,cron/— only the sub-packages each module actually uses.Root package stays
de.tum.cit.aet.thesisso@SpringBootApplicationcomponent scanning,@EntityScan, and@EnableJpaRepositoriescontinue to work with no explicit config.Client (
client/src/)app/App.tsx,Routes.tsx,layout/,styles/, plusapp/pages/for public/marketing pages (Landing, About, Privacy, Imprint, NotFound, Logout).core/components/,hooks/,utils/,providers/AuthenticationContext,requests/,config/) plus sub-modulestopic/,application/,user/,group/,admin/.thesis/interview/presentation/@/alias adopted everywhere: all 200+ relative imports were rewritten to@/…. Added"baseUrl": "."+"paths": { "@/*": ["src/*"] }totsconfig.json(webpack and vitest already had this alias configured but nothing used it).Tests
ResearchGroupServiceTestandThesisPresentationTitleTesthad to move because they exercise package-private methods on their (now-moved) services.BaseKeycloakIntegrationTestmoved intomock/(it's a shared base class). 27 tests gained explicit imports for sharedmock/*base classes that used to resolve via same-package. All 864 tests still pass.vi.mock(...)paths needed manual fixing (my import-rewrite regex caughtfrom/importstatements but not the mock-path strings).client/e2e/:thesis/,application/,topic/,interview/,presentation/,proposal/,group/,user/,admin/,navigation/. Shared helpers (helpers.ts,mailpit.ts,resource-lock.ts,auth.setup.ts) stay at the e2e root.playwright.config.tsunchanged —testDir: './e2e'discovers recursively. Fixedpublic-api.spec.ts: it usedpath.resolve(__dirname, '..', '..', 'server', 'uploads')which broke when the file moved one level deeper; bumped to three..segments.Notable design choices
core/rather than as top-level modules. This matches how the existing code treated them as supporting features for the main thesis lifecycle.ThesisRole,TopicRole,ApplicationReviewereach live with the entity they join (not in a shared "roles" module). Composite keys travel with their parent entity into akey/sub-package.GradingSchemeComponentsits inthesis/(where assessment logic lives), used by reference fromcore/group/settings — JPA imports across modules are fine.MailingService(low-level send) lives incore/admin/service/; per-featureMail*variable POJOs co-locate with their feature;MailVariablesBuilderstays incore/mailvariables/as the composer.Test plan
./gradlew spotlessApply checkstyleMain checkstyleTest compileJava compileTestJava— clean./gradlew test— 864/864 passcd client && pnpm exec tsc --noEmit— cleancd client && pnpm exec eslint src/— no errors, no warningscd client && pnpm exec vitest run— 84/84 passcd client && pnpm test(vitest + legacy nodetest/*.test.mjs) — all passcd client && pnpm build— webpack build succeeds./execute-e2e-local.sh— 227/231 Playwright tests pass. The 4 remaining failures are inaccount-deletion.spec.ts"Admin Operations" describe and appear to be pre-existing test-ordering interactions with the self-deletion tests earlier in the same file. Not caused by this PR.Reviewer notes
server/build.gradle,client/webpack.config.cjs,client/vitest.config.ts— unchanged.client/tsconfig.json— added the@/*paths config.client/eslint.config.mjs— updated theColorSchemeToggleButtonoverride path to its newcore/components/location (it's the only legitimate consumer ofuseMantineColorScheme).server/src/test/java/.../core/security/KeycloakSecurityIntegrationTest.javaandserver/src/test/java/.../core/user/service/AccessManagementServiceIntegrationTest.java— moved out of thekeycloak/folder and gained explicit imports for the sharedmock/*base classes.dependency/package on both sides is untouched.🤖 Generated with Claude Code