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

update Form526Submission.form w/ classificationCode before submitting to eVSS #12905

Merged
merged 0 commits into from
Jun 9, 2023

Conversation

lukey-luke
Copy link
Contributor

@lukey-luke lukey-luke commented Jun 6, 2023

Summary

Related issue(s)

Testing done

  • this is covered in bundle exec rspec /Users/verdanceluke/kyle_example/vets-api/spec/jobs/evss/disability_compensation_form/submit_form526_all_claim_spec.rb:84

Screenshots

Note: Optional

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

  • sidekiq job on the backend
  • this job is kicked off after veteran clicks submit on 526-ez form

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
    • Rails.logger added here in order to improve monitoring in datadog for this
  • Documentation has been updated (link to documentation)
    • n/a, eVSS has been notified and will make the api change available in staging later this week
  • 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 or Grafana (if applicable)
    • added logging, which I believe should be searchable in Datadog
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
    • n/a tested via rspec
  • I added a screenshot of the developed feature

Misc

noice

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?

@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 6, 2023 14:48 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 6, 2023 14:56 In progress
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 6, 2023 14:57 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 6, 2023 14:58 Inactive
@lukey-luke lukey-luke requested a review from mchae-nava June 6, 2023 15:01
@lukey-luke lukey-luke marked this pull request as ready for review June 6, 2023 15:01
@lukey-luke lukey-luke requested review from a team as code owners June 6, 2023 15:01
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 6, 2023 15:03 Inactive
Copy link
Contributor

@mchae-nava mchae-nava left a comment

Choose a reason for hiding this comment

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

Have some outcomes from the LH thread, but in the meantime, some considerations

@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 7, 2023 21:19 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 7, 2023 21:26 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 7, 2023 21:28 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 7, 2023 21:31 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 7, 2023 22:11 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 7, 2023 22:11 Inactive
Copy link
Contributor

@kylesoskin kylesoskin left a comment

Choose a reason for hiding this comment

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

Do you want the contention code given to be overwritten with nil if the service returns nil? (what it looks like it is currently doing) or is it supposed to keep the code it had if the service does not return a code?

@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 8, 2023 16:07 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 16:10 In progress
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 16:21 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 8, 2023 16:22 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 16:28 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 8, 2023 16:28 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 19:46 In progress
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 19:52 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 8, 2023 19:55 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 22:48 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 8, 2023 22:49 Inactive
@va-vsp-bot va-vsp-bot requested a deployment to bugfix-cc-endpoint/main/main June 8, 2023 23:19 In progress
@va-vfs-bot va-vfs-bot temporarily deployed to bugfix-cc-endpoint/main/main June 8, 2023 23:19 Inactive
Copy link
Contributor

@jeremy6d jeremy6d left a comment

Choose a reason for hiding this comment

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

This was a good PR! Just one little comment but definitely not a blocker. Appreciate the writeup -- always nice for us on support to kind of have more info on what the work's purpose is and where it touches.

api_key: 9d3868d1-ec15-4889-8002-2bff1b50ba62
health_assessment_path: health-data-assessment
evidence_pdf_path: evidence-pdf
ctn_classification_path: contention-classification/classifier
# ctn_classification_path: contention-classification/classifier
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need this comment in here?

@lukey-luke lukey-luke merged commit a52cbef into master Jun 9, 2023
@lukey-luke lukey-luke deleted the bugfix-cc-endpoint branch June 9, 2023 15:54
ryan-mcneil pushed a commit that referenced this pull request Dec 11, 2023
… to eVSS (#12905)

* fix missing request param

* log submission id before classification

* write update_form_classification() to actually update the form for eVSS

* WIP - troubleshooting method not getting called

* wip endponit

* still WIP testing form update functionality

* fix logic for updating form, test against changed outputs

* remove comment

* fix rubocop issues

* remove safeguards for handling legacy vro code

* simplify form update function

* update spec mock request to resemble current api response

* address rubocop

* change logging statement to work better w/ datadog

* switch to form.to_json

* move nil check up

* update settings for local vro testing

* add additional test case for handling null response

* address rubocop issues

* address rubocop issue

* update spec to reflect latest vro api response

---------

Co-authored-by: verdanceluke <luke@verdance.co>
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.

6 participants