FYST-526 Update refund taxes owed UI and add financial information to MD pdf/xml #5007

arinchoi03 · 2024-11-19T22:41:09Z

Link to pivotal/JIRA issue

https://codeforamerica.atlassian.net/browse/FYST-526

Is PM acceptance required? (delete one)

Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

Removed bank_name from all intakes!
Deconstructed account_holder_name for MD, since the XML requires the names separated out
Added joint_account_holder_name fields for MD
Implemented follow up question component for joint account holder name (see thread for convo if curious)
MD xml/pdf information flow through from tax_refund/taxes_owed -- lot of the xml is already filled out via another ticket; see subtasks for specifics needed to be implemented in this ticket

How to test?

Describe the testing approach taken to verify the changes, including:
- Unit/integration/manual tests

Screenshots (for visual changes)

UI

XML

PDF

…ve bank_name

…es-owed-ui-and-add-financial-information-to-md-pdf-xml

github-actions · 2024-11-21T19:14:08Z

Heroku app: https://gyr-review-app-5007-cef51b37824c.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5007 (optionally add --tail)

… have one

…count holder names

arinchoi03 · 2024-11-22T00:08:06Z

persona specs failing & bit mystifying why that is
still need to implement validations that match with md schema for these name fields, i just copied over what was in the name dob form at the moment. For example, I think the last name allowing 137 characters is probably not ideal.
maybe think about implementing a feature spec where the client toggles on the joint account holder checkbox & fills out info
styling for the headers of the account holder names
some open questions (1, 2) for Product, if not getting answer by tomorrow, ping Tiffany
all the xml/pdf stuff hasn't even been started yet 😓

…es-owed-ui-and-add-financial-information-to-md-pdf-xml

… nil/unfilled if payment method changes to mail

arinchoi03 · 2024-11-25T21:36:19Z

app/forms/state_file/md_tax_refund_form.rb

+      validates :account_holder_first_name, presence: true
+      validates :account_holder_last_name, presence: true
+      validates :account_holder_first_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,15})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.first_name.invalid_format') }}
+      validates :account_holder_last_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,31})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.last_name.invalid_format') }}


ask Tiffany about following schema strictly? (allowing numbers in first/last name)

Tiffany Chang sorry to be super annoying again but on “implement any character-type validations that are present in the schema for this field” for people’s names –

Currently the first and last names have this validation ([A-Za-z0-9'-] ?)*[A-Za-z0-9'-] which means any characters (alphabet, numbers, apostrophes and dash, white space). Do we want the same validation? I’m talking specifically about numbers…do we want to allow numbers here?

https://codeforamerica.atlassian.net/browse/FYST-526?focusedCommentId=13466

answered!
https://codeforamerica.atlassian.net/browse/FYST-526?focusedCommentId=13486

Arin Choi great catch--let’s allow alphabet, apostrophes and dash, and white space only. no numbers

Co-authored-by: Jenny Heath <[email protected]>

…itInd implementation

jenny-heath · 2024-11-26T00:35:30Z

...rate/20241121222604_add_joint_account_holder_name_and_account_holder_deconstructed_fields.rb

@@ -0,0 +1,14 @@
+class AddJointAccountHolderNameAndAccountHolderDeconstructedFields < ActiveRecord::Migration[7.1]
+  def change
+    add_column :state_file_md_intakes, :has_joint_account_holder, :integer, default: 0


ack i'm so sorry but we also usually have null: false on the enum columns. forgot to mention in the last comment 😓

i should've caught that! ty

jenny-heath · 2024-11-26T00:37:23Z

spec/lib/efile/md/md502_calculator_spec.rb

+
+  describe "#calculate_authorize_direct_deposit" do
+    context "bank authorization was given" do
+      it "should return X" do


minor but it now returns true (same for "nil" below)

jenny-heath · 2024-11-26T00:38:24Z

spec/lib/submission_builder/ty2024/states/md/documents/md502_spec.rb

+
+    context "AuthToDirectDepositInd" do
+      before do
+        intake.bank_authorization_confirmed = "yes"


sorry if this is a ridiculous question but does this...work? i would usually
intake.update(bank_authorization_confirmed: "yes")

yeahh i'm not sure what's going on but
allow(intake).to receive(:bank_authorization_confirmed_yes?).and_return true and intake.bank_authorization_confirmed = "yes"

passes this spec, but fails with:
intake.bank_authorization_confirmed_yes!
intake.update(bank_authorization_confirmed: "yes")

Even when I put a binding.pry and try to manually update by using intake.update(bank_authorization_confirmed: "yes") it returns unfilled for the value after successful update
I'm super confused...I'm going to go with the mocking of the call bank_authorization_confirmed_yes? to be more stubbed for this spec

I needed to set the payment type in the test so that on save (which happens with update and _yes! methods), if the payment type is not direct_deposit, it will run sanitize_bank_details which was setting it back to "unfilled"

intake.payment_or_deposit_type = 'direct_deposit' intake.update(bank_authorization_confirmed: "yes")

gonna leave the test as-is but just for posterity...

jenny-heath · 2024-11-26T00:38:57Z

spec/lib/submission_builder/ty2024/states/md/documents/md502_spec.rb

+        intake.payment_or_deposit_type = "direct_deposit"
+        intake.account_holder_first_name = "Jack"
+        intake.account_holder_middle_initial = "D"
+        intake.account_holder_last_name = "Hansel"


same question. idk if i just completely missed this on the first review sorry

it works, it seems. intake.update() works too -- i can update this one

jenny-heath

looks pretty good! a couple of minor comments but i don't think they require re-review.

the only other thing, which can be a follow-up to this work because it's just adding ignored columns: will we lose historical data when we remove df_data_import_failed_at altogether? does anyone care?

arinchoi03 · 2024-11-26T04:07:02Z

@jenny-heath I think that's a great question about df_data_import_failed_at -- I did create a ticket in the story where I was attempting to ignore it https://codeforamerica.atlassian.net/browse/FYST-1173
We're not really dropping the column yet but I think we should definitely discuss if we should. I'll add a computer gardening topic to follow up on this & potentially create a story if we think we want to drop with note to alert DS :)

