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.

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

Snagging fixes - CSV upload #75

Merged
merged 5 commits into from
Aug 15, 2023
Merged

Snagging fixes - CSV upload #75

merged 5 commits into from
Aug 15, 2023

Conversation

steventux
Copy link
Contributor

@steventux steventux commented Aug 2, 2023

Context

Design review of CSV upload has suggested some amendments and fixes.

Changes proposed in this pull request

Upload form amendments

image

Success message

image

Amended error for form submission with no file selected

image

Error message for non-CSV upload

image

Guidance to review

Link to Trello card

https://trello.com/c/VGKXwQu0/116-snagging-csv-upload

Checklist

  • Attach to Trello card
  • Rebased main
  • Cleaned commit history
  • Tested by running locally

@steventux steventux added the deploy Deploys a review app label Aug 2, 2023
@steventux
Copy link
Contributor Author

Snagging - CSV upload

@github-actions github-actions bot temporarily deployed to review-75 August 2, 2023 15:29 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 7, 2023 14:31 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 7, 2023 15:21 Destroyed
@steventux steventux marked this pull request as ready for review August 7, 2023 15:33
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 08:23 Destroyed
@steventux steventux added deploy Deploys a review app and removed deploy Deploys a review app labels Aug 8, 2023
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 08:40 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 08:42 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 09:06 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 09:38 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 09:41 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 13:26 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 8, 2023 19:47 Destroyed
Copy link
Contributor

@AbigailMcP AbigailMcP left a comment

Choose a reason for hiding this comment

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

Looking good!

One thing which I realise now wasn't clear on the ticket - rather than the flash message we intended to have a separate 'green panel' success page. Similar to this page the user sees after submitting feedback on refer:

Screenshot 2023-08-10 at 11 57 55

I'm not sure about the content underneath the panel though, one to check with @adamsilver / @johndoates

<h1 class="govuk-heading-l">Check the Children’s Barred List</h1>

<h2 class="govuk-heading-m">Upload file</h2>

<%= form_with model: @upload_form, url: [:support_interface, :uploads] do |f| %>
<%= f.govuk_error_summary %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<%= f.govuk_error_summary %>
<%= f.govuk_error_summary("There’s a problem") %>

@steventux steventux force-pushed the 116-snagging-csv-upload branch 2 times, most recently from cb633a5 to e338b1a Compare August 14, 2023 16:43
@github-actions github-actions bot temporarily deployed to review-75 August 14, 2023 16:44 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 14, 2023 16:57 Destroyed
@github-actions github-actions bot temporarily deployed to review-75 August 14, 2023 17:13 Destroyed
@@ -0,0 +1 @@
626d146b0e45c9c4d740f5f7963eb5ba
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, added by accident - this isn't a key we ever use in any environment, was generated locally, I've removed now.

@github-actions github-actions bot temporarily deployed to review-75 August 14, 2023 18:31 Destroyed
@steventux steventux added deploy Deploys a review app and removed deploy Deploys a review app labels Aug 15, 2023
@github-actions github-actions bot temporarily deployed to review-75 August 15, 2023 08:37 Destroyed
@steventux steventux merged commit 529d1e3 into main Aug 15, 2023
13 of 14 checks passed
@steventux steventux deleted the 116-snagging-csv-upload branch August 15, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploys a review app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants