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

CST - Added db migration to update evidence_submissions fields from type string to integer #20490

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

pmclaren19
Copy link
Contributor

@pmclaren19 pmclaren19 commented Jan 28, 2025

Summary

This pr is related to the work from #20105. As part of that work we need to update the request_id, claim_id and tracked_item_id to be integers instead of strings. Currently the table evidence_submissions is not being used and is behind the feature flag cst_send_evidence_submission_failure_emails which is currently disabled in upper environments.

This pr is currently blocking the pr here.

Related issue(s)

-Link to ticket created in va.gov-team repo department-of-veterans-affairs/va.gov-team#100037

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?

What areas of the site does it impact?

Claim Status Tool

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?

@pmclaren19 pmclaren19 self-assigned this Jan 28, 2025
@va-vfs-bot va-vfs-bot temporarily deployed to 100037_Update_Evidence_submission_field_data_types/main/main January 28, 2025 03:32 Inactive
@pmclaren19 pmclaren19 marked this pull request as ready for review January 28, 2025 14:13
@pmclaren19 pmclaren19 requested review from a team as code owners January 28, 2025 14:13
samcoforma
samcoforma previously approved these changes Jan 28, 2025
jerekshoe
jerekshoe previously approved these changes Jan 28, 2025
Copy link
Contributor

@stevenjcumming stevenjcumming left a comment

Choose a reason for hiding this comment

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

@jerekshoe
Copy link
Contributor

@pmclaren19 sorry I didn't catch this sooner, but there needs to be data migration for a column type change

https://depo-platform-documentation.scrollhelp.site/developer-docs/vets-api-database-migrations#vets-apidatabasemigrations-Changingacolumntype(🐉)

There isn't any data in the table currently, is there a need to worry about data migrations in that case?

@pmclaren19 pmclaren19 dismissed stale reviews from samcoforma and jerekshoe via d009624 January 29, 2025 19:53
@pmclaren19 pmclaren19 merged commit 61f867e into master Jan 29, 2025
26 checks passed
@pmclaren19 pmclaren19 deleted the 100037_Update_Evidence_submission_field_data_types branch January 29, 2025 20:04
gabezurita added a commit that referenced this pull request Jan 29, 2025
…-removal

* master:
  CST - Added db migration to update evidence_submissions fields from type string to integer (#20490)
  changes sign_in_state_code ttl to 60 minutes (#20488)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants