-
Notifications
You must be signed in to change notification settings - Fork 358
Development: Migrate Athena from profile to yml configuration
#12019
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
base: develop
Are you sure you want to change the base?
Development: Migrate Athena from profile to yml configuration
#12019
Conversation
WalkthroughReplaces profile-based activation for Athena and Apollon with custom Spring Conditions ( Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Development: Migrate Athena from profile to yml configuration
d8ad84a to
4ec8863
Compare
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: 2
🤖 Fix all issues with AI agents
In
`@src/main/webapp/app/core/admin/features/admin-feature-toggle.component.spec.ts`:
- Line 15: The import list in admin-feature-toggle.component.spec.ts includes an
unused symbol MODULE_FEATURE_ATHENA; remove MODULE_FEATURE_ATHENA from the
import statement that currently reads "import { MODULE_FEATURE_ATHENA,
MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } ..." so that only
MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, and PROFILE_IRIS remain, ensuring
there are no unused imports referenced in the test file.
- Line 111: The test assertion lengths are wrong: the component's ngOnInit
populates profileFeatures() from displayedProfiles (8 items) and
moduleFeatures() from displayedModuleFeatures (15 items), so update the spec to
assert the actual sizes (e.g., expect(comp.profileFeatures()).toHaveLength(8)
and expect(comp.moduleFeatures()).toHaveLength(15)) or, better, assert against
the source arrays (e.g.,
expect(comp.profileFeatures()).toHaveLength(component.displayedProfiles.length)
and
expect(comp.moduleFeatures()).toHaveLength(component.displayedModuleFeatures.length))
so the expectations stay correct if those arrays change.
| import { MockTranslateService } from 'test/helpers/mocks/service/mock-translate.service'; | ||
| import { MockProfileService } from 'test/helpers/mocks/service/mock-profile.service'; | ||
| import { MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_ATHENA, PROFILE_IRIS } from 'app/app.constants'; | ||
| import { MODULE_FEATURE_ATHENA, MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } from 'app/app.constants'; |
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.
Remove unused import MODULE_FEATURE_ATHENA.
The static analysis correctly identifies that MODULE_FEATURE_ATHENA is imported but never used in this test file.
🔧 Proposed fix
-import { MODULE_FEATURE_ATHENA, MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } from 'app/app.constants';
+import { MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } from 'app/app.constants';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { MODULE_FEATURE_ATHENA, MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } from 'app/app.constants'; | |
| import { MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } from 'app/app.constants'; |
🧰 Tools
🪛 GitHub Check: client-tests-selected
[failure] 15-15:
'MODULE_FEATURE_ATHENA' is declared but its value is never read.
🤖 Prompt for AI Agents
In
`@src/main/webapp/app/core/admin/features/admin-feature-toggle.component.spec.ts`
at line 15, The import list in admin-feature-toggle.component.spec.ts includes
an unused symbol MODULE_FEATURE_ATHENA; remove MODULE_FEATURE_ATHENA from the
import statement that currently reads "import { MODULE_FEATURE_ATHENA,
MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, PROFILE_IRIS } ..." so that only
MODULE_FEATURE_ATLAS, MODULE_FEATURE_EXAM, and PROFILE_IRIS remain, ensuring
there are no unused imports referenced in the test file.
src/main/webapp/app/core/admin/features/admin-feature-toggle.component.spec.ts
Show resolved
Hide resolved
This PR migrates the Apollon feature from Spring Profile to YML-based configuration, consistent with the approach used for LTI and other modules. Server Changes: - Added MODULE_FEATURE_APOLLON and APOLLON_ENABLED_PROPERTY_NAME constants - Added artemis.apollon.enabled: false to application-core.yml - Added isApollonEnabled() method to ArtemisConfigHelper and ModuleFeatureService - Created ApollonEnabled condition class for conditional bean loading - Replaced @Profile(PROFILE_APOLLON) with @conditional(ApollonEnabled.class) in: - ApollonConversionResource - ApollonConversionService - ApollonHealthIndicator - ModelingApollonApi - RestTemplate beans (apollonRestTemplate, shortTimeoutApollonRestTemplate) Client Changes: - Added MODULE_FEATURE_APOLLON constant to app.constants.ts - Removed PROFILE_APOLLON from ProfileFeature type - Updated admin feature toggle to show Apollon in modules section - Updated translations (EN/DE) to move Apollon from profiles to modules Test Changes: - Updated test base classes to use artemis.apollon.enabled=true property - Updated ApollonRequestMockProvider to use @conditional(ApollonEnabled.class) - Updated bean-instantiations.yml workflow Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add artemis.apollon.enabled=true to the example configuration files (application-artemis.yml) to make it clear to administrators how to enable the Apollon PDF conversion feature. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update translations and configuration comments to accurately describe that the Apollon module feature only controls PDF export functionality, not the modeling editor itself. Modeling exercises work normally without Apollon enabled - only the export button is affected. This prevents administrators from being misled into thinking disabling Apollon would disable the entire modeling functionality. Co-Authored-By: Claude Opus 4.5 <[email protected]>
All docstrings and comments now accurately describe that: - artemis.apollon.enabled only controls PDF export - The modeling editor is controlled by artemis.modeling.enabled - Disabling Apollon only affects the export button, not the editor Updated files: - ApollonEnabled.java: Clarified it's for PDF export only - Constants.java: Updated MODULE_FEATURE_APOLLON and property docs - ArtemisConfigHelper.java: Updated isApollonEnabled() javadoc - ModuleFeatureService.java: Updated isApollonEnabled() javadoc - DataExportExerciseCreationService.java: Fixed comment about "profile" Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add ApollonEnabled to ignored classes in ModelingApiArchitectureTest (needed for RestTemplateConfiguration's conditional bean creation) - Update admin-feature-toggle test counts: - Profile features: 10 -> 9 (PROFILE_APOLLON removed) - Module features: 13 -> 14 (MODULE_FEATURE_APOLLON added) Co-Authored-By: Claude Opus 4.5 <[email protected]>
This prevents the ApollonEnabled condition from being evaluated during buildagent profile tests, which don't have the apollon.enabled property configured. The condition should only be evaluated when the core profile is active. Follows the same pattern as AtlasMLRequestMockProvider. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add artemis.apollon.enabled=false to application-buildagent.yml - Add artemis.apollon.enabled=false to AbstractArtemisBuildAgentTest - Add APOLLON_ENABLED_PROPERTY_NAME to ModuleFeatureInfoContributorTest - Add MODULE_FEATURE_APOLLON to ModuleFeatureInfoContributorTest This ensures the apollon property is properly configured for buildagent tests and the ModuleFeatureInfoContributor unit tests. Co-Authored-By: Claude Opus 4.5 <[email protected]>
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: 5
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExerciseCreationService.java`:
- Around line 87-88: Update wording to consistently use the new "Apollon PDF
export" enablement model across DataExportExerciseCreationService: replace
occurrences of "apollon profile" or similar phrases in Javadoc, log messages,
and comments with "Apollon PDF export" or "Apollon PDF export enabled/disabled"
(references: the optional field modelingApollonApi and any Javadoc/logging
around the constructor and methods in DataExportExerciseCreationService,
including the block around lines 235-249). Ensure messages still clearly state
enabled vs. not enabled and adjust capitalization to match the existing "Apollon
PDF export" phrasing.
In `@src/main/webapp/app/core/admin/features/admin-feature-toggle.component.ts`:
- Around line 14-15: The displayedProfiles and profileDocumentationLinks arrays
still include PROFILE_ATHENA which is now represented by MODULE_FEATURE_ATHENA,
causing a duplicate/inactive Athena entry; remove PROFILE_ATHENA from
displayedProfiles and from profileDocumentationLinks (and any corresponding
references around the same component such as where PROFILE_ATHENA is used at
lines ~118-119 and ~166-167) so the UI and docs only reference
MODULE_FEATURE_ATHENA; ensure any display logic or tests that referenced
PROFILE_ATHENA are updated to use MODULE_FEATURE_ATHENA instead.
In `@src/main/webapp/i18n/en/featureToggles.json`:
- Around line 119-126: Remove the duplicate "athena" entry from the profiles
feature list in featureToggles.json: locate the JSON object with the "athena"
key under the profiles/features section and delete that object so Athena is only
defined as a module feature; ensure no other profile-scoped references remain
and that the adjacent "apollon" entry and JSON structure remain valid (commas,
braces) after removal.
In
`@src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java`:
- Line 64: Remove the PROFILE_ATHENA constant from the `@ActiveProfiles`
annotation on the AbstractSpringIntegrationIndependentTest class so the
annotation no longer activates that profile; locate the `@ActiveProfiles`(...)
declaration in AbstractSpringIntegrationIndependentTest and delete
PROFILE_ATHENA from the array while leaving the other profile constants
unchanged.
In
`@src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsLocalVCTest.java`:
- Line 26: Remove PROFILE_ATHENA from the `@ActiveProfiles` annotation in
AbstractSpringIntegrationJenkinsLocalVCTest to avoid masking
`@Conditional`(AthenaEnabled.class) migrations; update the annotation on the class
(the `@ActiveProfiles` declaration referencing SPRING_PROFILE_TEST,
PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC,
PROFILE_JENKINS, PROFILE_ATHENA) to omit PROFILE_ATHENA so tests rely on
property-based activation instead.
🧹 Nitpick comments (2)
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsLocalVCBatchTest.java (1)
29-29: RemovePROFILE_ATHENAhere to validate property‑based gating.
Withartemis.athena.enabled=truealready set below, keeping the profile can unintentionally load profile-specific config and hide leftover@Profile("athena")usage. Dropping it makes this test suite a better guard for the migration.♻️ Suggested change
-@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS }) +@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_AEOLUS })src/main/webapp/app/core/layouts/profiles/shared/profile.service.spec.ts (1)
3-3: Align fixture with module-feature activation.Now that Athena/Apollon are module features, keeping
PROFILE_ATHENAinactiveProfilescan imply profile-based activation and mask regressions in profile handling. Consider removing it from the fixture (and its import).♻️ Proposed update
-import { MODULE_FEATURE_APOLLON, MODULE_FEATURE_ATHENA, PROFILE_ATHENA, PROFILE_DEV, PROFILE_JENKINS, PROFILE_PROD } from 'app/app.constants'; +import { MODULE_FEATURE_APOLLON, MODULE_FEATURE_ATHENA, PROFILE_DEV, PROFILE_JENKINS, PROFILE_PROD } from 'app/app.constants'; @@ - activeProfiles: [PROFILE_PROD, PROFILE_JENKINS, PROFILE_ATHENA], + activeProfiles: [PROFILE_PROD, PROFILE_JENKINS],Also applies to: 92-94
| // we define the field as optional to allow the application to start even if Apollon PDF export is not enabled | ||
| private final Optional<ModelingApollonApi> modelingApollonApi; |
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.
Align Apollon wording with new enablement model.
The updated comment uses “PDF export not enabled”, but the Javadoc/logs below still refer to the “apollon profile”. Consider aligning those strings to avoid mixed terminology.
✅ Suggested wording cleanup
- * Stores the modeling submission as pdf if the apollon profile is active and the apollon conversion service works, otherwise stores it as json file.
+ * Stores the modeling submission as pdf if the Apollon PDF export is enabled and the conversion service works, otherwise stores it as JSON file.
- log.warn("Cannot include modeling submission content in data export as pdf because apollon profile is not active. Going to include the json file");
+ log.warn("Cannot include modeling submission content in data export as pdf because Apollon PDF export is not enabled. Going to include the JSON file");Also applies to: 235-249
🤖 Prompt for AI Agents
In
`@src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExerciseCreationService.java`
around lines 87 - 88, Update wording to consistently use the new "Apollon PDF
export" enablement model across DataExportExerciseCreationService: replace
occurrences of "apollon profile" or similar phrases in Javadoc, log messages,
and comments with "Apollon PDF export" or "Apollon PDF export enabled/disabled"
(references: the optional field modelingApollonApi and any Javadoc/logging
around the constructor and methods in DataExportExerciseCreationService,
including the block around lines 235-249). Ensure messages still clearly state
enabled vs. not enabled and adjust capitalization to match the existing "Apollon
PDF export" phrasing.
| MODULE_FEATURE_APOLLON, | ||
| MODULE_FEATURE_ATHENA, |
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.
Avoid duplicate Athena entries in the admin feature list.
With Athena now a module feature, keeping PROFILE_ATHENA in the profile list/documentation can show a misleading inactive entry. Consider removing it from displayedProfiles and profileDocumentationLinks.
♻️ Proposed cleanup
@@
- private readonly displayedProfiles: ProfileFeature[] = [
- PROFILE_IRIS,
- PROFILE_ATHENA,
+ private readonly displayedProfiles: ProfileFeature[] = [
+ PROFILE_IRIS,
PROFILE_THEIA,
@@
- private readonly profileDocumentationLinks: Partial<Record<ProfileFeature, string>> = {
- [PROFILE_IRIS]: 'https://docs.artemis.tum.de/admin/extensions-setup#iris--pyris-setup-guide',
- [PROFILE_ATHENA]: 'https://docs.artemis.tum.de/admin/extensions-setup#athena-service',
+ private readonly profileDocumentationLinks: Partial<Record<ProfileFeature, string>> = {
+ [PROFILE_IRIS]: 'https://docs.artemis.tum.de/admin/extensions-setup#iris--pyris-setup-guide',
[PROFILE_THEIA]: 'https://docs.artemis.tum.de/developer/setup#run-the-server-via-a-run-configuration-in-intellij',Also applies to: 118-119, 166-167
🤖 Prompt for AI Agents
In `@src/main/webapp/app/core/admin/features/admin-feature-toggle.component.ts`
around lines 14 - 15, The displayedProfiles and profileDocumentationLinks arrays
still include PROFILE_ATHENA which is now represented by MODULE_FEATURE_ATHENA,
causing a duplicate/inactive Athena entry; remove PROFILE_ATHENA from
displayedProfiles and from profileDocumentationLinks (and any corresponding
references around the same component such as where PROFILE_ATHENA is used at
lines ~118-119 and ~166-167) so the UI and docs only reference
MODULE_FEATURE_ATHENA; ensure any display logic or tests that referenced
PROFILE_ATHENA are updated to use MODULE_FEATURE_ATHENA instead.
| }, | ||
| "athena": { | ||
| "name": "Athena (AI Assessment)", | ||
| "description": "Enables Athena ML-based automated feedback suggestions for text and programming exercises." | ||
| }, | ||
| "apollon": { | ||
| "name": "Apollon PDF Export", | ||
| "description": "Enables PDF export of UML diagrams via the external Apollon conversion service. Modeling exercises work without this; only the export button is affected." |
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.
Remove Athena from profile features to avoid duplicate/misleading listing.
Since Athena is now a module feature, keeping it under profiles (Lines 34-37) will show it twice and suggests profile-based activation.
🛠️ Proposed fix
"profiles": {
"iris": {
"name": "Iris (AI Assistant)",
"description": "Enables the Iris AI virtual tutor powered by LLMs for personalized student support and code assistance."
},
- "athena": {
- "name": "Athena (AI Assessment)",
- "description": "Enables Athena ML-based automated feedback suggestions for text and programming exercises."
- },
"theia": {
"name": "Theia (Online IDE)",
"description": "Enables the Theia cloud-based IDE for programming exercises, allowing students to code in the browser."
},🤖 Prompt for AI Agents
In `@src/main/webapp/i18n/en/featureToggles.json` around lines 119 - 126, Remove
the duplicate "athena" entry from the profiles feature list in
featureToggles.json: locate the JSON object with the "athena" key under the
profiles/features section and delete that object so Athena is only defined as a
module feature; ensure no other profile-scoped references remain and that the
adjacent "apollon" entry and JSON structure remain valid (commas, braces) after
removal.
| // NOTE: we use a common set of active profiles to reduce the number of application launches during testing. This significantly saves time and memory! | ||
| // TODO: PROFILE_AEOLUS is bound to PROGRAMMING and LOCAL_VC and should not be active in an independent test context. | ||
| @ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_TEST_INDEPENDENT, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_ATHENA, PROFILE_APOLLON, PROFILE_IRIS, PROFILE_AEOLUS }) | ||
| @ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_TEST_INDEPENDENT, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_ATHENA, PROFILE_IRIS, PROFILE_AEOLUS }) |
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.
Consider dropping PROFILE_ATHENA from @ActiveProfiles.
Line 64: with property-based enablement in place, the profile can hide remaining profile-gated beans.
Suggested change
-@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_TEST_INDEPENDENT, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_ATHENA, PROFILE_IRIS, PROFILE_AEOLUS })
+@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_TEST_INDEPENDENT, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_IRIS, PROFILE_AEOLUS })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_TEST_INDEPENDENT, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_ATHENA, PROFILE_IRIS, PROFILE_AEOLUS }) | |
| `@ActiveProfiles`({ SPRING_PROFILE_TEST, PROFILE_TEST_INDEPENDENT, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_IRIS, PROFILE_AEOLUS }) |
🤖 Prompt for AI Agents
In
`@src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java`
at line 64, Remove the PROFILE_ATHENA constant from the `@ActiveProfiles`
annotation on the AbstractSpringIntegrationIndependentTest class so the
annotation no longer activates that profile; locate the `@ActiveProfiles`(...)
declaration in AbstractSpringIntegrationIndependentTest and delete
PROFILE_ATHENA from the array while leaving the other profile constants
unchanged.
| @ResourceLock("AbstractSpringIntegrationJenkinsLocalVCTest") | ||
| // NOTE: we use a common set of active profiles to reduce the number of application launches during testing. This significantly saves time and memory! | ||
| @ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS, PROFILE_APOLLON }) | ||
| @ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS }) |
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.
Consider dropping PROFILE_ATHENA from @ActiveProfiles.
Line 26: with property-based activation, keeping the profile can mask missed migrations to @Conditional(AthenaEnabled.class).
Suggested change
-@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS })
+@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_AEOLUS })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS }) | |
| `@ActiveProfiles`({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_AEOLUS }) |
🤖 Prompt for AI Agents
In
`@src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsLocalVCTest.java`
at line 26, Remove PROFILE_ATHENA from the `@ActiveProfiles` annotation in
AbstractSpringIntegrationJenkinsLocalVCTest to avoid masking
`@Conditional`(AthenaEnabled.class) migrations; update the annotation on the class
(the `@ActiveProfiles` declaration referencing SPRING_PROFILE_TEST,
PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC,
PROFILE_JENKINS, PROFILE_ATHENA) to omit PROFILE_ATHENA so tests rely on
property-based activation instead.
Similar to PR #12003 for LTI, this change migrates the Athena module from using @Profile("athena") annotation to @conditional(AthenaEnabled.class) with yml-based configuration (artemis.athena.enabled property). This provides a more consistent and unified approach to feature configuration across the Artemis platform. Changes: - Added AthenaEnabled condition class - Replaced @Profile(PROFILE_ATHENA) with @conditional(AthenaEnabled.class) - Added artemis.athena.enabled property to application-core.yml - Updated ArtemisConfigHelper and ModuleFeatureService - Updated RestTemplateConfiguration for Athena beans - Updated test base classes to use property instead of profile - Moved Athena from profiles to modules in admin feature toggle UI - Updated translations for module feature display Co-Authored-By: Claude Opus 4.5 <[email protected]>
Client-side fixes: - Migrate all PROFILE_ATHENA references to MODULE_FEATURE_ATHENA - Change isProfileActive() calls to isModuleFeatureActive() - Update test mocks to use activeModuleFeatures instead of activeProfiles - Fix admin-feature-toggle test: 10->9 profiles, 13->14 module features Server-side fixes: - Add athena.enabled: false to application-buildagent.yml - Add artemis.athena.enabled=false to AbstractArtemisBuildAgentTest - Add ATHENA_ENABLED_PROPERTY_NAME and MODULE_FEATURE_ATHENA to ModuleFeatureInfoContributorTest Files migrated from PROFILE_ATHENA to MODULE_FEATURE_ATHENA: - athena.service.ts - course-detail.component.ts - course-update.component.ts - request-feedback-button.component.ts (and spec) - exercise-details-student-actions.component.ts - external-data.guard.ts - user-settings-container.component.spec.ts - exercise-feedback-suggestion-options.component.ts (and spec) - programming-exercise-lifecycle.component.ts - admin-feature-toggle.component.spec.ts Co-Authored-By: Claude Opus 4.5 <[email protected]>
…tion issues - Add dedicated AthenaApiNotPresentException for proper error messages - Update all Athena service files to use the new exception class - Fix Apollon profile check to use module feature in modeling detail component Co-Authored-By: Claude Opus 4.5 <[email protected]>
9aa3ef1 to
108bf9f
Compare
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
|
@FelixTJDietrich Your PR description needs attention before it can be reviewed: Issues Found
How to Fix
This check validates that your PR description follows the PR template. A complete description helps reviewers understand your changes and speeds up the review process.
|
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: 1
🤖 Fix all issues with AI agents
In `@src/main/webapp/app/core/layouts/profiles/shared/profile.service.spec.ts`:
- Line 3: The test fixture includes PROFILE_ATHENA in activeProfiles which is
misleading now that Athena is enabled via module flag; remove PROFILE_ATHENA
from any activeProfiles arrays in profile.service.spec.ts and update any
assertions that expect that profile to instead check the module flag
(MODULE_FEATURE_ATHENA) or the module-feature API path; search for the
activeProfiles fixture and occurrences around lines ~92-93 and replace
profile-based checks for Athena with checks against MODULE_FEATURE_ATHENA (or
mock the module flag) so tests reflect module-based enablement rather than a
legacy profile constant.
🧹 Nitpick comments (2)
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsLocalVCTemplateTest.java (1)
29-29: DropPROFILE_ATHENAto avoid masking missing migrations.
Since the test already enables Athena via properties, keeping the profile can hide leftover@Profile("athena")usage and reduce coverage of the new conditional path.♻️ Suggested adjustment
-import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_ATHENA; @@ -@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS }) +@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_AEOLUS })src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationJenkinsLocalVCBatchTest.java (1)
29-29: Consider removingPROFILE_ATHENAfrom@ActiveProfiles.Same concern as the non-batch variant: with
artemis.athena.enabled=truein@TestPropertySource, the profile becomes redundant and may hide incomplete migrations to@Conditional(AthenaEnabled.class).Suggested change
-@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_ATHENA, PROFILE_AEOLUS }) +@ActiveProfiles({ SPRING_PROFILE_TEST, PROFILE_ARTEMIS, PROFILE_CORE, PROFILE_SCHEDULING, PROFILE_LOCALVC, PROFILE_JENKINS, PROFILE_AEOLUS })
| import { TestBed } from '@angular/core/testing'; | ||
| import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; | ||
| import { PROFILE_APOLLON, PROFILE_ATHENA, PROFILE_DEV, PROFILE_JENKINS, PROFILE_PROD } from 'app/app.constants'; | ||
| import { MODULE_FEATURE_APOLLON, MODULE_FEATURE_ATHENA, PROFILE_ATHENA, PROFILE_DEV, PROFILE_JENKINS, PROFILE_PROD } from 'app/app.constants'; |
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.
Align activeProfiles fixture with module-based Athena enablement.
If Athena is now only a module feature, keeping PROFILE_ATHENA in activeProfiles can mix activation models and make the fixture misleading. Consider removing it if the backend no longer exposes that profile.
🛠️ Suggested update
-import { MODULE_FEATURE_APOLLON, MODULE_FEATURE_ATHENA, PROFILE_ATHENA, PROFILE_DEV, PROFILE_JENKINS, PROFILE_PROD } from 'app/app.constants';
+import { MODULE_FEATURE_APOLLON, MODULE_FEATURE_ATHENA, PROFILE_DEV, PROFILE_JENKINS, PROFILE_PROD } from 'app/app.constants';
- activeProfiles: [PROFILE_PROD, PROFILE_JENKINS, PROFILE_ATHENA],
+ activeProfiles: [PROFILE_PROD, PROFILE_JENKINS],Also applies to: 92-93
🤖 Prompt for AI Agents
In `@src/main/webapp/app/core/layouts/profiles/shared/profile.service.spec.ts` at
line 3, The test fixture includes PROFILE_ATHENA in activeProfiles which is
misleading now that Athena is enabled via module flag; remove PROFILE_ATHENA
from any activeProfiles arrays in profile.service.spec.ts and update any
assertions that expect that profile to instead check the module flag
(MODULE_FEATURE_ATHENA) or the module-feature API path; search for the
activeProfiles fixture and occurrences around lines ~92-93 and replace
profile-based checks for Athena with checks against MODULE_FEATURE_ATHENA (or
mock the module flag) so tests reflect module-based enablement rather than a
legacy profile constant.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
Motivation and Context
Similar to PR #12003 for LTI, this change migrates the Athena module from using
@Profile("athena")annotation to@Conditional(AthenaEnabled.class)with yml-based configuration (artemis.athena.enabledproperty).This provides a more consistent and unified approach to feature configuration across the Artemis platform.
Description
This PR migrates the Athena module from Spring profile-based activation to a yml-based configuration using
@Conditional.Changes:
AthenaEnabledcondition class that checksartemis.athena.enabledproperty@Profile(PROFILE_ATHENA)with@Conditional(AthenaEnabled.class)in all Athena-related classes:AthenaResource,AthenaInternalResourceAthenaApi,AthenaFeedbackApiAthenaModuleService,AthenaFeedbackSendingService,AthenaFeedbackSuggestionsService,AthenaSubmissionSendingService,AthenaSubmissionSelectionService,AthenaDTOConverterService,AthenaRepositoryExportService,AthenaScheduleServiceAthenaHealthIndicator,AthenaAuthorizationInterceptorRestTemplateConfigurationAthenaRequestMockProvidertest componentartemis.athena.enabled: falseproperty toapplication-core.ymlArtemisConfigHelperwithisAthenaEnabled()methodModuleFeatureServicewithisAthenaEnabled()methodMODULE_FEATURE_ATHENAconstantSpecial handling for
AthenaScheduleService:This service previously used
@Profile("athena & scheduling"). It now uses both@Conditional(AthenaEnabled.class)and@Profile(PROFILE_SCHEDULING)to maintain the same behavior.Steps for Testing
artemis.athena.enabled=true→ Athena features should be availableartemis.athena.enabled=false(default) → Athena features should not be availableTestserver States
Note
Tutors on test servers can't reset the database.
Review Progress
Performance Review
Code Review
Maintainer
Summary by Claude
This PR migrates the Athena module from Spring profile-based configuration (
@Profile("athena")) to yml-based configuration using@Conditional(AthenaEnabled.class)and theartemis.athena.enabledproperty. This change aligns with the pattern established in PR #12003 for LTI and provides a more consistent configuration approach across the platform.🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
New Features
UI / Localization
✏️ Tip: You can customize this high-level summary in your review settings.