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

reverse flipper: champva_unique_temp_file_names #19661

Merged

Conversation

cloudmagic80
Copy link
Contributor

@cloudmagic80 cloudmagic80 commented Dec 1, 2024

Summary

This PR removes the feature toggle related to champva_unique_temp_file_names .

Related issue(s)

#19143
#19170

Testing done

Tested on 7959f2 and 1010d with attachments.

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

@cloudmagic80 cloudmagic80 changed the title features yml reverse flipper: champva_unique_temp_file_names Dec 1, 2024
@va-vfs-bot va-vfs-bot temporarily deployed to 97124-reverse-flipper-champva-unique-temp-file-names/main/main December 1, 2024 20:41 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 97124-reverse-flipper-champva-unique-temp-file-names/main/main December 1, 2024 21:11 Inactive
@cloudmagic80 cloudmagic80 marked this pull request as ready for review December 1, 2024 22:15
@cloudmagic80 cloudmagic80 requested review from a team as code owners December 1, 2024 22:15
@cloudmagic80
Copy link
Contributor Author

1 Warning
⚠️ This PR changes 491 LoC (not counting whitespace/newlines).
In order to ensure each PR receives the proper attention it deserves, we recommend not exceeding 200. Expect some delays getting reviews.

File Summary
Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

This warning doesn't apply to this PR since this PR only reverse the fillper

Copy link

github-actions bot commented Dec 2, 2024

1 Warning
⚠️ This PR changes 491 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, we recommend not exceeding
200. Expect some delays getting reviews.

File Summary

Files

  • config/features.yml (+0/-3)

  • modules/ivc_champva/app/controllers/ivc_champva/v1/uploads_controller.rb (+10/-23)

  • modules/ivc_champva/app/services/ivc_champva/attachments.rb (+12/-33)

  • modules/ivc_champva/app/services/ivc_champva/pdf_filler.rb (+11/-21)

  • modules/ivc_champva/spec/models/vha_10_7959c_spec.rb (+5/-21)

  • modules/ivc_champva/spec/models/vha_10_7959f_1_spec.rb (+5/-21)

  • modules/ivc_champva/spec/models/vha_10_7959f_2_spec.rb (+5/-21)

  • modules/ivc_champva/spec/services/attachments_spec.rb (+32/-83)

  • modules/ivc_champva/spec/services/pdf_filler_spec.rb (+75/-110)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, *.md, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/, *.bru, *.pdf
    

Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

@cloudmagic80 cloudmagic80 merged commit e08c74f into master Dec 5, 2024
54 of 56 checks passed
@cloudmagic80 cloudmagic80 deleted the 97124-reverse-flipper-champva-unique-temp-file-names branch December 5, 2024 16:17
mchristiansonVA pushed a commit that referenced this pull request Dec 5, 2024
* features yml

* reverse fillper

* remove uuid toggle
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