Api 40300 526 bd upload refactor

[mchristiansonVA]

Summary

• This work is behind a feature toggle (flipper): YES
• Adds new class DisabilityDocumentService to move document-specific logic for 526 uploads out of the BD class, to reduce the concerns handled by the BD class to simply handling the BD upload given a request body.
• Adds new class DocumentServiceBase to move shared document upload logic for all doc types out of the BD class, such that it can be reused for further refactoring of other doc type uploads (e.g. EWS, POA).
• Adds tests to exercise logic in new classes.

Related issue(s)

API-40300

Testing done

• New code is covered by unit tests
• Prior 526 upload logic used ClaimsApi::BD service methods to handle logic specific to the doc type and generate the appropriate upload body for submission to BD /upload.
• New code uses DisabilityDocumentService and DocumentServiceBase classes to handle logic specific to L123 & L023 doc types and generate the appropriate upload body for submission to BD /upload, using new ClaimsApi::BD service method to simply submit to BD /upload.
• New logic requires the following flippers to be enabled:
  Flipper.enable :claims_claim_uploader_use_bd
  Flipper.enable ::claims_api_bd_refactor
• Tested 526 v1 and v2 submissions, as well as attachment submissions, and confirmed BD /upload success.

What areas of the site does it impact?

526 v2 synchronous submissions
526 v2 submissions
526 v2 attachment submissions
526 v1 submissions
526 v1 attachment submissions

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

[rockwellwindsor-va]

I'm concerned this method will basically need to be duplicated in the other service files (if I am seeing the direction correctly.

Wondering if there is a way to move this to the service_base perhaps and make it universal enough that we can invoke this one method from the different service files?

I just feel like this method should still be shared, acknowledging of course that the direction may become clearer with another service in the mix to see.

[FonzMP]

good callout here. just had a sync with Matt and I and we were in agreeance 100%. I like the idea of an intermediary service, somewhat generic, and specific to the document functionality. The original idea was to move to servicebase but with that service, it handles most the root level and generic functionalities. The thought now is to have a DocumentService < ServiceBase that would handle some of these shared, generic document specific methods, and then we'll just have a Service for each form type, hopefully with just a singular "extraction" method to get the values needed to call the rest of the services within the DocumentService, which will be the builders.

Hopefully that makes sense.