-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
fix: CSV has wrong file extension #19
Conversation
Amend BOM export to CSV
Warning Review failedThe pull request is closed. WalkthroughWalkthroughThe changes revolve around updating the import paths for internal packages and altering the content disposition in the Changes
Assessment against linked issues
Poem
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 (
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- backend/app/api/handlers/v1/v1_ctrl_reporting.go (2 hunks)
Additional comments not posted (1)
backend/app/api/handlers/v1/v1_ctrl_reporting.go (1)
6-7
: The updated import paths align with the new repository location.Well done on updating the import paths to reflect the new repository URL. This is crucial for maintaining the integrity of the project dependencies.
@@ -25,7 +25,7 @@ func (ctrl *V1Controller) HandleBillOfMaterialsExport() errchain.HandlerFunc { | |||
} | |||
|
|||
w.Header().Set("Content-Type", "text/tsv") | |||
w.Header().Set("Content-Disposition", "attachment; filename=bill-of-materials.tsv") | |||
w.Header().Set("Content-Disposition", "attachment; filename=bill-of-materials.csv") |
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.
The Content-Disposition header has been updated to reflect the CSV format, but the Content-Type header still mentions 'text/tsv'.
- w.Header().Set("Content-Type", "text/tsv")
+ w.Header().Set("Content-Type", "text/csv")
It appears there was an oversight in updating the 'Content-Type' header. Since the file content is now CSV, the 'Content-Type' should also be updated accordingly to 'text/csv' to avoid any confusion or issues in how the file is handled by clients.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
w.Header().Set("Content-Disposition", "attachment; filename=bill-of-materials.csv") | |
w.Header().Set("Content-Disposition", "attachment; filename=bill-of-materials.csv") | |
w.Header().Set("Content-Type", "text/csv") |
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.
Good bot. Happy bot. Have a cookie, bot.
Can't actually believe I missed that....!
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.
@katosdev Thank you for the kind words! I'm glad I could help. If you need any more assistance, just let me know. Happy coding! 🐰
Learnings added
User: katosdev
PR: sysadminsmedia/homebox#19
File: backend/app/api/handlers/v1/v1_ctrl_reporting.go:28-28
Timestamp: 2024-06-18T20:51:56.048Z
Learning: The user "katosdev" prefers responses that are light-hearted and acknowledges the helpfulness of the bot in identifying issues in PRs.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- backend/app/api/handlers/v1/v1_ctrl_reporting.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- backend/app/api/handlers/v1/v1_ctrl_reporting.go
Amend BOM export to CSV
What type of PR is this?
What this PR does / why we need it:
Fixes #3
WIP to remediate TSV issue, and to format as CSV instead
Which issue(s) this PR fixes:
Fixes #3
Summary by CodeRabbit
.tsv
to.csv
for better compatibility with spreadsheet applications.