Issue# 98709 & 98710 POA Request List and Detail endpoints #19967

ojbucao · 2024-12-19T20:44:17Z

This PR implements the list and detail endpoints of POA Request API:

Serializer:
• Created PowerOfAttorneyRequestSerializer and PowerOfAttorneyRequestResolutionSerializer to format the JSON output in JSON:API standard.
• Includes flattened resolution details
Controller:
• Added PowerOfAttorneyRequestsController with index and show actions.
Specs:
• Added request specs to verify:
• GET /power_of_attorney_requests returns a list of requests.
• GET /power_of_attorney_requests/:id returns the details of a specific request.
• Added serializer specs to ensure accurate formatting of response data:

- Implement PowerOfAttorneyRequestSerializer to handle resolution attributes: - Includes `id`, `type`, `created_at`, `reason`, and conditional `creator_id` - Ensure safe handling of nil values with safe navigation (`&.`) and `try` - Add RSpec tests to validate: - Resolution serialization for decisions, expirations, and declinations - Handling of `reason` and `creator_id` conditionally - Nil resolution scenarios

- Implement `PowerOfAttorneyRequestSerializer` to standardize JSON:API output - Add request specs for controller endpoints (`index` and `show`) - Add serializer specs to ensure proper formatting of resolution details - Normalize response keys for consistency in test expectations

github-actions · 2024-12-19T20:45:07Z

	1 Warning
