Communication: Enable IRIS to answer with FAQs
#10093
Conversation
github-actions
bot
added
tests
server
cremertim
changed the title
Communication: Enable IRIS to answer with FAQs
github-actions
bot
added
the
core
# Conflicts: # src/main/webapp/i18n/de/iris.json # src/main/webapp/i18n/en/iris.json
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration.
WalkthroughThis pull request introduces a comprehensive implementation of FAQ (Frequently Asked Questions) ingestion functionality for the Iris system. The changes span multiple components across the backend and frontend, enabling the automatic and manual ingestion of FAQs into the Pyris system. The implementation includes a new Changes
Sequence DiagramsequenceDiagram
participant Instructor
participant FaqResource
participant FaqService
participant PyrisWebhookService
participant PyrisConnectorService
Instructor->>FaqResource: Trigger FAQ Ingestion
FaqResource->>FaqService: ingestFaqsIntoPyris(courseId, faqId)
FaqService->>PyrisWebhookService: autoUpdateFaqInPyris
PyrisWebhookService->>PyrisConnectorService: executeFaqAdditionWebhook
PyrisConnectorService-->>PyrisWebhookService: Webhook Response
PyrisWebhookService-->>FaqService: Ingestion Result
FaqService-->>FaqResource: Ingestion Completed
FaqResource-->>Instructor: Ingestion Status
Possibly related PRs
Suggested Labels
Suggested Reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (39)
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisWebhookFaqDeletionExecutionDTO.java (1)
10-11: Add JavaDoc documentation and field validation.Please enhance the record with:
- JavaDoc documentation describing the purpose of the DTO and its fields
- Validation annotations for the fields where appropriate
Here's the suggested implementation:
@JsonInclude(JsonInclude.Include.NON_EMPTY) +/** + * Data Transfer Object for FAQ deletion execution in the Pyris webhook system. + * + * @param pyrisFaqWebhookDTO The FAQ webhook data to be processed + * @param settings The pipeline execution settings for the deletion + * @param initialStages The initial stages of the pipeline execution + */ public record PyrisWebhookFaqDeletionExecutionDTO( + @NotNull PyrisFaqWebhookDTO pyrisFaqWebhookDTO, + @NotNull PyrisPipelineExecutionSettingsDTO settings, + @Valid List<PyrisStageDTO> initialStages) { }src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqIngestionStatusUpdateDTO.java (2)
9-11: Add JavaDoc documentation for better maintainability.The implementation follows the coding guidelines by using a Java record with minimal fields and appropriate JSON serialization settings. However, please add JavaDoc documentation to describe:
- The purpose of this DTO
- The meaning of each field
- Valid values for the
resultfield+/** + * Data Transfer Object for FAQ ingestion status updates from Pyris. + * + * @param result The result status of the FAQ ingestion job + * @param stages List of stages the ingestion job went through + * @param jobId Unique identifier of the ingestion job + */ @JsonInclude(JsonInclude.Include.NON_EMPTY) public record PyrisFaqIngestionStatusUpdateDTO(String result, List<PyrisStageDTO> stages, long jobId) { }
10-10: Consider using an enum for the result field.To ensure consistency and type safety, consider replacing the String result field with an enum that defines all possible status values.
public enum PyrisFaqIngestionResult { SUCCESS, FAILURE, IN_PROGRESS // Add other possible status values }Then update the record:
-public record PyrisFaqIngestionStatusUpdateDTO(String result, List<PyrisStageDTO> stages, long jobId) { +public record PyrisFaqIngestionStatusUpdateDTO(PyrisFaqIngestionResult result, List<PyrisStageDTO> stages, long jobId) {src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisExerciseSettings.java (1)
93-97: Enhance documentation and code style.While the implementation is correct, consider these improvements for better maintainability:
- Use formal JavaDoc style comments
- Place @OverRide annotations on separate lines
+ /** + * Returns FAQ ingestion settings for this exercise. + * @return null as exercises don't have exercise FAQ settings + */ @Override public IrisFaqIngestionSubSettings getIrisFaqIngestionSettings() { // Empty because exercises don't have exercise faq settings return null; } + /** + * Sets FAQ ingestion settings for this exercise. + * @param irisFaqIngestionSubSettings the settings to set (ignored) + */ @Override public void setIrisFaqIngestionSettings(IrisFaqIngestionSubSettings irisFaqIngestionSubSettings) { // Empty because exercises don't have exercise faq settings }Also applies to: 99-103
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/FaqIngestionWebhookJob.java (1)
6-9: Enhance JavaDoc documentation.The current JavaDoc has a typo and could be more descriptive about the job's role in the FAQ system.
/** - * An implementation of a PyrisJob for Faq Ingestion in Pyris. - * This job is used to reference the details of then Ingestion when Pyris sends a status update. + * An implementation of a PyrisJob for FAQ ingestion in Pyris. + * This job is used to reference the details of the ingestion when Pyris sends a status update. + * + * @param jobId The unique identifier for this ingestion job + * @param courseId The identifier of the course containing the FAQ + * @param faqId The identifier of the FAQ being ingested */src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java (3)
98-100: Refactor to avoid code duplication in FAQ ingestion logic.The code blocks in
createFaqandupdateFaqmethods that check if the FAQ is accepted and then callfaqService.autoIngestFaqsIntoPyrisare duplicated. Extract this logic into a private method to adhere to the DRY (Don't Repeat Yourself) principle and improve maintainability.Apply this refactoring:
private void ingestFaqIfAccepted(Long courseId, Faq faq) { if (isFaqAccepted(faq)) { faqService.autoIngestFaqsIntoPyris(courseId, faq); } }Then update the
createFaqandupdateFaqmethods:// In createFaq - if (checkIfFaqIsAccepted(savedFaq)) { - faqService.autoIngestFaqsIntoPyris(courseId, savedFaq); - } + ingestFaqIfAccepted(courseId, savedFaq); // In updateFaq - if (checkIfFaqIsAccepted(updatedFaq)) { - faqService.autoIngestFaqsIntoPyris(courseId, updatedFaq); - } + ingestFaqIfAccepted(courseId, updatedFaq);Also applies to: 127-129
243-244: Remove redundant role check;@EnforceAtLeastInstructorInCoursealready handles authorization.The explicit call to
authCheckService.checkHasAtLeastRoleInCourseElseThrowis unnecessary because the@EnforceAtLeastInstructorInCourseannotation on theingestFaqInIrismethod already ensures that only instructors can access this endpoint. Removing the redundant check simplifies the code.Apply this diff to remove the redundant authorization check:
public ResponseEntity<Void> ingestFaqInIris(@PathVariable Long courseId, @RequestParam(required = false) Optional<Long> faqId) { Course course = courseRepository.findByIdElseThrow(courseId); - authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null); faqService.ingestFaqsIntoPyris(courseId, faqId); return ResponseEntity.ok().build(); }
272-275: Rename method toisFaqAcceptedto follow Java naming conventions.Methods that return a boolean should be named using the
isprefix to enhance readability and align with Java conventions. RenamingcheckIfFaqIsAcceptedtoisFaqAcceptedimproves clarity.Apply this diff:
- private boolean checkIfFaqIsAccepted(Faq faq) { + private boolean isFaqAccepted(Faq faq) { return faq.getFaqState() == FaqState.ACCEPTED; }Ensure all references to this method are updated accordingly.
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSubSettingsType.java (1)
5-5: Consider addressing the TODO comment for consistency.While adding
FAQ_INGESTIONto theIrisSubSettingsTypeenum is correct, there is aTODOcomment suggesting to renameCHATtoPROGRAMMING_EXERCISE_CHAT. Addressing this would enhance clarity and maintain consistency in the enum values.If renaming is out of scope for this PR, consider creating a separate task to track this change.
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisWebhookFaqIngestionExecutionDTO.java (1)
10-11: Consider adding validation annotations for required fields.The DTO fields lack validation annotations. Consider adding
@NotNullfor required fields to ensure data integrity at the API boundary.@JsonInclude(JsonInclude.Include.NON_EMPTY) -public record PyrisWebhookFaqIngestionExecutionDTO(PyrisFaqWebhookDTO pyrisFaqWebhookDTO, PyrisPipelineExecutionSettingsDTO settings, List<PyrisStageDTO> initialStages) { +public record PyrisWebhookFaqIngestionExecutionDTO( + @NotNull(message = "FAQ webhook data must not be null") + PyrisFaqWebhookDTO pyrisFaqWebhookDTO, + @NotNull(message = "Pipeline settings must not be null") + PyrisPipelineExecutionSettingsDTO settings, + List<PyrisStageDTO> initialStages) {src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqWebhookDTO.java (2)
5-9: Enhance JavaDoc with field descriptions.While the class-level documentation is good, it would be beneficial to document individual fields using
@paramtags./** * Represents a webhook data transfer object for an FAQ in the Pyris system. * This DTO is used to encapsulate the information related to updates of lecture units, * providing necessary details such as lecture and course identifiers, names, and descriptions. + * + * @param faqId The unique identifier of the FAQ + * @param questionTitle The title of the FAQ question + * @param questionAnswer The answer to the FAQ question + * @param courseId The unique identifier of the course + * @param courseName The name of the course + * @param courseDescription The description of the course */
12-12: Add validation constraints for String fields.Consider adding validation annotations to ensure data integrity and prevent empty or oversized strings.
-public record PyrisFaqWebhookDTO(long faqId, String questionTitle, String questionAnswer, long courseId, String courseName, String courseDescription) { +public record PyrisFaqWebhookDTO( + long faqId, + @NotBlank(message = "Question title must not be blank") + @Size(max = 255, message = "Question title must not exceed 255 characters") + String questionTitle, + @NotBlank(message = "Question answer must not be blank") + String questionAnswer, + long courseId, + @NotBlank(message = "Course name must not be blank") + @Size(max = 255, message = "Course name must not exceed 255 characters") + String courseName, + String courseDescription) {src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/LectureIngestionWebhookJob.java (1)
Line range hint
6-8: Improve class documentation to explain access control behavior.The current documentation lacks explanation of why access is always denied. Consider enhancing it to explain the security model.
/** * An implementation of a PyrisJob for Lecture Ingestion in Pyris. * This job is used to reference the details of then Ingestion when Pyris sends a status update. + * + * This implementation enforces strict access control by denying access to both course and exercise + * contexts as lecture ingestion should only be handled through the designated service layer. + * + * @see PyrisJob */Also applies to: 10-10
src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedSettingsDTO.java (1)
Line range hint
7-14: Consider grouping related settings fields together.The FAQ ingestion settings could be grouped with other ingestion-related settings for better organization.
public record IrisCombinedSettingsDTO( IrisCombinedChatSubSettingsDTO irisChatSettings, IrisCombinedTextExerciseChatSubSettingsDTO irisTextExerciseChatSettings, IrisCombinedCourseChatSubSettingsDTO irisCourseChatSettings, IrisCombinedLectureIngestionSubSettingsDTO irisLectureIngestionSettings, + IrisCombinedFaqIngestionSubSettingsDTO irisFaqIngestionSettings, - IrisCombinedCompetencyGenerationSubSettingsDTO irisCompetencyGenerationSettings, - IrisCombinedFaqIngestionSubSettingsDTO irisFaqIngestionSettings + IrisCombinedCompetencyGenerationSubSettingsDTO irisCompetencyGenerationSettings ) {}src/main/webapp/app/entities/iris/settings/iris-sub-settings.model.ts (2)
9-9: Use consistent enum value format.The enum value format 'faq-ingestion' doesn't match the format of other ingestion-related values (e.g., 'lecture-ingestion'). Consider maintaining consistency.
- FAQ_INGESTION = 'faq-ingestion', + FAQ_INGESTION = 'faq-ingestion', // Current format matches existing pattern
50-53: Initialize boolean property.The
autoIngestOnFaqCreationproperty should be initialized to maintain consistency with other similar settings classes.export class IrisFaqIngestionSubSettings extends IrisSubSettings { type = IrisSubSettingsType.FAQ_INGESTION; - autoIngestOnFaqCreation: boolean; + autoIngestOnFaqCreation = false; }src/main/java/de/tum/cit/aet/artemis/communication/service/FaqService.java (2)
16-18: Add class-level documentation.Add JavaDoc to describe the service's purpose, dependencies, and profile configuration.
+/** + * Service for managing FAQ ingestion into the Pyris system. + * This service is only active when the CORE profile is enabled. + */ @Profile(PROFILE_CORE) @Service public class FaqService {
62-68: Add transaction boundary and logging to autoIngestFaqsIntoPyris.The method should include transaction boundary and logging for better observability.
+ @Transactional(readOnly = true) public void autoIngestFaqsIntoPyris(Long courseId, Faq faq) { + log.debug("Request to auto-ingest FAQ {} for course {}", faq.getId(), courseId); if (pyrisWebhookService.isEmpty()) { + log.debug("Pyris webhook service not available, skipping auto-ingestion"); return; } + log.debug("Auto-updating FAQ {} in Pyris", faq.getId()); pyrisWebhookService.get().autoUpdateFaqInPyris(courseId, faq); }src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSettings.java (1)
63-67: Add JavaDoc for the new FAQ ingestion settings methods.While the implementation is correct, consider adding JavaDoc comments to document these methods, consistent with the class's documentation style.
Add documentation like this:
+ /** + * Gets the FAQ ingestion settings for this Iris instance. + * + * @return The FAQ ingestion settings + */ public abstract IrisFaqIngestionSubSettings getIrisFaqIngestionSettings(); + /** + * Sets the FAQ ingestion settings for this Iris instance. + * + * @param irisFaqIngestionSubSettings The FAQ ingestion settings to set + */ public abstract void setIrisFaqIngestionSettings(IrisFaqIngestionSubSettings irisFaqIngestionSubSettings);src/test/javascript/spec/component/iris/settings/iris-global-settings-update.component.spec.ts (1)
52-52: Improve test case clarity and add test for FAQ ingestion settings.While the updated assertion is correct, consider:
- Making the test case name more descriptive about what it's checking
- Adding a specific test case for the FAQ ingestion settings component
Consider updating the test like this:
- it('Setup works correctly', () => { + it('should render all sub-settings components including FAQ ingestion', () => { fixture.detectChanges(); expect(comp.settingsUpdateComponent).toBeTruthy(); expect(getSettingsSpy).toHaveBeenCalledOnce(); expect(fixture.debugElement.queryAll(By.directive(IrisCommonSubSettingsUpdateComponent))).toHaveLength(6); + // Verify FAQ ingestion settings component is present + const subSettings = fixture.debugElement.queryAll(By.directive(IrisCommonSubSettingsUpdateComponent)); + expect(subSettings.some(el => el.componentInstance.subSettings === comp.settingsUpdateComponent?.irisSettings.irisFaqIngestionSettings)).toBeTrue(); });src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java (1)
116-120: Fix incorrect JavaDoc description.The JavaDoc comment incorrectly states that this method checks for the "aeolus" profile. Update it to correctly describe the IRIS profile check.
/** - * Checks if the aeolus profile is active + * Checks if the iris profile is active * - * @return true if the aeolus profile is active, false otherwise + * @return true if the iris profile is active, false otherwise */src/main/webapp/app/faq/faq.service.ts (1)
166-172: Remove unnecessary HttpParams creation.The empty HttpParams object is not needed since no query parameters are being used.
ingestFaqsInPyris(courseId: number): Observable<HttpResponse<void>> { - const params = new HttpParams(); return this.http.post<void>(`api/courses/${courseId}/faqs/ingest`, null, { - params: params, observe: 'response', }); }src/main/java/de/tum/cit/aet/artemis/iris/web/IrisResource.java (1)
135-135: Consider consistency in response map handling.Other similar endpoints (
getLecturesIngestionStateandgetLectureUnitsIngestionState) return maps with multiple entries. Consider delegating the map creation toPyrisWebhookServicefor consistency.-Map<Long, IngestionState> responseMap = Map.of(faqId, pyrisWebhookService.getFaqIngestionState(courseId, faqId)); +return ResponseEntity.ok(pyrisWebhookService.getFaqIngestionState(courseId, faqId));And update
PyrisWebhookService.getFaqIngestionStateto returnMap<Long, IngestionState>.src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisStatusUpdateService.java (1)
140-148: Fix JavaDoc comment.The JavaDoc comment incorrectly mentions "lecture ingestion" instead of "FAQ ingestion".
Apply this diff to fix the JavaDoc:
- * Handles the status update of a lecture ingestion job. + * Handles the status update of a FAQ ingestion job.src/main/java/de/tum/cit/aet/artemis/iris/web/open/PublicPyrisStatusUpdateResource.java (2)
171-177: Improve error message for job type check.The error message "Run ID is not an ingestion job" is misleading since it's specifically checking for a lecture ingestion job.
- throw new ConflictException("Run ID is not an ingestion job", "Job", "invalidRunId"); + throw new ConflictException("Run ID is not a lecture ingestion job", "Job", "invalidRunId");
179-200: LGTM! Consider adding logging.The implementation follows the established pattern for status update endpoints. Consider adding debug logging for better observability.
public ResponseEntity<Void> setStatusOfFaqIngestionJob(@PathVariable String runId, @RequestBody PyrisFaqIngestionStatusUpdateDTO statusUpdateDTO, HttpServletRequest request) { + log.debug("Received FAQ ingestion status update for run ID: {}", runId); PyrisJob job = pyrisJobService.getAndAuthenticateJobFromHeaderElseThrow(request, PyrisJob.class); if (!job.jobId().equals(runId)) { throw new ConflictException("Run ID in URL does not match run ID in request body", "Job", "runIdMismatch"); } if (!(job instanceof FaqIngestionWebhookJob faqIngestionWebhookJob)) { - throw new ConflictException("Run ID is not an ingestion job", "Job", "invalidRunId"); + throw new ConflictException("Run ID is not a FAQ ingestion job", "Job", "invalidRunId"); } pyrisStatusUpdateService.handleStatusUpdate(faqIngestionWebhookJob, statusUpdateDTO); + log.debug("Successfully processed FAQ ingestion status update for run ID: {}", runId); return ResponseEntity.ok().build(); }src/test/javascript/spec/service/faq.service.spec.ts (1)
261-275: Enhance test coverage for FAQ ingestion.While the basic happy path is tested, consider adding:
- Error case testing
- Request body verification
- Response body verification if applicable
it('should handle errors when ingesting FAQs', () => { const courseId = 123; const expectedUrl = `api/courses/${courseId}/faqs/ingest`; service.ingestFaqsInPyris(courseId).subscribe({ error: (error) => { expect(error.status).toBe(500); } }); const req = httpMock.expectOne({ url: expectedUrl, method: 'POST' }); req.flush('Error', { status: 500, statusText: 'Server Error' }); }); it('should send correct request body when ingesting FAQs', () => { const courseId = 123; const expectedUrl = `api/courses/${courseId}/faqs/ingest`; service.ingestFaqsInPyris(courseId).subscribe(); const req = httpMock.expectOne({ url: expectedUrl, method: 'POST' }); expect(req.request.body).toEqual(/* expected request body */); req.flush({}, { status: 200, statusText: 'OK' }); });src/test/java/de/tum/cit/aet/artemis/communication/FaqIntegrationTest.java (1)
172-174: Enhance webhook DTO verification.While the test verifies the authentication token, consider verifying other important aspects of the webhook DTO.
irisRequestMockProvider.mockFaqDeletionWebhookRunResponse(dto -> { assertThat(dto.settings().authenticationToken()).isNotNull(); + assertThat(dto.settings().baseUrl()).isNotNull(); + assertThat(dto.settings().runId()).isNotNull(); + assertThat(dto.faqIds()).isNotEmpty(); });src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java (3)
207-220: Consider retrying failed requests.The FAQ addition webhook could benefit from retry logic for transient failures.
Consider using Spring Retry or implementing a custom retry mechanism:
@Retryable( value = { RestClientException.class }, maxAttempts = 3, backoff = @Backoff(delay = 1000) ) public void executeFaqAdditionWebhook(PyrisFaqWebhookDTO toUpdateFaq, PyrisWebhookFaqIngestionExecutionDTO executionDTO)
242-261: Improve error handling in getFaqIngestionState.The error message concatenation could lead to unclear messages.
- throw new PyrisConnectorException("Error fetching ingestion state for faq" + faqId); + throw new PyrisConnectorException(String.format("Error fetching ingestion state for FAQ %d in course %d", faqId, courseId));
151-154: Consider extracting job filtering logic.The job filtering logic could be moved to a separate method for better readability and reusability.
+ private boolean isLectureIngestionJobInProgress(long courseId, long lectureId, long lectureUnitId) { + return pyrisJobService.currentJobs().stream() + .filter(job -> job instanceof LectureIngestionWebhookJob) + .map(job -> (LectureIngestionWebhookJob) job) + .anyMatch(ingestionJob -> + ingestionJob.courseId() == courseId && + ingestionJob.lectureId() == lectureId && + ingestionJob.lectureUnitId() == lectureUnitId); + } IngestionState getLectureUnitIngestionState(long courseId, long lectureId, long lectureUnitId) { // ... if (state != IngestionState.DONE) { - if (pyrisJobService.currentJobs().stream() - .filter(job -> job instanceof LectureIngestionWebhookJob) - .map(job -> (LectureIngestionWebhookJob) job) - .anyMatch(ingestionJob -> ingestionJob.courseId() == courseId && - ingestionJob.lectureId() == lectureId && - ingestionJob.lectureUnitId() == lectureUnitId)) { + if (isLectureIngestionJobInProgress(courseId, lectureId, lectureUnitId)) { return IngestionState.IN_PROGRESS; } }src/test/javascript/spec/component/faq/faq.component.spec.ts (2)
248-254: Enhance test coverage for error scenarios.While the test verifies the basic ingestion functionality, it could be improved by:
- Testing with multiple FAQs
- Verifying the exact error message content
it('should call the service to ingest lectures when ingestLecturesInPyris is called', () => { - faqComponent.faqs = [faq1]; + const faq2 = createFaq(2, 'category2', '#0ab84f'); + faqComponent.faqs = [faq1, faq2]; const ingestSpy = jest.spyOn(faqService, 'ingestFaqsInPyris').mockImplementation(() => of(new HttpResponse<void>({ status: 200 }))); faqComponent.ingestFaqsInPyris(); expect(ingestSpy).toHaveBeenCalledWith(faq1.course?.id); expect(ingestSpy).toHaveBeenCalledOnce(); + expect(faqComponent.faqs).toHaveLength(2); });
256-262: Verify error handling with specific error messages.The error handling test should verify the exact error message being logged.
it('should log error when error occurs', () => { alertServiceStub = jest.spyOn(alertService, 'error'); faqComponent.faqs = [faq1]; - jest.spyOn(faqService, 'ingestFaqsInPyris').mockReturnValue(throwError(() => new Error('Error while ingesting'))); + const errorMessage = 'Error while ingesting FAQs'; + jest.spyOn(faqService, 'ingestFaqsInPyris').mockReturnValue(throwError(() => new Error(errorMessage))); faqComponent.ingestFaqsInPyris(); expect(alertServiceStub).toHaveBeenCalledOnce(); + expect(alertServiceStub).toHaveBeenCalledWith(errorMessage); });src/test/java/de/tum/cit/aet/artemis/iris/PyrisFaqIngestionTest.java (2)
61-73: Consider adding cleanup in tearDown.While the setup is thorough, consider adding a tearDown method to clean up created resources.
+ @AfterEach + void tearDown() { + faqRepository.deleteAll(); + courseRepository.deleteAll(); + }
228-244: Enhance error message assertions.The test verifies error responses but could be more specific in error message assertions.
- assertThat(response.getContentAsString()).contains("Run ID in URL does not match run ID in request body"); + assertThat(response.getContentAsString()) + .as("Should contain specific error message for mismatched run ID") + .contains("Run ID in URL does not match run ID in request body");src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (1)
259-265: Consider simplifying null check chain.The nested null checks and conditions could be simplified using Optional.
- IrisCourseSettings courseSettings = irisSettingsRepository.findCourseSettings(courseId).isPresent() ? irisSettingsRepository.findCourseSettings(courseId).get() : null; - if (courseSettings != null && courseSettings.getIrisFaqIngestionSettings() != null && courseSettings.getIrisFaqIngestionSettings().isEnabled() - && courseSettings.getIrisFaqIngestionSettings().getAutoIngestOnFaqCreation()) { + boolean shouldAutoIngest = irisSettingsRepository.findCourseSettings(courseId) + .map(settings -> settings.getIrisFaqIngestionSettings()) + .filter(faqSettings -> faqSettings.isEnabled() && faqSettings.getAutoIngestOnFaqCreation()) + .isPresent(); + if (shouldAutoIngest) {src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java (1)
370-382: Fix typo in method name.The method name
combinceFaqIngestionSubSettingscontains a typo.- public IrisCombinedFaqIngestionSubSettingsDTO combinceFaqIngestionSubSettings(ArrayList<IrisSettings> settingsList, boolean minimal) { + public IrisCombinedFaqIngestionSubSettingsDTO combineFaqIngestionSubSettings(ArrayList<IrisSettings> settingsList, boolean minimal) {src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.ts (2)
99-99: Consider using optional chaining for nested property access.The nested property access could be simplified using optional chaining to improve readability and reduce the chance of runtime errors.
-this.autoFaqIngestion = this.irisSettings?.irisFaqIngestionSettings?.autoIngestOnFaqCreation ?? false; +this.autoFaqIngestion = this.irisSettings?.irisFaqIngestionSettings?.autoIngestOnFaqCreation ?? false;
139-141: Consider using optional chaining for nested property access.Similar to the previous suggestion, the nested property access in the save method could be simplified.
-if (this.irisSettings && this.irisSettings.irisFaqIngestionSettings) { - this.irisSettings.irisFaqIngestionSettings.autoIngestOnFaqCreation = this.autoFaqIngestion; -} +if (this.irisSettings?.irisFaqIngestionSettings) { + this.irisSettings.irisFaqIngestionSettings.autoIngestOnFaqCreation = this.autoFaqIngestion; +}🧰 Tools
🪛 Biome (1.9.4)
[error] 139-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20241217122200_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (48)
src/main/java/de/tum/cit/aet/artemis/communication/service/FaqService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java(9 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisCourseSettings.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisExerciseSettings.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisGlobalSettings.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSettings.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSubSettings.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSubSettingsType.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedFaqIngestionSubSettingsDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedSettingsDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/repository/IrisSettingsRepository.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisStatusUpdateService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqIngestionStatusUpdateDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqWebhookDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisWebhookFaqDeletionExecutionDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisWebhookFaqIngestionExecutionDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/FaqIngestionWebhookJob.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/LectureIngestionWebhookJob.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/iris/web/IrisResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/web/open/PublicPyrisStatusUpdateResource.java(2 hunks)src/main/webapp/app/entities/iris/settings/iris-settings.model.ts(4 hunks)src/main/webapp/app/entities/iris/settings/iris-sub-settings.model.ts(2 hunks)src/main/webapp/app/faq/faq.component.html(1 hunks)src/main/webapp/app/faq/faq.component.ts(6 hunks)src/main/webapp/app/faq/faq.service.ts(2 hunks)src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.html(1 hunks)src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.ts(4 hunks)src/main/webapp/i18n/de/faq.json(1 hunks)src/main/webapp/i18n/de/iris.json(2 hunks)src/main/webapp/i18n/en/faq.json(1 hunks)src/main/webapp/i18n/en/iris.json(2 hunks)src/test/java/de/tum/cit/aet/artemis/communication/FaqIntegrationTest.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/core/connector/IrisRequestMockProvider.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/iris/AbstractIrisIntegrationTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/iris/PyrisFaqIngestionTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/iris/PyrisLectureIngestionTest.java(2 hunks)src/test/javascript/spec/component/faq/faq.component.spec.ts(6 hunks)src/test/javascript/spec/component/iris/settings/iris-course-settings-update.component.spec.ts(2 hunks)src/test/javascript/spec/component/iris/settings/iris-global-settings-update.component.spec.ts(1 hunks)src/test/javascript/spec/component/lecture/lecture-detail.component.spec.ts(1 hunks)src/test/javascript/spec/service/faq.service.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (46)
src/test/java/de/tum/cit/aet/artemis/iris/AbstractIrisIntegrationTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSubSettingsType.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/iris/settings/iris-global-settings-update.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedFaqIngestionSubSettingsDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisWebhookFaqDeletionExecutionDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/repository/IrisSettingsRepository.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/lecture/lecture-detail.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/entities/iris/settings/iris-sub-settings.model.ts (1)
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqIngestionStatusUpdateDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/faq/faq.service.ts (1)
src/main/webapp/i18n/de/faq.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisExerciseSettings.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/communication/FaqIntegrationTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqWebhookDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/service/faq.service.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/LectureIngestionWebhookJob.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/entities/iris/settings/iris-settings.model.ts (1)
src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedSettingsDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/i18n/de/iris.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/test/javascript/spec/component/iris/settings/iris-course-settings-update.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisWebhookFaqIngestionExecutionDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSettings.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/faq/faq.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/test/java/de/tum/cit/aet/artemis/iris/PyrisFaqIngestionTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/FaqIngestionWebhookJob.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSubSettings.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/iris/PyrisLectureIngestionTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/test/java/de/tum/cit/aet/artemis/core/connector/IrisRequestMockProvider.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/iris/web/IrisResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisCourseSettings.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisStatusUpdateService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisGlobalSettings.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/faq/faq.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/faq/faq.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/iris/web/open/PublicPyrisStatusUpdateResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/communication/service/FaqService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🪛 Biome (1.9.4)
src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.ts
[error] 136-136: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 139-139: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: client-tests-selected
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Build and Push Docker Image
- GitHub Check: Analyse
🔇 Additional comments (41)
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisExerciseSettings.java (1)
93-103: Verify architectural decision for FAQ settings.The implementation correctly indicates that FAQ settings don't belong at the exercise level. This aligns with the PR objectives of enabling IRIS to answer with FAQs, as FAQs are typically managed at the course level.
Let's verify this architectural decision by checking the course settings implementation:
✅ Verification successful
Architectural decision for FAQ settings is correct ✅
The implementation in
IrisCourseSettingsconfirms that FAQ settings are properly managed at the course level, with complete JPA mappings and accessor methods. The exercise-level implementation returning null is the correct approach as FAQs are course-wide resources.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that FAQ settings are properly implemented at course level # Expected: Find IrisFaqIngestionSubSettings being used in IrisCourseSettings rg -l "IrisFaqIngestionSubSettings" --type javaLength of output: 712
Script:
#!/bin/bash # Check implementation of FAQ settings in IrisCourseSettings ast-grep --pattern 'class IrisCourseSettings { $$$ getIrisFaqIngestionSettings() { $$$ } $$$ }' # Also check the actual implementation file content cat src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisCourseSettings.javaLength of output: 4603
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/job/FaqIngestionWebhookJob.java (2)
1-5: LGTM! Clean imports and package structure.The package and import declarations follow the coding guidelines, using specific imports instead of wildcards.
10-21: Verify the canAccess implementations.Both
canAccessmethods returnfalse, which means this job can't access any courses or exercises. Given that this job is part of the FAQ system integration:
- Should it at least allow access to the course specified by
courseId?- If the restrictions are intentional, please add comments explaining why.
Let's check how other webhook jobs implement these methods:
✅ Verification successful
The canAccess implementations are correct
The
falsereturns are intentional. This follows the same pattern asLectureIngestionWebhookJob, where webhook-based ingestion jobs don't need course or exercise access as they only process incoming data.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other PyrisJob implementations to compare access patterns ast-grep --pattern 'class $_ implements PyrisJob { $$$ canAccess($_, $_) { $$$ } $$$ }'Length of output: 225
Script:
#!/bin/bash # First, find the PyrisJob interface rg "interface PyrisJob" -l # Then find implementations rg "implements PyrisJob" -A 10 # Also search for canAccess method patterns rg "canAccess\((Course|Exercise)" -A 3Length of output: 10002
src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedFaqIngestionSubSettingsDTO.java (1)
1-7: LGTM!The new record
IrisCombinedFaqIngestionSubSettingsDTOis well-defined and aligns with the use of Java Records for DTOs. The@JsonInclude(JsonInclude.Include.NON_EMPTY)annotation is appropriately used to control JSON serialization.src/main/webapp/app/entities/iris/settings/iris-settings.model.ts (1)
25-25: LGTM!The addition of
irisFaqIngestionSettingsproperty follows the existing pattern consistently across all relevant settings classes.Also applies to: 36-36, 48-48
src/main/java/de/tum/cit/aet/artemis/iris/repository/IrisSettingsRepository.java (1)
48-49: LGTM! The query modification follows repository best practices.The addition of
LEFT JOIN FETCH irisSettings.irisFaqIngestionSettingsis consistent with the existing query structure and properly fetches the FAQ ingestion settings alongside other settings.src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisSubSettings.java (1)
45-46: LGTM! The JSON subtype configuration is properly structured.The addition of
IrisFaqIngestionSubSettingsto@JsonSubTypesfollows the established pattern and naming convention.src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisGlobalSettings.java (1)
38-40: LGTM! Field declaration follows established patterns.The JPA annotations and field declaration are consistent with other settings in the class, maintaining a clean and uniform approach to settings management.
src/test/javascript/spec/component/iris/settings/iris-course-settings-update.component.spec.ts (1)
65-65: LGTM! Test coverage properly updated.The test modifications correctly reflect the addition of FAQ ingestion settings:
- Updated element count expectation from 5 to 6
- Added assertion to verify FAQ ingestion settings initialization
Also applies to: 98-98
src/test/java/de/tum/cit/aet/artemis/iris/AbstractIrisIntegrationTest.java (1)
66-66: LGTM! Consistent implementation of FAQ ingestion settings activation.The changes follow the established pattern of activating Iris sub-settings, maintaining code consistency.
Also applies to: 89-89
src/test/javascript/spec/component/lecture/lecture-detail.component.spec.ts (1)
Line range hint
109-115: LGTM! Well-structured error handling test.The test case properly validates error handling by:
- Mocking the error scenario
- Verifying error logging
- Using specific jest matchers
src/main/webapp/app/faq/faq.service.ts (1)
163-165: LGTM! Well-documented API integration.The method follows Angular service patterns and properly implements the FAQ ingestion endpoint.
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisStatusUpdateService.java (2)
16-16: LGTM!The imports are correctly added and follow the coding guidelines.
Also applies to: 23-23
136-138: LGTM!The method follows the established pattern for status update handlers and correctly delegates to
removeJobIfTerminatedElseUpdate.src/test/java/de/tum/cit/aet/artemis/communication/FaqIntegrationTest.java (1)
32-33: LGTM! Dependency injection follows best practices.The
IrisRequestMockProvideris properly injected using constructor injection.src/test/javascript/spec/component/faq/faq.component.spec.ts (2)
23-28: LGTM! Clean import organization and test setup.The new imports and setup code are well-organized and follow the established patterns in the codebase.
Also applies to: 49-50, 66-69
264-279: LGTM! Comprehensive settings validation test.The test case thoroughly validates the FAQ ingestion settings and profile configuration.
src/test/java/de/tum/cit/aet/artemis/core/connector/IrisRequestMockProvider.java (2)
175-182: LGTM! Well-structured mock implementation.The mock implementation follows the established pattern and correctly handles the webhook response.
193-200: LGTM! Consistent deletion webhook mock implementation.The deletion webhook mock follows the same pattern as the ingestion mock, maintaining consistency.
src/test/java/de/tum/cit/aet/artemis/iris/PyrisFaqIngestionTest.java (1)
75-87: LGTM! Comprehensive auto-ingestion test cases.The test cases thoroughly verify both enabled and disabled auto-ingestion scenarios.
Also applies to: 89-97
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (1)
248-251: LGTM! Clear settings validation.The settings validation logic is clear and consistent with the lecture ingestion pattern.
src/test/java/de/tum/cit/aet/artemis/iris/PyrisLectureIngestionTest.java (1)
280-280: LGTM! Method rename improves clarity.The renaming of
addIngestionWebhookJobtoaddLectureIngestionWebhookJobbetter reflects its specific purpose and aligns with the introduction of FAQ ingestion functionality.Also applies to: 298-298
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java (2)
23-23: LGTM! Imports are properly organized.The new imports for FAQ ingestion settings are appropriately placed and follow the existing import organization pattern.
Also applies to: 32-32
200-229: LGTM! FAQ ingestion settings implementation is consistent.The implementation of
updatefor FAQ ingestion settings follows the established pattern and includes proper authorization checks. The method correctly handles:
- Null settings validation
- Settings initialization
- Admin-only updates for course and global settings
src/main/webapp/i18n/en/faq.json (1)
11-12: LGTM! Clear and consistent label text.The new label "Send FAQs to Iris" is clear, concise, and follows the existing labeling pattern.
src/main/webapp/i18n/de/faq.json (1)
11-12: LGTM! Translation follows guidelines.The German translation "FAQs an IRIS schicken" correctly uses informal style (dutzen) as required by the coding guidelines.
src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.html (1)
75-93: LGTM! Implementation follows best practices.The FAQ ingestion settings section:
- Correctly uses new Angular control flow syntax (@if)
- Maintains consistent structure with other settings sections
- Includes proper accessibility attributes
- Has correct model bindings
src/main/webapp/app/faq/faq.component.html (1)
43-43: LGTM! Button implementation follows best practices.The FAQ ingestion button:
- Uses new Angular control flow syntax (@if)
- Maintains consistent styling with other buttons
- Has proper accessibility attributes
- Uses appropriate layout utilities for alignment
Also applies to: 48-53
src/main/webapp/app/iris/settings/iris-settings-update/iris-settings-update.component.ts (2)
15-15: LGTM! Property declaration follows Angular guidelines.The new property
autoFaqIngestionis correctly declared with proper type and initialization.Also applies to: 46-46
129-131: LGTM! Proper initialization of FAQ ingestion settings.The initialization of FAQ ingestion settings follows the established pattern and is correctly implemented.
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java (2)
25-26: LGTM! Clean import organization.The imports are properly organized and specific to the required job types.
96-109: LGTM! Well-documented method with proper implementation.The lecture ingestion webhook job method is well-documented and correctly implemented.
src/main/webapp/app/faq/faq.component.ts (3)
3-3: LGTM! Clean import organization.The imports are properly organized and follow Angular style guidelines.
Also applies to: 22-24
42-43: LGTM! Property declarations follow Angular guidelines.The new properties are correctly declared with proper types and initialization.
95-102: LGTM! Proper subscription and initialization handling.The profile info subscription and settings initialization are correctly implemented.
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java (6)
38-38: LGTM! Clean import organization.The import for FAQ ingestion settings is properly organized.
165-169: LGTM! Proper initialization of FAQ ingestion settings.The initialization method follows the established pattern and is correctly implemented.
277-282: LGTM! Consistent settings update handling.The FAQ ingestion settings are properly updated in both global and course settings.
Also applies to: 327-332
608-609: LGTM! Proper settings combination.The FAQ ingestion settings are correctly combined with other settings in all relevant methods.
Also applies to: 635-636, 663-664
695-695: LGTM! Proper default settings initialization.The FAQ ingestion settings are correctly initialized in the default settings.
773-773: LGTM! Feature enablement check properly implemented.The FAQ ingestion feature check is correctly added to the switch statement.
src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/communication/service/FaqService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisCourseSettings.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (1)
315-321: 🛠️ Refactor suggestionAdd null check for
toUpdateFaqsparameterThe method
executeFaqDeletionWebhookshould validate the input parametertoUpdateFaqsto prevent a potentialNullPointerException.Apply this diff to add a null check:
private String executeFaqDeletionWebhook(PyrisFaqWebhookDTO toUpdateFaqs) { + if (toUpdateFaqs == null) { + throw new IllegalArgumentException("FAQ data cannot be null"); + } String jobToken = pyrisJobService.addFaqIngestionWebhookJob(0, 0); PyrisPipelineExecutionSettingsDTO settingsDTO = new PyrisPipelineExecutionSettingsDTO(jobToken, List.of(), artemisBaseUrl); PyrisWebhookFaqDeletionExecutionDTO executionDTO = new PyrisWebhookFaqDeletionExecutionDTO(toUpdateFaqs, settingsDTO, List.of()); pyrisConnectorService.executeFaqDeletionWebhook(executionDTO); return jobToken; }
🧹 Nitpick comments (10)
src/test/javascript/spec/component/faq/faq.component.spec.ts (3)
66-69: Consider using a more complete mock profile.The mock profile info object could be enhanced to better represent real-world scenarios.
const profileInfo = { activeProfiles: [], + ribbonEnv: 'dev', + inProduction: false, + version: 'test' } as unknown as ProfileInfo;
256-262: Enhance error test specificity.The error test could be more specific about the expected error message.
alertServiceStub = jest.spyOn(alertService, 'error'); faqComponent.faqs = [faq1]; -jest.spyOn(faqService, 'ingestFaqsInPyris').mockReturnValue(throwError(() => new Error('Error while ingesting'))); +const errorMessage = 'Error while ingesting FAQs'; +jest.spyOn(faqService, 'ingestFaqsInPyris').mockReturnValue(throwError(() => new Error(errorMessage))); faqComponent.ingestFaqsInPyris(); -expect(alertServiceStub).toHaveBeenCalledOnce(); +expect(alertServiceStub).toHaveBeenCalledExactlyOnceWith('artemis.faq.error.ingest');
264-279: Improve test isolation and readability.The settings test could be improved by:
- Moving mock responses to constants
- Adding negative test cases
+const MOCK_PROFILE_INFO: ProfileInfo = { + activeProfiles: [PROFILE_IRIS], +} as ProfileInfo; + +const MOCK_IRIS_SETTINGS: IrisCourseSettings = { + irisFaqIngestionSettings: { + enabled: true, + }, +} as IrisCourseSettings; + it('should set faqIngestionEnabled based on service response', () => { faqComponent.faqs = [faq1]; - const profileInfoResponse = { - activeProfiles: [PROFILE_IRIS], - } as ProfileInfo; - const irisSettingsResponse = { - irisFaqIngestionSettings: { - enabled: true, - }, - } as IrisCourseSettings; - jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(profileInfoResponse)); - jest.spyOn(irisSettingsService, 'getCombinedCourseSettings').mockImplementation(() => of(irisSettingsResponse)); + jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(MOCK_PROFILE_INFO)); + jest.spyOn(irisSettingsService, 'getCombinedCourseSettings').mockImplementation(() => of(MOCK_IRIS_SETTINGS)); faqComponent.ngOnInit(); expect(irisSettingsService.getCombinedCourseSettings).toHaveBeenCalledWith(faqComponent.courseId); expect(faqComponent.faqIngestionEnabled).toBeTrue(); }); + +it('should disable FAQ ingestion when settings are disabled', () => { + faqComponent.faqs = [faq1]; + const disabledSettings = { ...MOCK_IRIS_SETTINGS }; + disabledSettings.irisFaqIngestionSettings.enabled = false; + jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(MOCK_PROFILE_INFO)); + jest.spyOn(irisSettingsService, 'getCombinedCourseSettings').mockImplementation(() => of(disabledSettings)); + faqComponent.ngOnInit(); + expect(faqComponent.faqIngestionEnabled).toBeFalse(); +});src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java (2)
103-110: Consider making timeout values configurable.The timeout duration and unit are hardcoded. Consider moving these values to configuration properties for better maintainability and flexibility.
+ @Value("${artemis.iris.webhook.timeout.duration:60}") + private long webhookTimeoutDuration; + + @Value("${artemis.iris.webhook.timeout.unit:MINUTES}") + private TimeUnit webhookTimeoutUnit; public String addLectureIngestionWebhookJob(long courseId, long lectureId, long lectureUnitId) { var token = generateJobIdToken(); var job = new LectureIngestionWebhookJob(token, courseId, lectureId, lectureUnitId); - long timeoutWebhookJob = 60; - TimeUnit unitWebhookJob = TimeUnit.MINUTES; - jobMap.put(token, job, timeoutWebhookJob, unitWebhookJob); + jobMap.put(token, job, webhookTimeoutDuration, webhookTimeoutUnit); return token; }
Line range hint
119-125: Extract common timeout handling logic.The timeout handling logic is duplicated between
addLectureIngestionWebhookJobandaddFaqIngestionWebhookJob. Consider extracting this into a private helper method.+ private void putJobWithTimeout(String token, PyrisJob job) { + jobMap.put(token, job, webhookTimeoutDuration, webhookTimeoutUnit); + } public String addFaqIngestionWebhookJob(long courseId, long faqId) { var token = generateJobIdToken(); var job = new FaqIngestionWebhookJob(token, courseId, faqId); - long timeoutWebhookJob = 60; - TimeUnit unitWebhookJob = TimeUnit.MINUTES; - jobMap.put(token, job, timeoutWebhookJob, unitWebhookJob); + putJobWithTimeout(token, job); return token; }src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java (2)
207-220: Simplify method signature by removing unused parameter.The
toUpdateFaqparameter is only used for logging. Consider removing it and extracting the FAQ ID from theexecutionDTOparameter instead.- public void executeFaqAdditionWebhook(PyrisFaqWebhookDTO toUpdateFaq, PyrisWebhookFaqIngestionExecutionDTO executionDTO) { + public void executeFaqAdditionWebhook(PyrisWebhookFaqIngestionExecutionDTO executionDTO) { var endpoint = "/api/v1/webhooks/faqs"; try { restTemplate.postForEntity(pyrisUrl + endpoint, objectMapper.valueToTree(executionDTO), Void.class); } catch (HttpStatusCodeException e) { - log.error("Failed to send faq {} to Pyris: {}", toUpdateFaq.faqId(), e.getMessage()); + log.error("Failed to send faq {} to Pyris: {}", executionDTO.pyrisFaq().faqId(), e.getMessage()); throw toIrisException(e); } catch (RestClientException | IllegalArgumentException e) { - log.error("Failed to send faq {} to Pyris: {}", toUpdateFaq.faqId(), e.getMessage()); + log.error("Failed to send faq {} to Pyris: {}", executionDTO.pyrisFaq().faqId(), e.getMessage()); throw new PyrisConnectorException("Could not fetch response from Pyris"); } }
227-240: Enhance log messages with specific FAQ details.The log messages are generic. Consider including the FAQ IDs from the execution DTO for better debugging.
public void executeFaqDeletionWebhook(PyrisWebhookFaqDeletionExecutionDTO executionDTO) { var endpoint = "/api/v1/webhooks/faqs/delete"; try { restTemplate.postForEntity(pyrisUrl + endpoint, objectMapper.valueToTree(executionDTO), Void.class); } catch (HttpStatusCodeException e) { - log.error("Failed to send faqs to Pyris", e); + log.error("Failed to delete FAQs {} from Pyris", executionDTO.faqIds(), e); throw toIrisException(e); } catch (RestClientException | IllegalArgumentException e) { - log.error("Failed to send faqs to Pyris", e); + log.error("Failed to delete FAQs {} from Pyris", executionDTO.faqIds(), e); throw new PyrisConnectorException("Could not fetch response from Pyris"); } }src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (2)
248-251: OptimizefaqIngestionEnabledmethod to avoid redundant callsIn the
faqIngestionEnabledmethod, you're callingirisSettingsService.getRawIrisSettingsFor(course)twice. This can be optimized by storing the result in a local variable to avoid redundant method calls.Apply this diff to optimize the method:
private boolean faqIngestionEnabled(Course course) { + var irisSettings = irisSettingsService.getRawIrisSettingsFor(course); - return irisSettingsService.getRawIrisSettingsFor(course).getIrisFaqIngestionSettings() != null - && irisSettingsService.getRawIrisSettingsFor(course).getIrisFaqIngestionSettings().isEnabled(); + return irisSettings.getIrisFaqIngestionSettings() != null + && irisSettings.getIrisFaqIngestionSettings().isEnabled(); }
254-261: Simplify null checks and avoid redundant repository callsIn the
autoUpdateFaqInPyrismethod, you're callingirisSettingsRepository.findCourseSettings(courseId)multiple times. Assign the result to a variable to prevent redundant calls and simplify the null checks.Apply this diff to optimize the code:
public void autoUpdateFaqInPyris(Long courseId, Faq newFaq) { - IrisCourseSettings courseSettings = irisSettingsRepository.findCourseSettings(courseId).isPresent() ? irisSettingsRepository.findCourseSettings(courseId).get() : null; + Optional<IrisCourseSettings> optionalCourseSettings = irisSettingsRepository.findCourseSettings(courseId); + IrisCourseSettings courseSettings = optionalCourseSettings.orElse(null); if (courseSettings != null && courseSettings.getIrisFaqIngestionSettings() != null && courseSettings.getIrisFaqIngestionSettings().isEnabled() && courseSettings.getIrisFaqIngestionSettings().getAutoIngestOnFaqCreation()) { addFaqToPyris(newFaq); } }src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java (1)
Line range hint
121-125: Remove redundant call tocheckIsInstructorForAcceptedFaqIn the
updateFaqmethod,checkIsInstructorForAcceptedFaq(faq.getFaqState(), courseId);is called twice consecutively with the same parameters, which is unnecessary.Apply this diff to remove the redundant call:
public ResponseEntity<FaqDTO> updateFaq(@RequestBody Faq faq, @PathVariable Long faqId, @PathVariable Long courseId) { log.debug("REST request to update Faq : {}", faq); if (faqId == null || !faqId.equals(faq.getId())) { throw new BadRequestAlertException("Id of FAQ and path must match", ENTITY_NAME, "idNull"); } checkIsInstructorForAcceptedFaq(faq.getFaqState(), courseId); Faq existingFaq = faqRepository.findByIdElseThrow(faqId); - checkIsInstructorForAcceptedFaq(faq.getFaqState(), courseId); if (!Objects.equals(existingFaq.getCourse().getId(), courseId)) { throw new BadRequestAlertException("Course ID of the FAQ provided courseID must match", ENTITY_NAME, "idNull"); } Faq updatedFaq = faqRepository.save(faq); if (checkIfFaqIsAccepted(updatedFaq)) { faqService.autoIngestFaqsIntoPyris(courseId, updatedFaq); } FaqDTO dto = new FaqDTO(updatedFaq); return ResponseEntity.ok().body(dto); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java(8 hunks)src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisStatusUpdateService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqWebhookDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java(4 hunks)src/test/javascript/spec/component/faq/faq.component.spec.ts(6 hunks)src/test/javascript/spec/service/faq.service.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/faqingestionwebhook/PyrisFaqWebhookDTO.java
- src/test/javascript/spec/service/faq.service.spec.ts
- src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisFaqIngestionSubSettings.java
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisStatusUpdateService.java
🧰 Additional context used
📓 Path-based instructions (7)
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisJobService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/faq/faq.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-tests-selected
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Build and Push Docker Image
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
🔇 Additional comments (13)
src/test/javascript/spec/component/faq/faq.component.spec.ts (3)
23-28: LGTM! Imports are well-organized.The new imports are correctly structured and follow Angular's best practices for importing services and models.
49-50: LGTM! Service declarations follow testing conventions.The service declarations are properly typed and follow Jest testing patterns.
248-254: LGTM! Ingestion test follows best practices.The test case properly verifies the service call and uses appropriate Jest matchers.
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java (1)
288-295: 🛠️ Refactor suggestionAdd null check for
toUpdateFaqparameterThe method
executeFaqAdditionWebhookshould validate the input parametertoUpdateFaqto prevent a potentialNullPointerException.Apply this diff to add a null check:
private String executeFaqAdditionWebhook(PyrisFaqWebhookDTO toUpdateFaq) { + if (toUpdateFaq == null) { + throw new IllegalArgumentException("FAQ data cannot be null"); + } String jobToken = pyrisJobService.addFaqIngestionWebhookJob(toUpdateFaq.courseId(), toUpdateFaq.faqId()); PyrisPipelineExecutionSettingsDTO settingsDTO = new PyrisPipelineExecutionSettingsDTO(jobToken, List.of(), artemisBaseUrl); PyrisWebhookFaqIngestionExecutionDTO executionDTO = new PyrisWebhookFaqIngestionExecutionDTO(toUpdateFaq, settingsDTO, List.of()); pyrisConnectorService.executeFaqAdditionWebhook(toUpdateFaq, executionDTO); return jobToken; }Likely invalid or redundant comment.
src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java (2)
211-229: Implementation ofupdatemethod forIrisFaqIngestionSubSettingslooks goodThe
updatemethod correctly handles null checks, updates properties based on user roles, and follows best practices for settings management.
379-382: Implementation ofcombineFaqIngestionSubSettingsmethod is appropriateThe method effectively combines the
enabledfield for FAQ ingestion settings, maintaining consistency with the existing pattern for combining sub-settings.src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java (7)
38-38: LGTM!The import is correctly placed and follows the established pattern for domain settings imports.
178-182: LGTM!The initialization method follows the established pattern and properly handles the FAQ ingestion settings initialization.
290-295: LGTM!The FAQ ingestion settings update is properly integrated into the global settings update process, following the established pattern.
340-345: LGTM!The FAQ ingestion settings update is properly integrated into the course settings update process, maintaining proper inheritance from parent settings.
621-622: LGTM!The FAQ ingestion settings are properly integrated into the combined global settings DTO construction.
648-649: LGTM!The FAQ ingestion settings are properly integrated into the course-level combined settings DTO construction, with appropriate handling of minimal settings for student access.
786-786: LGTM!The FAQ ingestion feature check is properly integrated into the settings verification logic.
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisWebhookService.java
Show resolved
Hide resolved
|
|
||
| @Profile(PROFILE_CORE) | ||
| @Service | ||
| public class FaqService { |
There was a problem hiding this comment.
currently, this service only methods using the PyrisWebhookService. Would it make sense to use PROFILE_IRIS (used for both iris and pyris)?
Might not make sense if you plan to extend this service with functionality that does not rely on pyris.
| } | ||
|
|
||
| public void deleteFaqInPyris(Faq existingFaq) { | ||
| if (!profileService.isIrisActive()) { |
There was a problem hiding this comment.
why do you have this additional check here, but not for the other methods?
| @@ -212,6 +226,24 @@ public ResponseEntity<Set<String>> getFaqCategoriesForCourseByState(@PathVariabl | |||
| return ResponseEntity.ok().body(faqs); | |||
| } | |||
|
|
|||
| /** | |||
| * POST /courses/{courseId}/ingest | |||
There was a problem hiding this comment.
url seems to be outdated (missing /faqs/
| @@ -113,6 +113,15 @@ public boolean isAeolusActive() { | |||
| return isProfileActive(Constants.PROFILE_AEOLUS); | |||
| } | |||
|
|
|||
| /** | |||
| * Checks if the aeolus profile is active | |||
There was a problem hiding this comment.
comment refers to aelous, but should pbl. be about iris
| } | ||
|
|
||
| /** | ||
| * Executes a webhook and send faqs to the webhook with the given variant |
There was a problem hiding this comment.
comment is the same as executeFaqAdditionWebhook. maybe you could be more specific here (e.g. "executeFaqAdditionWebhook adds faqs...", "executeFaqDeletionWebhook deletes faqs...").
| * @param faq The faq that got Updated | ||
| * @return jobToken if the job was created else null | ||
| */ | ||
| public String addFaqToPyris(Faq faq) { |
There was a problem hiding this comment.
just from reading the comments, it's not too easy to make out the difference between addFaqToPyris and autoUpdateFaqInPyris.
Little bit out of scope: I am aware that the JavaDoc regarding vector database in Pyris is used in multiple places. In my opinion, that should be irrelevant for Artemis since Artemis does not need to have any knowledge about Pyris apart from its interfaces (i.e. REST API).
| * @param faq The faqs that got Updated / erased | ||
| * @return jobToken if the job was created | ||
| */ | ||
| public String deleteFaqFromPyrisDB(Faq faq) { |
There was a problem hiding this comment.
for instance same here: why is it relevant for Artemis that it's stored in a vector DB? Analogous, you wouldn't have an REST-endpoint in Artemis called "deleteProgrammingExerciseFromDB" - rather "deleteProgrammingExercise".
| if (parentSettings == null) { | ||
| throw new IllegalArgumentException("Cannot delete the FAQ Ingestion settings"); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Why do we return null in this case? Should we maybe include that in the docs?
| @@ -8,7 +8,8 @@ | |||
| "accept": "FAQ akzeptieren", | |||
| "reject": "FAQ ablehnen", | |||
| "filterLabel": "Filter", | |||
| "createOrEditLabel": "FAQ erstellen oder bearbeiten" | |||
| "createOrEditLabel": "FAQ erstellen oder bearbeiten", | |||
| "ingestLabel": "FAQs an IRIS schicken" | |||
There was a problem hiding this comment.
If a user does not know too much about IRIS, this might not be too descriptive IMO. What benefits would I have of doing that? e.g. sth like "IRIS/chatbot will use the FAQs to answer questions more precisely..."
| Faq faq = new Faq(); | ||
| faq.setQuestionTitle("Lorem Ipsum"); | ||
| irisRequestMockProvider.mockFaqIngestionWebhookRunResponse(dto -> { | ||
| assertThat(dto.settings().authenticationToken()).isNotNull(); |
There was a problem hiding this comment.
i don't really see how this is testing what the method name suggests
Contains migration
Note
This PR depends on the counterpart in IRIS. For testing, deploy Pyris#191 via
deploy:pyris-testif it is not already deployed (lock:pyris-test). You can use TS1, TS3, TS5 for testing since it has Iris enabled.Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
In recent PRs (#9325) we implemented a FAQ system for Artemis. Students can manually lookup the FAQ via the Course, however IRIS is not aware of this FAQ and will not answer with the content of the FAQ, even if this information is accessible
Description
This PR adds a new ingestion pipeline to the IRIS subsystem and enables IRIS to recevie all the information of the FAQ system. Furthermore it enables IRIS to respond in the course chat with this information.
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the release notes:
Release Notes
New Features
Improvements
Bug Fixes
Technical Updates