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

API-26273-526-v2-sec-6-serv-info-pdf-gen #12748

Merged
merged 0 commits into from
May 31, 2023
Merged

API-26273-526-v2-sec-6-serv-info-pdf-gen #12748

merged 0 commits into from
May 31, 2023

Conversation

stiehlrod
Copy link
Contributor

@stiehlrod stiehlrod commented May 19, 2023 •
edited
Loading

Summary

Maps the service information properties to the 526 PDF Generator (Column C, 526EZ PDF Generator Mapping on Schema Planning Doc).

Related issue(s)

Testing done

  • rspec
  • Postman

Screenshots

Note: Optional

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

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

@stiehlrod stiehlrod added Lighthouse lighthouse claimsApi modules/claims_api labels May 19, 2023
@stiehlrod stiehlrod self-assigned this May 19, 2023
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 19, 2023 20:05 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 19, 2023 20:25 Inactive
@stiehlrod stiehlrod changed the title WIP - adds serv info to pdf mapping API-26273-526-v2-sec-6-serv-info-pdf-gen May 22, 2023
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 14:18 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 14:18 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 15:11 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 15:12 Inactive
@stiehlrod stiehlrod marked this pull request as ready for review May 23, 2023 15:17
@stiehlrod stiehlrod requested review from a team as code owners May 23, 2023 15:17
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 15:18 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 15:18 Inactive
mchristiansonVA
mchristiansonVA reviewed May 23, 2023
View reviewed changes
modules/claims_api/lib/claims_api/v2/disability_compensation_pdf_mapper.rb Outdated
@pdf_data
end

def symbolize_ser_info
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Can we just spell out "service" instead or "ser" in method name?

modules/claims_api/lib/claims_api/v2/disability_compensation_pdf_mapper.rb Outdated
@pdf_data
end

def most_recent_ser_per
Copy link
Contributor

Choose a reason for hiding this comment

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

Also nitpick: Can we spell out "service_period" instead of "ser_per" in method name?

@stiehlrod stiehlrod marked this pull request as draft May 23, 2023 19:57
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 22:47 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 23, 2023 22:47 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 24, 2023 12:59 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 24, 2023 13:00 Inactive
@stiehlrod stiehlrod marked this pull request as ready for review May 24, 2023 13:51
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 24, 2023 13:51 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 24, 2023 13:52 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 24, 2023 13:53 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 24, 2023 13:53 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 25, 2023 15:20 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 25, 2023 15:26 Inactive
rockwellwindsor-va
rockwellwindsor-va reviewed May 25, 2023
View reviewed changes
modules/claims_api/lib/claims_api/v2/disability_compensation_pdf_mapper.rb
si[:prisonerOfWarConfinement][:confinementDates][:endDate] = end_date
si[:confinedAsPrisonerOfWar] = true if start
si
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not totally sure this would handle multiple confinements. When I run something through it like
"confinements": [ { "approximateBeginDate": "2021-11-06", "approximateEndDate": "2023-12-09" }, { "approximateBeginDate": "2016-11-06", "approximateEndDate": "2017-12-09" } ],

the last one just overrides the first. Obviously totally possible I misunderstand what the output needs to be exactly so just let me know if that is the case but I thought it was worth at least asking about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rockwellwindsor-va good catch, thanks! Pushing now.

@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 26, 2023 15:04 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 26, 2023 15:04 Inactive
rockwellwindsor-va
rockwellwindsor-va approved these changes May 26, 2023
View reviewed changes
Copy link
Contributor

@rockwellwindsor-va rockwellwindsor-va left a comment

Choose a reason for hiding this comment

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

Looks good to me!

rockwellwindsor-va
rockwellwindsor-va previously approved these changes May 26, 2023
View reviewed changes
Copy link
Contributor

@rockwellwindsor-va rockwellwindsor-va left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me! (twice !)

@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 30, 2023 13:50 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 30, 2023 13:50 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 30, 2023 14:54 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 30, 2023 14:54 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 30, 2023 14:54 In progress
@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 31, 2023 14:26 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 31, 2023 14:30 Inactive
rockwellwindsor-va
rockwellwindsor-va approved these changes May 31, 2023
View reviewed changes
Copy link
Contributor

@rockwellwindsor-va rockwellwindsor-va left a comment

Choose a reason for hiding this comment

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

Pulled it down just now, ran RSpec, everything still looks good to me on this.

@va-vsp-bot va-vsp-bot requested a deployment to API-26273-526-v2-sec-6-serv-info-pdf-gen/main/main May 31, 2023 14:42 In progress
@stiehlrod stiehlrod merged commit c589b29 into master May 31, 2023
@stiehlrod stiehlrod deleted the API-26273-526-v2-sec-6-serv-info-pdf-gen branch May 31, 2023 14:54
ryan-mcneil pushed a commit that referenced this pull request Dec 11, 2023
@stiehlrod
API-26273-526-v2-sec-6-serv-info-pdf-gen (#12748)
b23884f 
* WIP - adds serv info to pdf mapping

* Adds service info mapping

* Changes schema; removes confinement from confinements

* foramtted schema

* rubocop

* Adds unitAddress and confinement changes back to schema and form_526

* Adds back the unitAddress to request and example.json

* Changes confinement dates to be more realistic

* Addresses PR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mchristiansonVA mchristiansonVA mchristiansonVA left review comments

@rockwellwindsor-va rockwellwindsor-va rockwellwindsor-va approved these changes

Assignees

@stiehlrod stiehlrod

Labels
claimsApi modules/claims_api Lighthouse lighthouse
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stiehlrod @mchristiansonVA @rockwellwindsor-va @va-vfs-bot @va-vsp-bot