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

[DBEX] log when Form 0781 document upload fails #17625

Merged
merged 3 commits into from
Jul 26, 2024

Conversation

freeheeling
Copy link
Contributor

Summary

Objective: Implements a system for periodically polling the Lighthouse uploads/status endpoint to confirm the status of supplemental documents submitted to Lighthouse as part of a Form 526 Submission. The purpose of this system is to surface successes and failures that happen in the Form 526 process after being forwarded to Lighthouse for processing.

  • Add logging for form 0781 type documents that are reported by Lighthouse to have failed upload (includes test)
  • Add form0781_types? instance method to Lighthouse526DocumentUpload (includes test)
  • Refactor update_document_status helper method in UpdateDocumentsStatusService
  • This work is behind a feature toggle (flipper): YES/NO
  • Responsible team: Disability Benefits Experience Team 2 (dBeX Carbs 🥖)

Related issue(s)

Testing done

  • New code is covered by unit tests

What areas of the site does it impact?

  • non-user facing, asynchronous job tracking the status of uploaded documents submitted in conjunction with Form 526EZ

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

@freeheeling freeheeling added disability-experience To manage benefits disability claims experience. disability-compensation Label used for Pull Requests that impact Disability Compensation claims (526EZ) labels Jul 19, 2024
@va-vfs-bot va-vfs-bot temporarily deployed to dbex/87623-failed-0781-document-upload-logging/main/main July 19, 2024 18:50 Inactive
tblackwe
tblackwe previously approved these changes Jul 22, 2024
@freeheeling freeheeling marked this pull request as ready for review July 22, 2024 16:00
@freeheeling freeheeling requested review from a team as code owners July 22, 2024 16:00
Comment on lines +83 to +93

def status_updater(status, document_upload)
# UploadStatusUpdater encapsulates all parsing of a status response from Lighthouse
@status_updater ||= BenefitsDocuments::Form526::UploadStatusUpdater.new(status, document_upload)
end

def statsd_document_base_key(statsd_document_type_key)
@statsd_document_base_key ||= "#{STATSD_BASE_KEY}.#{statsd_document_type_key}"
end

def log_failure(document_upload)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. I like that you pulled out some logic fromupdate_document_status and made these methods 🎉

return unless document_upload.form0781_types?

submission = document_upload.form526_submission
Rails.logger.info(
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though it's a failure, you want this at the info level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rmtolmach Considering your posed question, in ef9e562, I've updated the log level to warn. Thanks!

rmtolmach
rmtolmach previously approved these changes Jul 22, 2024
Copy link
Contributor

@rmtolmach rmtolmach left a comment

Choose a reason for hiding this comment

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

LGTM as long as the info logger is the logging level you want. Otherwise, @ me and I'll re-review if you make any changes.

@freeheeling freeheeling dismissed stale reviews from rmtolmach and tblackwe via ef9e562 July 22, 2024 20:14
@freeheeling freeheeling force-pushed the dbex/87623-failed-0781-document-upload-logging branch from 418d1cc to ef9e562 Compare July 22, 2024 20:14
@freeheeling
Copy link
Contributor Author

@rmtolmach Reposting...

Considering your posed question, in ef9e562, I've updated the log level to warn. Thanks!

@freeheeling freeheeling merged commit f9069d1 into master Jul 26, 2024
19 checks passed
@freeheeling freeheeling deleted the dbex/87623-failed-0781-document-upload-logging branch July 26, 2024 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disability-compensation Label used for Pull Requests that impact Disability Compensation claims (526EZ) disability-experience To manage benefits disability claims experience.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants