Skip to content

Add README documentation for lib/common library components#27784

Open
AdamKing0126 wants to merge 5 commits intomasterfrom
ak/common-library-readme-updates
Open

Add README documentation for lib/common library components#27784
AdamKing0126 wants to merge 5 commits intomasterfrom
ak/common-library-readme-updates

Conversation

@AdamKing0126
Copy link
Copy Markdown
Contributor

Summary

  • Expands lib/common/README.md to document all components: FileHelpers, HashHelpers, PdfHelpers, S3Helpers, VirusScan, ConvertToPdf, and links to the client/ and models/ subdirectory READMEs
  • Adds lib/common/exceptions/README.md with the full exception class hierarchy grouped by category, base class descriptions, and usage examples
  • Adds lib/common/models/README.md covering Base, Collection, RedisStore, Resource, CacheAside, and supporting types

Test plan

  • Review each README for accuracy against the source files
  • Confirm exceptions/README.md class table matches files in lib/common/exceptions/
  • Confirm models/README.md examples are consistent with the actual class APIs

Documents all components in lib/common including FileHelpers, HashHelpers,
PdfHelpers, S3Helpers, VirusScan, and ConvertToPdf in the top-level README.
Adds new README files for exceptions/ and models/ covering the full class
hierarchy and usage patterns for each.
@AdamKing0126 AdamKing0126 requested a review from a team as a code owner April 15, 2026 21:23
Copilot AI review requested due to automatic review settings April 15, 2026 21:23
@AdamKing0126 AdamKing0126 marked this pull request as draft April 15, 2026 21:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Expands documentation for the lib/common shared utility library in vets-api, adding comprehensive READMEs for common models and exception hierarchies to make component usage and contracts easier to discover and apply consistently.

Changes:

  • Expanded lib/common/README.md to document Common library components and link to sub-READMEs.
  • Added lib/common/exceptions/README.md describing the exception hierarchy and usage.
  • Added lib/common/models/README.md documenting non-ActiveRecord model base classes, collections, and Redis-backed models.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
lib/common/README.md Provides an index-style overview of Common components with usage snippets and links.
lib/common/exceptions/README.md Documents exception classes, intended HTTP statuses, and raising patterns.
lib/common/models/README.md Documents Common model base classes/concerns (Virtus/Dry::Struct/RedisStore/Collection).

Comment thread lib/common/README.md
Comment thread lib/common/models/README.md Outdated
Comment thread lib/common/models/README.md Outdated
Comment thread lib/common/exceptions/README.md
Comment thread lib/common/exceptions/README.md
Comment thread lib/common/exceptions/README.md Outdated
Comment thread lib/common/README.md Outdated
@AdamKing0126
Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Copilot AI and others added 3 commits April 15, 2026 21:36
- Fix Common::VirusScan: false means not confirmed clean (infected or scanning blocked/skipped); add Settings.clamav.mock type-casting note
- Fix Common::ConvertToPdf: always writes ClamAV temp file first; PDFs return a temp copy, not a pass-through
- Fix Common::Models intro to use Common namespace (not Common::Models::*)
- Fix Comparable::Ascending/Descending to Common::Ascending/Descending
- Fix exceptions README: TimeoutError -> Timeout with status 500
- Move FailedDependency (424) from 5xx to 4xx section
- Fix OpenIdServiceError status to %{status} (caller-supplied)
- Fix ServiceError to 500, UpstreamPartialFailure to 502, PrescriptionRefillResponseMismatch to 502

Agent-Logs-Url: https://github.com/department-of-veterans-affairs/vets-api/sessions/fb441a1b-2707-43ee-8389-e1d27056c4c7

Co-authored-by: AdamKing0126 <7659922+AdamKing0126@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

@copilot apply changes based on the comments in this thread

Applied all review feedback in commit cbc7a43:

  • Common::VirusScan: Updated to clarify false means "not confirmed clean" (includes infected and scanning skipped/blocked), and added a note that Settings.clamav.mock should be cast to a real boolean via ActiveModel::Type::Boolean.new.cast(...).
  • Common::ConvertToPdf: Updated to clarify it always writes to a ClamAV temp file first; for PDFs it returns a temp-file copy (caller must clean up), not a transparent pass-through.
  • Common::Models intro: Reworded to avoid implying a Common::Models::* namespace — classes live directly under Common.
  • Comparable::Ascending/Comparable::Descending → corrected to Common::Ascending/Common::Descending.
  • Exceptions table: Fixed TimeoutErrorTimeout (status 500, not 504); moved FailedDependency (424) to the 4xx section; fixed OpenIdServiceError to %{status}; corrected ServiceError to 500, UpstreamPartialFailure to 502, and PrescriptionRefillResponseMismatch to 502.

@AdamKing0126 AdamKing0126 marked this pull request as ready for review April 21, 2026 19:47
@AdamKing0126 AdamKing0126 enabled auto-merge (squash) April 21, 2026 19:48
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.

4 participants