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

[GYR1-636] Reenable pending spec tests in f13614c_pdf_spec.rb #5642

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

spompea-cfa
Copy link
Contributor

@spompea-cfa spompea-cfa commented Feb 26, 2025

Link to Jira issue

Is PM acceptance required?

  • No - merge after code review approval

What was done?

Reënabled four spec tests that got turned off while we updated code for the
ty2024 updates.

1) it "can successfully write everything that comes out of #hash_for_pdf to

the PDF"

Originally this test checked that the filler class's fields matched exactly the
fields in a generated PDF. But with our current approach (ty2024), the filler
class is not doing anything with fields that won't get filled in, so this test
won't quite work as-is. So, I updated the logic slightly to ensure that at least
the intersection of fields between the two aforementioned sources is equal to
the generated pdf's fields (i.e., we're not trying to fill in any fields that
don't exist).

In the process, it was necessary to clear out all out-of-date field mappings
that still lingered in the filler class.

2) it "fills out answers from the DB into the pdf"

Replaced all the old PDF field keys with keys from ty2024 pdf.

3) testing gated questions/values

We don't use the 'gated' field mechanism anymore.

  • I removed this spec test.
  • I also removed the fetch_gated_value from the filler class.
  • Note: I left the GATES data structure in place at the top fo the filler
    class. Why? It's still used by VitaMinFormBuilderFor13614 for some very
    bespoke logic around setting default values in maybe one or a handful of form
    elements in the Hub editable 14-C (otherwise, I would've deleted GATES too).

4) it "15080 fields are nil" do

Small changes were needed to update this. Note that I made the related updates
in the filler class's vita_consent_to_disclose_info method too; however, to
the best of my knowledge, this is not currently used.

How to test?

  • Run the spec tests :O

Copy link

Heroku app: https://gyr-review-app-5642-1037555a0fba.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5642 (optionally add --tail)

@spompea-cfa spompea-cfa force-pushed the gyr1-636-reenable-pending-spec-tests-in-f13614c_pdf_spec branch from a56da90 to e0f604a Compare February 28, 2025 03:36
@spompea-cfa spompea-cfa changed the title [wip] [GYR1-636] Reenable pending spec tests in f13614c_pdf_spec.rb [GYR1-636] Reenable pending spec tests in f13614c_pdf_spec.rb Feb 28, 2025
@@ -141,26 +138,6 @@ def hash_for_pdf

answers["form1[0].page2[0].receivedMoneyFrom[0].howManyJobs[0]"] = @intake.job_count.to_s

# PAGE 2: INCOME
answers.merge!(
yes_no_checkboxes("form1[0].page2[0].Part3[0].q3Scholarships[0]", @intake.had_scholarships, include_unsure: true),
Copy link
Contributor

Choose a reason for hiding this comment

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

why were these removed? Are they outdated from previous years 13614c?

Copy link
Contributor

@embarnard embarnard left a comment

Choose a reason for hiding this comment

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

This was a lot of work to add all these new lines into the spec! Good job, approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants