PBP 103350 Migrations for new form submissions and attempts tables

[danlim715]

Summary

• This work is behind a feature toggle (flipper): YES
• Added some new tables to track form submissions and attempts to BPDS and Lighthouse systems, that's detached from the simple forms API. Here is the ADR flow for proposed tables:
  
• Pension and Burial Programs Team to own this work

Related issue(s)

• Link to ticket created in va.gov-team repo

Testing done

• New code is covered by unit tests
• Describe what the old behavior was prior to the change
• Describe the steps required to verify your changes are working as expected. Exclusively stating 'Specs run' is NOT acceptable as appropriate testing
• If this work is behind a flipper:
  • Tests need to be written for both the flipper on and flipper off scenarios. Docs.
  • What is the testing plan for rolling out the feature?

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

Requested Feedback

(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

[balexandr]

@danlim715 The first thing that jumps out to me is that both of these sets of tables have identical, or almost identical, columns. Could we not just have two tables and add a service column that populates with either LightHouse or BPDS? And in the attempts table have both bpds_id and benefits_intake_uuid and it's only populated for the service that's being used?

[danlim715]

@danlim715 The first thing that jumps out to me is that both of these sets of tables have identical, or almost identical, columns. Could we not just have two tables and add a service column that populates with either LightHouse or BPDS? And in the attempts table have both bpds_id and benefits_intake_uuid and it's only populated for the service that's being used?

@balexandr I agree with the excessively DRY columns, but Wayne and Matt K specifically said they wanted to avoid STI at all costs cuz they didn't want null columns, especially cuz they foresee having to expand the tables with more service-specific attributes in the future. This is why I looked into delegated types and heavily considered implementing it into this setup since we would be able to eliminate the DRY columns but still allow for service-specific fields. But we discussed and concluded that using delegated types might be overkill for our use case, since the list of submission service types won't be growing anytime soon and we won't even leverage all the benefits of STI i.e. pagination, since we're not displaying this data. Plus, the setup and associations required for delegated types would make things more confusing and harder to maintain. But if you also want to advocate for usage of delegated types, then I'm with you.

[balexandr]

@danlim715 The first thing that jumps out to me is that both of these sets of tables have identical, or almost identical, columns. Could we not just have two tables and add a service column that populates with either LightHouse or BPDS? And in the attempts table have both bpds_id and benefits_intake_uuid and it's only populated for the service that's being used?

@balexandr I agree with the excessively DRY columns, but Wayne and Matt K specifically said they wanted to avoid STI at all costs cuz they didn't want null columns, especially cuz they foresee having to expand the tables with more service-specific attributes in the future. This is why I looked into delegated types and heavily considered implementing it into this setup since we would be able to eliminate the DRY columns but still allow for service-specific fields. But we discussed and concluded that using delegated types might be overkill for our use case, since the list of submission service types won't be growing anytime soon and we won't even leverage all the benefits of STI i.e. pagination, since we're not displaying this data. Plus, the setup and associations required for delegated types would make things more confusing and harder to maintain. But if you also want to advocate for usage of delegated types, then I'm with you.

Ahh okay I guess it;s safer this way and easier to query if need be especially if it plans to grow in different directions.

[TaiWilkin]

It looks like SavedClaim has stuff for the encrypted columns on the model and the table. Do we need that here?

[wayne-weibel]

The 'other' file danger is complaining about is the bpds readme

[mjknight50]

Please hold off on merging this.

[sanjabaj2]

Can we resolve conflicts and push it for approval?

[mjknight50]

GTG

[mjknight50]

Even better! GTG

[LindseySaari]

Danger is failing due to the README updates going out with the migration changes (see danger failure)

[rmtolmach]

updates look good ✅