-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Iris
: Fix message rating
#9653
Iris
: Fix message rating
#9653
Conversation
WalkthroughThe changes involve a modification to the Changes
Suggested labels
Suggested reviewers
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/iris/iris-chat-http.service.ts (1)
98-98
: Consider adding error handling documentation.While the implementation is correct, it would be helpful to document the expected error scenarios (e.g., invalid message ID, session not found) to guide component-level error handling.
Add JSDoc to describe error scenarios:
/** * Sets a helpfulness rating for a message * @param {number} sessionId of the session of the message that should be rated * @param {number} messageId of the message that should be rated * @param {boolean} helpful rating of the message * @return {Observable<EntityResponseType>} an Observable of the HTTP responses + * @throws {HttpErrorResponse} 404 if message or session not found + * @throws {HttpErrorResponse} 403 if user not authorized to rate message */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/webapp/app/iris/iris-chat-http.service.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/iris/iris-chat-http.service.ts (1)
🔇 Additional comments (1)
src/main/webapp/app/iris/iris-chat-http.service.ts (1)
98-98
: LGTM! The implementation follows Angular best practices.
The change correctly moves the helpful
parameter from URL path to request body, which is more RESTful and aligns with conventional practices for PUT requests.
Let's verify the REST controller endpoint compatibility:
✅ Verification successful
The frontend service implementation correctly matches the backend endpoint
The REST controller endpoint in IrisMessageResource.java
accepts the helpful
parameter as a request body (@RequestBody Boolean helpful
), which perfectly aligns with the HTTP service implementation that sends the parameter in the request body using httpClient.put()
. The endpoint URL structure /sessions/${sessionId}/messages/${messageId}/helpful
also matches exactly between frontend and backend.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the REST controller endpoint matches the new implementation
# Expected: Controller should accept the boolean value in request body
# Search for the corresponding REST controller endpoint
rg -t java -A 10 "helpful.*IrisMessage" src/main/java
Length of output: 55
Script:
#!/bin/bash
# Let's try different search patterns to locate the REST controller endpoint
# Search for controller methods handling message rating
rg -t java "PUT.*messages/.*helpful" src/main/java
# Search for controller class handling Iris messages
rg -t java "class.*Controller.*Iris" src/main/java
# Search for helpful-related method signatures
rg -t java "@PutMapping.*helpful" src/main/java
Length of output: 477
Script:
#!/bin/bash
# Let's examine the implementation of the helpful endpoint in IrisMessageResource.java
rg -A 10 "@PutMapping.*helpful" src/main/java/de/tum/cit/aet/artemis/iris/web/IrisMessageResource.java
Length of output: 877
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes make sense imo, having the value in the body looks better than again pushing it into the path 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally. Rating messages now works as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code changes make sense
Checklist
General
Motivation and Context
This PR fixes the problem with rating Iris messages; the messages couldn't be rated anymore, and Iris showed an error.
Description
As there was an update with the REST controller and the IrisChatHttpService was not updated, a simple change was needed to resolve this problem. The change now sends the value via the body as a raw value.
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
Client
Summary by CodeRabbit
New Features
rateMessage
functionality by sending thehelpful
parameter as part of the request body, enhancing the structure of HTTP requests.Bug Fixes
Documentation