Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add byte order mark to talpa export for better excel support #2455

Merged
merged 1 commit into from
Nov 16, 2023
Merged

feat: add byte order mark to talpa export for better excel support #2455

merged 1 commit into from
Nov 16, 2023

Conversation

rikuke
Copy link
Collaborator

@rikuke rikuke commented Nov 14, 2023

Description ✨

Talpa csv export works better in Windows excel, when a byte order mark (BOM) is appended to the beginning of the file.

Issues 🐛

Testing ⚗️

Screenshots 📸

Additional notes 🗒️

@rikuke rikuke requested review from sirtawast and mjturt November 14, 2023 11:32
@terovirtanen
Copy link
Contributor

API branch is deployed to platta: https://helsinkilisa-pr2455.api.dev.hel.ninja/healthz 🚀🚀🚀

@terovirtanen
Copy link
Contributor

APPLICANT branch is deployed to platta: https://helsinkilisa-ui-pr2455.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://helsinkilisa-ui-pr2455.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr2455.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://helsinkilisa-ui-handler-pr2455.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

API branch is deployed to platta: https://helsinkilisa-pr2455.api.dev.hel.ninja/healthz 🚀🚀🚀

@terovirtanen
Copy link
Contributor

APPLICANT branch is deployed to platta: https://helsinkilisa-ui-pr2455.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://helsinkilisa-ui-pr2455.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr2455.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://helsinkilisa-ui-handler-pr2455.dev.hel.ninja 😆🎉🎉🎉

@sonarqubecloud SonarQubeCloud
Copy link

[yjdh] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarqubecloud SonarQubeCloud
Copy link

[yjdh] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 142 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarqubecloud SonarQubeCloud
Copy link

[yjdh] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 304 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarqubecloud SonarQubeCloud
Copy link

[yjdh] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

76.9% 76.9% Coverage
0.0% 0.0% Duplication

@terovirtanen
Copy link
Contributor

API branch is deployed to platta: https://helsinkilisa-pr2455.api.dev.hel.ninja/healthz 🚀🚀🚀

@terovirtanen
Copy link
Contributor

APPLICANT branch is deployed to platta: https://helsinkilisa-ui-pr2455.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://helsinkilisa-ui-pr2455.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr2455.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://helsinkilisa-ui-handler-pr2455.dev.hel.ninja 😆🎉🎉🎉

mjturt
mjturt approved these changes Nov 15, 2023
View reviewed changes
backend/benefit/applications/api/v1/application_batch_views.py
@@ -190,7 +190,7 @@ def talpa_export_batch(self, request, *args, **kwargs) -> StreamingHttpResponse:
)

response = StreamingHttpResponse(
csv_service.get_csv_string_lines_generator(True),
csv_service.get_csv_string_lines_generator(True, True),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually call functions with named arguments, at least with more than one argument. It makes the code clearer but also bulkier, so I guess it's a matter of preference.

@rikuke rikuke merged commit 1d23a7a into main Nov 16, 2023
57 checks passed
@rikuke rikuke deleted the HL-1035-talpa-BOM branch November 16, 2023 06:51
@github-actions github-actions bot mentioned this pull request Nov 16, 2023
Merged
@github-actions github-actions bot mentioned this pull request Dec 8, 2023
Merged
@github-actions github-actions bot mentioned this pull request Dec 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjturt mjturt mjturt approved these changes

@sirtawast sirtawast Awaiting requested review from sirtawast

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rikuke @terovirtanen @mjturt