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

CSV previews are sometimes broken - refactoring of CSV previews controller #3952

Merged
merged 2 commits into from
Feb 12, 2024
Merged

CSV previews are sometimes broken - refactoring of CSV previews controller #3952

merged 2 commits into from
Feb 12, 2024

Conversation

unoduetre
Copy link
Contributor

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

This is a refactoring related to the previous work on CSV previews: #3945

Why

Trello card

@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 1, 2024 16:08 Inactive
@unoduetre unoduetre marked this pull request as ready for review February 1, 2024 16:09
@unoduetre unoduetre requested a review from hannako February 6, 2024 15:37
hannako
hannako reviewed Feb 6, 2024
View reviewed changes
Copy link
Contributor

@hannako hannako left a comment

Choose a reason for hiding this comment

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

Thanks for doing this Mat, I think this is a great improvement. I've made a couple of notes, mainly around the spec.

app/services/csv_preview_service.rb Outdated Show resolved Hide resolved
test/unit/services/csv_preview_service_test.rb Outdated Show resolved Hide resolved
@unoduetre unoduetre force-pushed the 2328-csv-previews-are-sometimes-broken-l-2 branch from c0a74be to 9df60b3 Compare February 8, 2024 16:05
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 8, 2024 16:05 Inactive
@unoduetre unoduetre force-pushed the 2328-csv-previews-are-sometimes-broken-l-2 branch from 9df60b3 to 10ef3a4 Compare February 8, 2024 16:11
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 8, 2024 16:11 Inactive
@unoduetre unoduetre requested a review from hannako February 8, 2024 16:18
@unoduetre unoduetre force-pushed the 2328-csv-previews-are-sometimes-broken-l-2 branch from 10ef3a4 to 9c0fdcf Compare February 8, 2024 16:20
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 8, 2024 16:20 Inactive
@unoduetre unoduetre force-pushed the 2328-csv-previews-are-sometimes-broken-l-2 branch from 9c0fdcf to d168590 Compare February 12, 2024 10:04
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 12, 2024 10:04 Inactive
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 12, 2024 10:23 Inactive
@unoduetre unoduetre force-pushed the 2328-csv-previews-are-sometimes-broken-l-2 branch from 1ae6ff5 to 980e8a0 Compare February 12, 2024 10:30
@govuk-ci govuk-ci temporarily deployed to govuk-frontend-app-pr-3952 February 12, 2024 10:30 Inactive
hannako
hannako approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@hannako hannako left a comment

Choose a reason for hiding this comment

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

Looks great, thanks Mat

@unoduetre unoduetre merged commit 85b9172 into main Feb 12, 2024
13 checks passed
@unoduetre unoduetre deleted the 2328-csv-previews-are-sometimes-broken-l-2 branch February 12, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannako hannako hannako approved these changes

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
@unoduetre @hannako @govuk-ci