Mobile: Create new Secure Messaging Threads endpoint #12851

Tonksthebear · 2023-05-31T22:36:40Z

Summary

Create a new endpoint that uses the new Secure Messaging Threads api

Related issue(s)

department-of-veterans-affairs/va.gov-team#5621

Testing done

Added tests to the module spec

Screenshots

Note: Optional

What areas of the site does it impact?

None (completely new endpoint)

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 or Grafana (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

How to integrate pagination

aherzberg · 2023-06-01T22:07:18Z

modules/mobile/spec/request/threads_request_spec.rb

+
+require 'rails_helper'
+require_relative '../support/iam_session_helper'
+require_relative '../support/mobile_sm_client_helper'


with my VCR PR just being merged, these should be pointing to the helpers folder.

Ooo good catch, thanks. I just merged master so I'll make sure that passes

aherzberg · 2023-06-01T22:21:29Z

modules/mobile/spec/request/threads_request_spec.rb

+    end
+  end
+
+  context 'Advanced User' do


I believe all mobile users are premium users (I forget where I heard this so we may want to verify that ) so I don't think basic and advanced user specs will apply.

Nevermind. I just looked up the slack conversation that talked about this and it's not true. we can have MHV advanced and basic users.

aherzberg · 2023-06-01T22:38:25Z

modules/mobile/docs/openapi.yaml

+          content:
+            application/json:
+              schema:
+                $ref: ./schemas/SecureMessageList.yml


Should this be SecureMessageThreadList?

kpethtel

I think Andrew is probably right about renaming that schema file. Looks good and I don't consider any of my feedback to be blocking.

kpethtel · 2023-06-02T13:46:50Z

modules/mobile/docs/openapi.yaml

+        - description: The field to sort the results by
+          in: query
+          name: sortField,
+          required: true


Do we know if all of these are actually required? Will our front end even be using them? The client is written to always use them but it's not clear if omitting them causes any issues because they don't have any sample specs addressing it, nor do they require the params in the controller. I guess we'll find out in testing.

I can always set a default in the controller so that it's not required

None of the fields outside of folder id are required so I required from those

kpethtel · 2023-06-02T13:51:18Z

modules/mobile/docs/openapi.yaml

+            type: string
+            enum: [ ASC, DESC ]
+        - $ref: "#/components/parameters/InflectionHeader"
+      responses:


The question has been coming up recently of "should these docs define all possible responses?" That would mean that every single entry in this file should have a section for 500, 502, 503, etc. I don't think you need to make any changes, but we do need to have that conversation sometime and this PR is reminding me. I'm going to file a ticket today for figuring that out.

Ya I was going to wait until that discussion comes to a conclusion

I've created a ticket for it, so I'd say just skip it for now.

kpethtel · 2023-06-02T13:56:20Z

modules/mobile/spec/request/threads_request_spec.rb

+      end
+
+      expect(response).to be_successful
+      expect(response).to match_camelized_response_schema('message_threads')


Would it would be worth having a "matches exactly this" test? It's not the most graceful, but I've been burned by schema tests too many times and don't trust them. Also, it can be helpful at times to have an example of the data in the specs.

I remember you saying that before. I just updated it now

aherzberg

LGTM

kpethtel · 2023-06-02T18:55:09Z

modules/mobile/docs/openapi.yaml

+          schema:
+            type: string
+            enum: [ SENDER_NAME, RECIPIENT_NAME, SENT_DATE, DRAFT_DATE ]
+        - description: The order to sort the results by


Do you know if this defaults to ASC or DESC? Could be worth capturing that and also the fact that page defaults to 1 (even though that's pretty obvious).

DESC I believe. I just tried sender_name and the order was ISLAM, MOHAMMAD RAFIQ then GOPARAJU, BHANU

aherzberg

LGTM

* Create new endpoint * Rubocop * Update documentation and add new error response * Update parameter name to what front end uses * Update spec to new directory structure * Reference correct 200 schema * Update spec to check specific return * Remove required from parameters that aren't required * Update documentation description

github-actions bot added the mobile label May 31, 2023

va-vsp-bot requested a deployment to secure_messaging_threads/main/main May 31, 2023 22:37 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main May 31, 2023 22:48 Inactive

Tonksthebear marked this pull request as ready for review June 1, 2023 18:30

Tonksthebear requested review from a team as code owners June 1, 2023 18:30

github-actions bot added the console-services-review label Jun 1, 2023

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 1, 2023 18:30 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 1, 2023 18:31 Inactive

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 1, 2023 19:01 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 1, 2023 19:09 Inactive

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 1, 2023 21:04 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 1, 2023 21:43 Inactive

aherzberg reviewed Jun 1, 2023

View reviewed changes

Tonksthebear requested a review from aherzberg June 1, 2023 22:10

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 1, 2023 22:10 In progress

aherzberg reviewed Jun 1, 2023

View reviewed changes

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 1, 2023 22:39 Inactive

kpethtel previously approved these changes Jun 2, 2023

View reviewed changes

Tonksthebear dismissed kpethtel’s stale review via 02f4c44 June 2, 2023 16:38

Tonksthebear requested review from kpethtel and aherzberg June 2, 2023 16:38

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 2, 2023 16:38 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 2, 2023 16:39 Inactive

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 2, 2023 16:43 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 2, 2023 16:44 Inactive

aherzberg previously approved these changes Jun 2, 2023

View reviewed changes

Tonksthebear dismissed aherzberg’s stale review via bb79ef6 June 2, 2023 17:26

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 2, 2023 17:26 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 2, 2023 17:29 Inactive

Tonksthebear requested a review from aherzberg June 2, 2023 17:34

va-vsp-bot requested a deployment to secure_messaging_threads/main/main June 2, 2023 17:38 In progress

va-vfs-bot temporarily deployed to secure_messaging_threads/main/main June 2, 2023 17:38 Inactive

kpethtel reviewed Jun 2, 2023

View reviewed changes

aherzberg approved these changes Jun 5, 2023

View reviewed changes

Tonksthebear merged commit 87d02f2 into master Jun 5, 2023

Tonksthebear deleted the secure_messaging_threads branch June 5, 2023 23:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mobile: Create new Secure Messaging Threads endpoint #12851

Mobile: Create new Secure Messaging Threads endpoint #12851

Tonksthebear commented May 31, 2023

aherzberg Jun 1, 2023

Tonksthebear Jun 1, 2023

aherzberg Jun 1, 2023

aherzberg Jun 1, 2023

aherzberg Jun 1, 2023

Tonksthebear Jun 2, 2023

kpethtel left a comment

kpethtel Jun 2, 2023

Tonksthebear Jun 2, 2023

Tonksthebear Jun 2, 2023

kpethtel Jun 2, 2023

Tonksthebear Jun 2, 2023

kpethtel Jun 2, 2023

kpethtel Jun 2, 2023

Tonksthebear Jun 2, 2023

aherzberg left a comment

kpethtel Jun 2, 2023

Tonksthebear Jun 5, 2023

aherzberg left a comment

Mobile: Create new Secure Messaging Threads endpoint #12851

Mobile: Create new Secure Messaging Threads endpoint #12851

Conversation

Tonksthebear commented May 31, 2023

Summary

Related issue(s)

Testing done

Screenshots

What areas of the site does it impact?

Acceptance criteria

Requested Feedback

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

kpethtel 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aherzberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aherzberg left a comment

Choose a reason for hiding this comment