-
Notifications
You must be signed in to change notification settings - Fork 0
bug/matching-refactor-csv-check #163
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
Conversation
WalkthroughThe changes update the file validation logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RestAdapter
Client->>RestAdapter: Send file upload request
RestAdapter->>RestAdapter: Check if filename ends with CSV_EXTENSION
RestAdapter->>RestAdapter: Validate content type against CSV_CONTENT_TYPES
RestAdapter-->>Client: Return success or error response
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 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: 1
🧹 Nitpick comments (1)
matching-service/src/main/java/de/muenchen/oss/swim/matching/adapter/in/rest/RestAdapter.java (1)
77-79: Improve error message specificity.The error message "File needs to be a csv" could be more specific about what validation failed (extension or content type).
Suggested improvement:
- throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "File needs to be a csv"); + final String errorDetail = file.getOriginalFilename() == null ? "Missing filename" : + !file.getOriginalFilename().endsWith(CSV_EXTENSION) ? "File extension must be .csv" : + "Invalid content type. Expected one of: " + CSV_CONTENT_TYPES; + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Invalid file format: " + errorDetail);This provides more context about which validation rule failed, helping users correct their input.
🧰 Tools
🪛 GitHub Actions: build all
[error] 77-77: Medium: Possible null pointer dereference in de.muenchen.oss.swim.matching.adapter.in.rest.RestAdapter.update(MultipartFile) due to return value of called method.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
matching-service/src/main/java/de/muenchen/oss/swim/matching/adapter/in/rest/RestAdapter.java(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: build all
matching-service/src/main/java/de/muenchen/oss/swim/matching/adapter/in/rest/RestAdapter.java
[error] 77-77: Medium: Possible null pointer dereference in de.muenchen.oss.swim.matching.adapter.in.rest.RestAdapter.update(MultipartFile) due to return value of called method.
🔇 Additional comments (1)
matching-service/src/main/java/de/muenchen/oss/swim/matching/adapter/in/rest/RestAdapter.java (1)
36-37: Good addition of constants for file validation.Defining constants for CSV extension and content types improves code maintainability and makes the validation more explicit. Including
application/vnd.ms-excelas an acceptable content type allows handling Excel-generated CSV files as required.
Description
Matching refactor csv check. Allow excel csv and check extension.
Summary by CodeRabbit