Create Mobile appeal model

[aherzberg]

Summary

Create a model for /v0/appeal/{id} resource

the following endpoint does not use a model, defining the schema of the output in the serializer using Structs. These only specify the top level attributes so any nested objects do not have a defined schema. This has caused issues with schema changing out from under us.

We can fix this by creating a model for all of these endpoints that will validate the full schema.

Related issue(s)

department-of-veterans-affairs/va-mobile-app#8145

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?

Screenshots

Note: Optional

What areas of the site does it impact?

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

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?

[github-actions]

1 Warning

⚠️

This PR changes 294 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 (+3/-0) modules/mobile/app/controllers/mobile/v0/claims_and_appeals_controller.rb (+4/-0) modules/mobile/app/models/mobile/v0/adapters/appeal.rb (+29/-0) modules/mobile/app/models/mobile/v0/appeals/appeal.rb (+137/-0) modules/mobile/app/models/mobile/v0/appeals/docket.rb (+13/-0) modules/mobile/app/models/mobile/v0/appeals/evidence.rb (+11/-0) modules/mobile/spec/requests/mobile/v0/appeal_spec.rb (+88/-9) 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

[github-actions]

Doesn't depend on instance state (maybe move it to another class?) - UtilityFunction

[aherzberg]

There's enough being enforced that wasn't before that I think there's a good chance it could cause a rise in error rates. This will keep that from becoming an issue.

[aherzberg]

note that I didn't add the appeals model to the index of this controller because it already uses a claim overview model. We could put the appeal model before the claim overview but that doesn't seem like it'd be of any benefit

[aherzberg]

typo

[aherzberg]

took the appeal response from before this change and made sure the model didn't change anything about the response.

[aherzberg]

This is really good. I'm actually a little surprised that the DRY models are smart enough to automatically coerce the data into the correct classes.

I also think your choice to use a feature flag was a good one. I do think there's a good chance for errors here, so this gives you the opportunity to test easily on staging and prod.

My one thought is that I don't know if it's worth making models for every nested object. Take a look at the payment info model I recently created: modules/mobile/app/models/mobile/v0/payment_information.rb

You can handle nested attributes like:

      attribute :payment_account do
        attribute :account_type, Types::String
        attribute :financial_institution_name, Types::String
        attribute :account_number, Types::String
        attribute :financial_institution_routing_number, Types::String
      end

and bypass all the sub-models, which I think just add clutter. IMO it's only worth having a separate model for things like date and location that are used in multiple places. A docket, for example, probably isn't an independent entity that will be used elsewhere. As we move to making models for everything, I think this approach of having a model for every nested item may be harder to maintain. What do you think?

I didn't realize you could nest attributes like that. That is better. I've updated to do this but unfortunately it looks like there's some issues with having the optional modifier on nested structs. https://discourse.dry-rb.org/t/dry-struct-how-to-create-a-nested-struct-in-an-optional-array/1810/4 . I tried a few different ways of doing it but was getting errors, and according to that forum post, having a separate class was the only way to do it. Let me know if you know how to get around that though. I did remove the classes that weren't optional though.

[kpethtel]

I didn't know about that optional limitation, but I think this is a good compromise. 👍