⚠️	This PR changes `223` 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/accredited_representative_portal/app/controllers/accredited_representative_portal/v0/power_of_attorney_requests_controller.rb (+6/-41) modules/accredited_representative_portal/app/serializers/accredited_representative_portal/power_of_attorney_request_resolution_serializer.rb (+19/-0) modules/accredited_representative_portal/app/serializers/accredited_representative_portal/power_of_attorney_request_serializer.rb (+15/-0) modules/accredited_representative_portal/spec/factories/power_of_attorney_decision.rb (+6/-0) modules/accredited_representative_portal/spec/factories/power_of_attorney_request.rb (+3/-0) modules/accredited_representative_portal/spec/factories/power_of_attorney_request_resolution.rb (+12/-4) modules/accredited_representative_portal/spec/requests/accredited_representative_portal/v0/power_of_attorney_requests_spec.rb (+16/-27) modules/accredited_representative_portal/spec/serializers/accredited_representative_portal/power_of_attorney_request_resolution_serializer_spec.rb (+34/-0) modules/accredited_representative_portal/spec/serializers/accredited_representative_portal/power_of_attorney_request_serializer_spec.rb (+40/-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

…h specs - Implement `PowerOfAttorneyRequestSerializer` to handle serialization of power of attorney requests, including nested resolution data. - Implement `PowerOfAttorneyRequestResolutionSerializer` to serialize resolution details, accommodating various resolution subtypes. - Add comprehensive specs for both serializers to ensure accurate and dynamic handling of attributes. - Adjust model factories accordingly

nihil2501

Some changes to serializer definitions and queries.

nihil2501 · 2024-12-20T16:38:36Z

...app/controllers/accredited_representative_portal/v0/power_of_attorney_requests_controller.rb

      def index
-        render json: POA_REQUEST_LIST_MOCK_DATA
+        poa_requests = PowerOfAttorneyRequest.includes(:resolution)


Let's add .limit(100) just in case.

nihil2501 · 2024-12-20T16:40:59Z

...app/controllers/accredited_representative_portal/v0/power_of_attorney_requests_controller.rb

      def index
-        render json: POA_REQUEST_LIST_MOCK_DATA
+        poa_requests = PowerOfAttorneyRequest.includes(:resolution)


This is where we want to includes the rest of the object graph to not have N+1 query like the original proof of concept spec shows. So maybe this becomes includes(resolution: :resolving)?

nihil2501 · 2024-12-20T16:41:04Z

...app/controllers/accredited_representative_portal/v0/power_of_attorney_requests_controller.rb

      end

      def show
-        render json: POA_REQUEST_ITEM_MOCK_DATA
+        poa_request = PowerOfAttorneyRequest.includes(:resolution).find(params[:id])


This is where we want to includes the rest of the object graph to not have N+1 query like the original proof of concept spec shows. So maybe this becomes includes(resolution: :resolving)?

nihil2501 · 2024-12-20T16:42:15Z

...tal/app/serializers/accredited_representative_portal/power_of_attorney_request_serializer.rb

+    attribute :created_at do |object|
+      object.created_at.iso8601
+    end


How was this getting serialized without the custom attribute?

nihil2501 · 2024-12-20T16:42:58Z

...tal/app/serializers/accredited_representative_portal/power_of_attorney_request_serializer.rb

+    attribute :resolution do |object|
+      next nil if object.resolution.blank?
+
+      AccreditedRepresentativePortal::PowerOfAttorneyRequestResolutionSerializer.new(
+        object.resolution
+      ).serializable_hash[:data][:attributes]
+    end


Should we use a relationship serializer? In fact it looks like we want to follow their example here for polymorphism:

class MovieSerializer include JSONAPI::Serializer has_many :actors, serializer: Proc.new do |record, params| if record.comedian? ComedianSerializer elsif params[:use_drama_serializer] DramaSerializer else ActorSerializer end end end

We can't control the structure that way. The result isn't flat like how we want it to be (they get placed under "relationships"). And there doesn't seem to be a way to override the default serialization when using relationships.

nihil2501 · 2024-12-20T16:49:23Z

...ializers/accredited_representative_portal/power_of_attorney_request_resolution_serializer.rb

+# frozen_string_literal: true
+
+module AccreditedRepresentativePortal
+  class PowerOfAttorneyRequestResolutionSerializer


Follow the library's guide for polymorphic relationship instead.

nihil2501 · 2024-12-20T16:49:51Z

...ializers/accredited_representative_portal/power_of_attorney_request_resolution_serializer.rb

+      object.resolving.type
+    end
+
+    attribute :reason


Reason will only need to be serialized on a declination in the decision serializer variant.

nihil2501 · 2024-12-20T16:50:35Z

...ializers/accredited_representative_portal/power_of_attorney_request_resolution_serializer.rb

+    attribute :created_at do |object|
+      object.created_at.iso8601
+    end


How was this serializing without the custom attribute?

nihil2501 · 2024-12-20T16:52:20Z

...ializers/accredited_representative_portal/power_of_attorney_request_resolution_serializer.rb

+    attribute :type do |object|
+      object.resolving_type.demodulize.underscore.split('_').last
+    end


Once we have the polymorphic variants, each of them can hardcode this rather than manipulate the name of the Ruby class. In the end we should get: "type": "decision" and "type": "expiration".

…ved query performance - Updated `includes` to reference `resolving` instead of just `resolution` - Added a limit of 100 records in the `index` action to optimize data retrieval

ojbucao added 3 commits December 19, 2024 12:34

(fix) Apply requested PR changes; all specs passing

a58b23b

ojbucao changed the title ~~POA Request API List and Show endpoints~~ Issue# 98709 & 98710 POA Request List and Detail endpoints Dec 19, 2024

ojbucao requested review from chumpy and nihil2501 December 19, 2024 20:53

ojbucao added the benefits-accredited-rep-facing label Dec 19, 2024

github-actions bot added the test-passing label Dec 19, 2024

ojbucao force-pushed the art/98710/mvp-poa-reqs-api-2 branch 2 times, most recently from ecf8aa3 to fe03d67 Compare December 19, 2024 20:57

ojbucao force-pushed the art/98710/mvp-poa-reqs-api-2 branch from fe03d67 to c7eebf8 Compare December 19, 2024 21:05

ojbucao marked this pull request as ready for review December 19, 2024 21:41

ojbucao requested review from a team as code owners December 19, 2024 21:41

github-actions bot added the require-backend-approval label Dec 19, 2024

ojbucao marked this pull request as draft December 19, 2024 22:21

This was referenced Dec 19, 2024

Implement POA Request List API Endpoint department-of-veterans-affairs/va.gov-team#98710

Open

Implement POA Request Detail API Endpoint department-of-veterans-affairs/va.gov-team#98709

Open

nihil2501 requested changes Dec 20, 2024

View reviewed changes

va-vfs-bot temporarily deployed to art/98710/mvp-poa-reqs-api-2/main/main December 20, 2024 17:30 Inactive

(fix) Refactor AR PowerOfAttorneyRequestController includes for impro…

e3032d5

…ved query performance - Updated `includes` to reference `resolving` instead of just `resolution` - Added a limit of 100 records in the `index` action to optimize data retrieval

va-vfs-bot deployed to art/98710/mvp-poa-reqs-api-2/main/main December 21, 2024 00:28 View deployment

github-actions bot added test-failure and removed test-passing labels Dec 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue# 98709 & 98710 POA Request List and Detail endpoints #19967

Issue# 98709 & 98710 POA Request List and Detail endpoints #19967

ojbucao commented Dec 19, 2024 •

edited

Loading

github-actions bot commented Dec 19, 2024 •

edited

Loading

Files

nihil2501 left a comment

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

ojbucao Dec 21, 2024 •

edited

Loading

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

nihil2501 Dec 20, 2024

Issue# 98709 & 98710 POA Request List and Detail endpoints #19967

Are you sure you want to change the base?

Issue# 98709 & 98710 POA Request List and Detail endpoints #19967

Conversation

ojbucao commented Dec 19, 2024 • edited Loading

github-actions bot commented Dec 19, 2024 • edited Loading

Files

nihil2501 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojbucao Dec 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ojbucao commented Dec 19, 2024 •

edited

Loading

github-actions bot commented Dec 19, 2024 •

edited

Loading

ojbucao Dec 21, 2024 •

edited

Loading