…to allow numbers (boooo)

arinchoi03 · 2024-11-26T18:01:53Z

app/forms/state_file/md_tax_refund_form.rb

@@ -46,8 +46,8 @@ class MdTaxRefundForm < QuestionsForm
      with_options if: -> { has_joint_account_holder == 'yes' } do
        validates :joint_account_holder_first_name, presence: true
        validates :joint_account_holder_last_name, presence: true
-        validates :joint_account_holder_first_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,15})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.first_name.invalid_format') }}
-        validates :joint_account_holder_last_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,137})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.last_name.invalid_format') }}


rubymine said {1} after the [a-zA-Z] was redundant, so I removed it

arinchoi03 · 2024-11-26T18:57:07Z

closing & reopening b/c migrations were changed

github-actions · 2024-11-26T19:03:35Z

Heroku app: https://gyr-review-app-5007-3938db5fac03.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5007 (optionally add --tail)

…es-owed-ui-and-add-financial-information-to-md-pdf-xml

arinchoi03 added the wip denotes a work in progress that isn't ready for formal review label Nov 19, 2024

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch 2 times, most recently from 1a839f1 to 661fa00 Compare November 19, 2024 22:55

Add joint account holder name & bank_authorization_confirmation, remo…

9ce7a6e

…ve bank_name

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch from 661fa00 to 9ce7a6e Compare November 21, 2024 17:13

Ensure joint_account_holder_name is set to nil if payment method changes

32d75ff

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch from 32f56e1 to 32d75ff Compare November 21, 2024 17:56

Merge remote-tracking branch 'origin' into FYST-526-update-refund-tax…

3166c36

…es-owed-ui-and-add-financial-information-to-md-pdf-xml

arinchoi03 added 5 commits November 21, 2024 14:32

Remove bank_name from taxes owed/tax_refund

f27a562

Reorganize db migration to separate name columns from the one new column

8e19444

Ignore bank_name for all models

c047de8

Conditionally show joint account holder name if client indicates they…

4dcf802

… have one

Update migrations to fit data model better and add validations for ac…

ad00864

…count holder names

arinchoi03 added 7 commits November 21, 2024 16:32

Address spec failures that reference bank_name

82e10b0

Make sure to annotate AZ

75f7eec

More spec failures oy

9f2d157

Address more spec failures

5e37860

Remove references to bank_name from persona factories

6ed6f77

Move persona factories out of 2023 folder

8d67efc

Add schema-specific validations for first and last name

2c0239a

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch from 07fffc4 to f0e3644 Compare November 22, 2024 18:49

Add suffix and middle initial validation/specs

09d242f

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch from f0e3644 to 09d242f Compare November 22, 2024 18:54

arinchoi03 added 5 commits November 22, 2024 10:59

Remove unnecessary blank suffix option

233092f

Use i18n translations for labels

fa21585

Style updates to be more in line with designs

526e07f

Merge remote-tracking branch 'origin' into FYST-526-update-refund-tax…

59268d4

…es-owed-ui-and-add-financial-information-to-md-pdf-xml

Make sure all name fields and bank_authorization_confirmed get set to…

d307903

… nil/unfilled if payment method changes to mail

arinchoi03 commented Nov 25, 2024

View reviewed changes

arinchoi03 and others added 6 commits November 25, 2024 14:54

Reference intake attribute directly from set_line

6ba181f

Co-authored-by: Jenny Heath <[email protected]>

