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

Add Swagger Documentation for PdfGenerator2122Controller #18395

Open
wants to merge 47 commits into
base: master
Choose a base branch
from

Conversation

opticbob
Copy link
Contributor

@opticbob opticbob commented Sep 10, 2024

Summary

  • This work is behind a feature toggle (flipper): NO
  • (Summarize the changes that have been made to the platform): The Swagger documentation for RepresentationManagement::V0::PdfGenerator2122Controller has been added in the rswag format and the existing documentation for the RepresentationManagement::V0::PowerOfAttorneyController has been converted to that same format.
  • (If bug, how to reproduce)
  • (What is the solution, why is this the solution?)
  • (Which team do you work for, does your team own the maintenance of this component?): FAR/ARM
  • (If introducing a flipper, what is the success criteria being targeted?)

Related issue(s)

Testing done

  • New code is covered by unit tests: No, this is a documentation only change.
  • 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: With the application up and running you can load http://localhost:3000/representation_management/v0/apidocs in a web browser and see the docs in JSON format. You can also follow the instructions here to load the swagger viewer then point it at that same url to get a more interactive view.
  • 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

Tall screenshot of entire `RepresentationManagement `apidocs page

Screenshot 2024-10-08 at 15-13-33 Swagger UI

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas): It only touches the RepresentationManagement apidocs page.

Acceptance criteria

  • I 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

Copy link

github-actions bot commented Sep 11, 2024

1 Warning
⚠️ This PR changes 428 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

  • modules/representation_management/README.rdoc (+3/-0)

  • modules/representation_management/app/controllers/representation_management/v0/apidocs_controller.rb (+1/-1)

  • modules/representation_management/spec/requests/representation_management/v0/pdf_generator_2122_swagger_spec.rb (+28/-0)

  • modules/representation_management/spec/requests/representation_management/v0/power_of_attorney_swagger_spec.rb (+55/-0)

  • modules/representation_management/spec/support/rswag_config.rb (+207/-0)

  • modules/representation_management/spec/support/swagger_shared_components/v0.rb (+123/-0)

  • rakelib/rswag.rake (+9/-0)

  • spec/swagger_helper.rb (+1/-0)

    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

@va-vfs-bot va-vfs-bot temporarily deployed to 91500_document_PdfGenerator2122Controller/main/main September 11, 2024 21:37 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 91500_document_PdfGenerator2122Controller/main/main September 16, 2024 22:01 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 91500_document_PdfGenerator2122Controller/main/main September 17, 2024 16:09 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 91500_document_PdfGenerator2122Controller/main/main September 17, 2024 19:39 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 91500_document_PdfGenerator2122Controller/main/main September 17, 2024 20:09 Inactive
Copy link
Contributor

@holdenhinkle holdenhinkle left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of minor comments.

title: 'va.gov Representation Management API',
version: '0.1.0',
termsOfService: 'https://developer.va.gov/terms-of-service',
description: 'The API for managing representation for VA Forms 21-22 and 21-22a'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the description should reflect the broader range of functionalities provided by these APIs. Something like, 'A set of APIs powering the POA Widget, Find a Representative, and Appoint a Representative.' @jvcAdHoc What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-10-08 154209

@@ -63,6 +63,16 @@ namespace :rswag do
run_tasks_in_parallel(%w[rswag:appeals_api:prod rswag:appeals_api:dev])
end
end

namespace :representation_management do
desc 'Generate rswag docs for the representation_management'
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps change this to 'Generate rswag docs for representation_management' (i.e. remove the word 'the').

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

Copy link
Contributor

@holdenhinkle holdenhinkle left a comment

Choose a reason for hiding this comment

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

LGTM!

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