Iris: Redesign and refactor Iris UI
#8579
Conversation
github-actions
bot
added
tests
server
WalkthroughThe recent updates primarily focus on renaming and refactoring classes related to the Iris chat sessions, transitioning from Changes
This table provides a concise overview of the changes across the system, highlighting the shift towards a more specialized tutor chat session management and enhanced user data handling. Tip Early Access Features
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Out of diff range and nitpick comments (4)
src/main/webapp/app/iris/iris-chat-base.service.ts (3)
31-31: Document the constructor parameters to enhance code readability and maintainability.
198-208: ThegetCurrentSessionOrCreatemethod throws a generic error. Use a more descriptive error message or handle specific cases differently.
214-224: Similar togetCurrentSessionOrCreate, thecreateNewSessionmethod should provide more detailed error messages for better debugging and user feedback.src/main/webapp/app/iris/base-chatbot/iris-base-chatbot.component.ts (1)
188-209: TheresendMessagemethod should provide feedback to the user when a message cannot be resent due to missing ID or content.
| overflow: hidden; | ||
| transition: all 0.2s; | ||
|
|
||
| max-height: 0; |
There was a problem hiding this comment.
Remove the unexpected empty line before the declaration to maintain consistency and adhere to CSS best practices.
- Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| max-height: 0; | |
| max-height: 0; |
| min-height: 0; | ||
| padding: 0 5px; | ||
|
|
||
| display: flex; |
There was a problem hiding this comment.
Remove the unexpected empty line before the declaration to maintain consistency and adhere to CSS best practices.
- Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| display: flex; | |
| display: flex; |
| justify-content: center; | ||
| width: 100%; | ||
|
|
||
| .part { |
There was a problem hiding this comment.
Reorder CSS selectors to maintain specificity and prevent potential styling conflicts.
- .part {
+ .chat-status-bar .progress-bar .part {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| .part { | |
| .chat-status-bar .progress-bar .part { |
| background-color: var(--success); | ||
| transform: scaleX(1); | ||
| } | ||
| &.in_progress { |
There was a problem hiding this comment.
Add an empty line before the rule for better readability and to adhere to CSS best practices.
+
&.done {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| &.in_progress { | |
| &.in_progress { | |
| &.done { |
| background-color: var(--warning); | ||
| transition: transform 10s linear; | ||
| } | ||
| &.skipped { |
There was a problem hiding this comment.
Add an empty line before the rule for better readability and to adhere to CSS best practices.
+
&.skipped {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| &.skipped { | |
| &.skipped { |
| @@ -129,8 +129,8 @@ public void checkRateLimit(IrisSession session, User user) { | |||
| */ | |||
| @SuppressWarnings("unchecked") | |||
| private <S extends IrisSession> IrisSubFeatureWrapper<S> getIrisSessionSubService(S session) { | |||
| if (session instanceof IrisChatSession chatSession) { | |||
| return (IrisSubFeatureWrapper<S>) new IrisSubFeatureWrapper<>(irisChatSessionService, chatSession); | |||
| if (session instanceof IrisTutorChatSession chatSession) { | |||
There was a problem hiding this comment.
Improve the exception message for clarity when an unknown Iris session type is encountered.
- throw new BadRequestException("Unknown Iris session type " + session.getClass().getSimpleName());
+ throw new BadRequestException("The provided Iris session type '" + session.getClass().getSimpleName() + "' is not supported.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (session instanceof IrisTutorChatSession chatSession) { | |
| if (session instanceof IrisTutorChatSession chatSession) { |
| if (chatSession.getExercise().isExamExercise()) { | ||
| throw new ConflictException("Iris is not supported for exam exercises", "Iris", "irisExamExercise"); | ||
| } | ||
| // TODO support more exercise types |
There was a problem hiding this comment.
Consider opening a GitHub issue to track the TODO for supporting more exercise types in requestAndHandleResponse.
Would you like me to open a GitHub issue for this task?
| private route: ActivatedRoute, | ||
| ) {} | ||
|
|
||
| ngOnInit() { |
There was a problem hiding this comment.
Ensure proper error handling for parseInt in case of invalid exerciseId values.
- const exerciseId = parseInt(params['exerciseId'], 10);
+ const exerciseId = parseInt(params['exerciseId'], 10);
+ if (isNaN(exerciseId)) {
+ throw new Error('Invalid exercise ID');
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ngOnInit() { | |
| ngOnInit() { | |
| const exerciseId = parseInt(params['exerciseId'], 10); | |
| if (isNaN(exerciseId)) { | |
| throw new Error('Invalid exercise ID'); | |
| } |
| WHERE s.exercise.id = :exerciseId | ||
| AND s.user.id = :userId | ||
| ORDER BY s.creationDate DESC | ||
| LIMIT 1 |
There was a problem hiding this comment.
The LIMIT clause is not supported in JPQL; consider revising the query to fetch only the latest record correctly.
- LIMIT 1
+ // JPQL does not support LIMIT. Consider using a different approach to fetch only the latest record.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| LIMIT 1 | |
| // JPQL does not support LIMIT. Consider using a different approach to fetch only the latest record. |
| * Checks the availability of Iris by sending a heartbeat request. | ||
| */ | ||
| private checkHeartbeat(): void { | ||
| if (this.disconnected) return; |
There was a problem hiding this comment.
Add error handling for the getIrisStatus HTTP request to manage potential failures gracefully.
+ .catch((error) => {
+ console.error('Failed to fetch Iris status:', error);
+ this.active = false;
+ this.activeSubject.next(this.active);
+ });Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (this.disconnected) return; | |
| if (this.disconnected) return; | |
| .catch((error) => { | |
| console.error('Failed to fetch Iris status:', error); | |
| this.active = false; | |
| this.activeSubject.next(this.active); | |
| }); |
…e/iris/client-refactor
github-actions
bot
added
the
database
- Merged `IrisCourseChatService` and `IrisExerciseChatService` into a single service: `IrisChatService` - Added new enum `ChatServiceMode` to handle different chat modes
…temis into feature/iris/client-refactor
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Pyris V2 now submits status updates that we want to display in the UI.
It did also not look really pretty.
Additionally, the previous implementation was overengineered.
Description
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Test Coverage
Screenshots
CleanShot.2024-05-12.at.16.52.23.mp4