Refactor ignored_columns to inherit and add to and not overwrite

b2219bd

Add more specs on account holder names on md502

8c565ba

Test bank_authorization_confirmed param

32a3033

Set default to 0 for has_joint_account_holder column

66c08cb

Add specs, address xml/pdf failure due to change in AuthToDirectDepos…

7fef8f3

…itInd implementation

jenny-heath reviewed Nov 26, 2024

View reviewed changes

jenny-heath approved these changes Nov 26, 2024

View reviewed changes

arinchoi03 added 2 commits November 25, 2024 19:32

Default null for has_joint_account_holder

0d68106

Rename expectations

4d82db6

arinchoi03 added 2 commits November 25, 2024 20:11

Refactor specs

1778dde

Cover the "no" scenario which shouldnt really occur

a8d228c

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch from bd8ad7f to 307d579 Compare November 26, 2024 17:59

Update md502 validations for first and last names of account holders …

8461769

…to allow numbers (boooo)

arinchoi03 force-pushed the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch from 307d579 to 8461769 Compare November 26, 2024 18:00

arinchoi03 commented Nov 26, 2024

View reviewed changes

arinchoi03 closed this Nov 26, 2024

arinchoi03 reopened this Nov 26, 2024

arinchoi03 added the in-acceptance label Nov 26, 2024

arinchoi03 added 2 commits November 27, 2024 13:40

Drop account_holder_name from StateFileMdIntakes

fd6e3a1

Merge remote-tracking branch 'origin' into FYST-526-update-refund-tax…

0f98c19

…es-owed-ui-and-add-financial-information-to-md-pdf-xml

arinchoi03 merged commit a074c6d into main Nov 27, 2024
7 checks passed

arinchoi03 deleted the FYST-526-update-refund-taxes-owed-ui-and-add-financial-information-to-md-pdf-xml branch November 27, 2024 22:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FYST-526 Update refund taxes owed UI and add financial information to MD pdf/xml #5007

FYST-526 Update refund taxes owed UI and add financial information to MD pdf/xml #5007

arinchoi03 commented Nov 19, 2024 •

edited by anisharamnani

Loading

github-actions bot commented Nov 21, 2024

arinchoi03 commented Nov 22, 2024 •

edited

Loading

arinchoi03 Nov 25, 2024 •

edited

Loading

arinchoi03 Nov 25, 2024

arinchoi03 Nov 26, 2024 •

edited

Loading

jenny-heath Nov 26, 2024

arinchoi03 Nov 26, 2024

jenny-heath Nov 26, 2024

jenny-heath Nov 26, 2024

arinchoi03 Nov 26, 2024 •

edited

Loading

arinchoi03 Nov 26, 2024 •

edited

Loading

jenny-heath Nov 26, 2024

arinchoi03 Nov 26, 2024 •

edited

Loading

jenny-heath left a comment

arinchoi03 commented Nov 26, 2024

arinchoi03 Nov 26, 2024

arinchoi03 commented Nov 26, 2024

github-actions bot commented Nov 26, 2024

FYST-526 Update refund taxes owed UI and add financial information to MD pdf/xml #5007

FYST-526 Update refund taxes owed UI and add financial information to MD pdf/xml #5007

Conversation

arinchoi03 commented Nov 19, 2024 • edited by anisharamnani Loading

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

What was done?

How to test?

Screenshots (for visual changes)

UI

XML

PDF

github-actions bot commented Nov 21, 2024

arinchoi03 commented Nov 22, 2024 • edited Loading

arinchoi03 Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

arinchoi03 Nov 25, 2024

Choose a reason for hiding this comment

arinchoi03 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

jenny-heath Nov 26, 2024

Choose a reason for hiding this comment

arinchoi03 Nov 26, 2024

Choose a reason for hiding this comment

jenny-heath Nov 26, 2024

Choose a reason for hiding this comment

jenny-heath Nov 26, 2024

Choose a reason for hiding this comment

arinchoi03 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

arinchoi03 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

jenny-heath Nov 26, 2024

Choose a reason for hiding this comment

arinchoi03 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

jenny-heath left a comment

Choose a reason for hiding this comment

arinchoi03 commented Nov 26, 2024

arinchoi03 Nov 26, 2024

Choose a reason for hiding this comment

arinchoi03 commented Nov 26, 2024

github-actions bot commented Nov 26, 2024

arinchoi03 commented Nov 19, 2024 •

edited by anisharamnani

Loading

arinchoi03 commented Nov 22, 2024 •

edited

Loading

arinchoi03 Nov 25, 2024 •

edited

Loading

arinchoi03 Nov 26, 2024 •

edited

Loading

arinchoi03 Nov 26, 2024 •

edited

Loading

arinchoi03 Nov 26, 2024 •

edited

Loading

arinchoi03 Nov 26, 2024 •

edited

Loading