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

Modified letters generator route to be POST and updated tests #12794

Merged
merged 0 commits into from
Jun 6, 2023
Merged

Modified letters generator route to be POST and updated tests #12794

merged 0 commits into from
Jun 6, 2023

Conversation

samcoforma
Copy link
Contributor

@samcoforma samcoforma commented May 24, 2023 •
edited
Loading

Summary

  • Updated route and related RSpec test to accept POST requests
  • Fixes bug for Benefit Summary downloads with options

Related issue(s)

Testing done

  • RSpec tests updated and passing

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

@samcoforma samcoforma requested a review from kjduensing May 24, 2023 15:36
@va-vfs-bot va-vfs-bot temporarily deployed to letters-generator-benefit-summary-fix/main/main May 24, 2023 15:47 Inactive
kjduensing
kjduensing previously approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@kjduensing kjduensing left a comment

Choose a reason for hiding this comment

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

LGTM

@samcoforma samcoforma requested a review from kjduensing May 30, 2023 16:20
@va-vsp-bot va-vsp-bot requested a deployment to letters-generator-benefit-summary-fix/main/main May 30, 2023 16:20 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to letters-generator-benefit-summary-fix/main/main May 30, 2023 16:25 Inactive
kjduensing
kjduensing previously approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@kjduensing kjduensing left a comment

Choose a reason for hiding this comment

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

LGTM

@va-vsp-bot va-vsp-bot requested a deployment to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:16 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:17 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:20 In progress
@va-vsp-bot va-vsp-bot requested a deployment to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:22 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:27 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:35 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:36 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:37 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to letters-generator-benefit-summary-fix/main/main June 5, 2023 12:38 Inactive
@samcoforma
Copy link
Contributor Author

@kjduensing I had to modify the vcr cassettes to make sure that false options were being passed to lighthouse. Could you re-approve this PR?

@samcoforma samcoforma requested a review from kjduensing June 5, 2023 12:43
@samcoforma samcoforma marked this pull request as ready for review June 5, 2023 12:43
@samcoforma samcoforma requested review from a team as code owners June 5, 2023 12:43
kjduensing
kjduensing approved these changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@kjduensing kjduensing left a comment

Choose a reason for hiding this comment

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

This looks like a nice PR.

ryan-mcneil
ryan-mcneil approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@ryan-mcneil ryan-mcneil left a comment

Choose a reason for hiding this comment

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

👌

@samcoforma samcoforma merged commit 9bf5b9b into master Jun 6, 2023
@samcoforma samcoforma deleted the letters-generator-benefit-summary-fix branch June 6, 2023 16:06
ryan-mcneil pushed a commit that referenced this pull request Dec 11, 2023
@samcoforma
Modified letters generator route to be POST and updated tests (#12794)
77555b3 
* modified letters generator route to be POST and updated tests

* updated letters generator download endpoint to work with POST requests and Benefits Summary options

* restored boolean casting and selection in letters generator controller

* fixing option filtering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryan-mcneil ryan-mcneil ryan-mcneil approved these changes

@kjduensing kjduensing kjduensing approved these changes

Assignees
No one assigned
Labels
console-services-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@samcoforma @kjduensing @ryan-mcneil @va-vfs-bot @va-vsp-bot