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

Add 0969 section 4 pdf mapping #19940

Merged
merged 8 commits into from
Jan 14, 2025
Merged

Add 0969 section 4 pdf mapping #19940

merged 8 commits into from
Jan 14, 2025

Conversation

TaiWilkin
Copy link
Contributor

@TaiWilkin TaiWilkin commented Dec 17, 2024 •
edited
Loading

Summary

  • Adds PDF mapping for Associated Incomes in 21P-0969 (form currently unreleased)

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Manually call the PDF filler with sample data, and confirm the form is filled correctly

What areas of the site does it impact?

Income and Assets (0969)

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 (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

@va-vfs-bot va-vfs-bot temporarily deployed to income-pdf-financial/main/main December 17, 2024 23:28 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to income-pdf-financial/main/main January 6, 2025 21:49 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to income-pdf-financial/main/main January 6, 2025 22:57 Inactive
ToddWebDev
ToddWebDev previously approved these changes Jan 7, 2025
View reviewed changes
@TaiWilkin TaiWilkin marked this pull request as ready for review January 7, 2025 15:20
@TaiWilkin TaiWilkin requested review from a team as code owners January 7, 2025 15:20
@TaiWilkin TaiWilkin enabled auto-merge (squash) January 7, 2025 15:21
balexandr
balexandr previously approved these changes Jan 7, 2025
View reviewed changes
@va-vfs-bot va-vfs-bot temporarily deployed to income-pdf-financial/main/main January 7, 2025 15:35 Inactive
LindseySaari
LindseySaari reviewed Jan 8, 2025
View reviewed changes
Gemfile.lock Outdated
@@ -74,10 +74,10 @@ GIT

GIT
remote: https://github.com/department-of-veterans-affairs/vets-json-schema
revision: a84abd360072a0d4aaa8b1984cee2c33e1844335
revision: 143e7d682372ffee0fd9de0b3b246403d0a61cb4
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you'll want to rebase here. The Vets Json Schema has been since updated

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 just merged in master to ensure I'm moving from the correct revision. Is that sufficient, or am I missing another change? (I see that a vets_json_schema 24.7.3 was created this morning, but I'm not certain we're ready to update to that.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the vets_json_schema version 24.7.3 necessary for this PR?

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, just up to 24.7.2

Copy link
Contributor

Choose a reason for hiding this comment

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

@LindseySaari - Can we move this along? I think Tai's PR is correct based upon the latest.

Copy link
Member

Choose a reason for hiding this comment

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

From the conversation in another pull request, the latest version will be upgraded in this pull request.

@TaiWilkin
Copy link
Contributor Author

@mjknight50

@TaiWilkin TaiWilkin mentioned this pull request Jan 14, 2025
Closed
9 tasks
@TaiWilkin TaiWilkin dismissed stale reviews from balexandr and ToddWebDev via bcb09bb January 14, 2025 17:07
@RachalCassity RachalCassity linked an issue Jan 14, 2025 that may be closed by this pull request
PCIU Migration: Step 6g: vets-website PCIU removal department-of-veterans-affairs/va.gov-team#99359
Closed
6 tasks
@RachalCassity RachalCassity mentioned this pull request Jan 14, 2025
Merged
@TaiWilkin TaiWilkin merged commit 5f3fb09 into master Jan 14, 2025
22 checks passed
@TaiWilkin TaiWilkin deleted the income-pdf-financial branch January 14, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LindseySaari LindseySaari LindseySaari left review comments

@mjknight50 mjknight50 mjknight50 left review comments

@RachalCassity RachalCassity RachalCassity approved these changes

@ToddWebDev ToddWebDev ToddWebDev left review comments

@balexandr balexandr balexandr left review comments

Assignees
No one assigned
Labels
require-backend-approval test-passing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PCIU Migration: Step 6g: vets-website PCIU removal
7 participants
@TaiWilkin @balexandr @mjknight50 @LindseySaari @ToddWebDev @RachalCassity @va-vfs-bot