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

[SIMPLE_FORMS] feat: updates to submission builder in accordance with new direction #18517

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

pennja
Copy link
Contributor

@pennja pennja commented Sep 19, 2024

Summary

  • This work updates the submission archive builder in accordance with VBA direction

Related issue(s)

Testing done

  • New code is covered by unit tests

Requested Feedback

Any

def submission_pdf_filename
@submission_pdf_filename ||= "form_#{form_data_hash['form_number']}.pdf"
form_number = form_data_hash['form_number']
@submission_pdf_filename ||= "form_#{form_number}_vagov_#{benefits_intake_uuid}.pdf"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using underscores here given that both the form_number and benefits_intake_uuid will contain dashes.

@@ -29,7 +29,7 @@ def handle_error(message, error, **)
end

def temp_directory_path
@temp_directory_path ||= Rails.root.join("tmp/#{benefits_intake_uuid}-#{SecureRandom.hex}/").to_s
@temp_directory_path ||= Rails.root.join("tmp/#{benefits_intake_uuid}-#{SecureRandom.hex}-archive/").to_s
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ensuring there are no collisions with similar temp folder/file logic elsewhere.

Thrillberg
Thrillberg previously approved these changes Sep 19, 2024
Copy link
Contributor

@Thrillberg Thrillberg left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Glad to see the options get simplified. 😄

@ryan-mcneil
Copy link
Contributor

@pennja it's probably worth merging master after the changes we made

Copy link

Backend-review-group approval confirmed.

@pennja pennja merged commit 9655e0b into master Sep 24, 2024
23 checks passed
@pennja pennja deleted the jap/simple-forms/1633-submission-archive-tweaks branch September 24, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants