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

12289-ihub-appointments-betamocks #2208

Merged
merged 1 commit into from Aug 18, 2018
Merged

Conversation

hpjaj
Copy link
Contributor

@hpjaj hpjaj commented Aug 14, 2018

Background

We are trying out the iHub external service through their "Appointments" endpoint. We now have this integrated into Vets-API. Next step is to enable betamocks support so that the FE can build out the UI, with the standard iHub Appointments endpoint response in place to build against.

Sibling PR in Mockdata repo

https://github.com/department-of-veterans-affairs/vets-api-mockdata/pull/49

Sibling Devops PR

https://github.com/department-of-veterans-affairs/devops/pull/3142

Definition of Done

Betamocks for appointments endpoint will respond with default data.

This cannot currently be flexible to create unique beta mock data responses, based on the signed in user. This is due to a regular expression matching rule in the betamocks gem.

For now, a default for any logged in user will work just fine.

Unique to this PR

  • Configure betamocks for iHub appointments endpoint

Applies to all PRs

  • Appropriate test coverage & logging
  • Swagger docs have been updated, if applicable
  • Provide link to originating GitHub issue, or connected to it via ZenHub
  • Does not contain any sensitive information (i.e. PII/credentials/internal URLs/etc., in logging, hardcoded, or in specs)

@va-bot va-bot temporarily deployed to 12289-ihub-appointments-betamocks/master August 14, 2018 17:50 Inactive
@hpjaj hpjaj force-pushed the 12289-ihub-appointments-betamocks branch from d0cd578 to 444b1bf Compare August 14, 2018 17:56
@va-bot va-bot temporarily deployed to 12289-ihub-appointments-betamocks/master August 14, 2018 18:03 Inactive
@@ -67,7 +67,7 @@ ihub:
url: "https://qacrmdac.np.crm.vrm.vba.va.gov"
appointments:
timeout: 30
mock: false
mock: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering my shallow experience with the project, is it typical to default settings to true for mocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kfrz - Yeah, when you first build out the endpoint(s), and do not yet have any betamocking in place, you would want this value to be set to false.

Later on, once you have configured betamocking, gotten the recordings in place in the associated repo, etc., then in order to leverage the betamocking, this mock needs to then be set to true. This essentially turns on betamocking.

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 This makes sense! Changes LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify - if this is a live API, and you opt to set the default to true, you need to make sure that there is a corresponding devops PR that overrides the mock setting to false in staging and prod - i.e. don't accidentaly enable mock data in production!

Copy link
Contributor Author

@hpjaj hpjaj Aug 14, 2018

Choose a reason for hiding this comment

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

Hey @patrickvinograd - yeah, here is the sibling devops PR:

https://github.com/department-of-veterans-affairs/devops/pull/3142

And to your point, I will not be merging any of the other sibling PR's until the devops one is merged and live.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know. Is this caveat documented in the betamocks/engineering guidelines anywhere?

@hpjaj
Copy link
Contributor Author

hpjaj commented Aug 14, 2018

@bshyong @ncksllvn - Here is what the betamocked data response will be for all users, locally, and in dev:

{
  "data": {
    "id":"",
    "type":"ihub_appointments_responses",
    "attributes": {
      "appointments": [
        {
          "start_time":"1996-01-12T08:12:00",
          "appointment_status_code":"",
          "appointment_status_name":"",
          "assigning_facility":"",
          "clinic_code":"409",
          "clinic_name":"ZZCHY WID BACK",
          "facility_code":"442",
          "facility_name":"CHEYENNE VAMC",
          "local_id":"2960112.0812",
          "other_information":"",
          "status_code":"2",
          "status_name":"CHECKED OUT",
          "type_code":"9",
          "type_name":"REGULAR"
        },
        {
          "start_time":"1996-02-15T09:50:00",
          "appointment_status_code":"",
          "appointment_status_name":"",
          "assigning_facility":"",
          "clinic_code":"409",
          "clinic_name":"ZZCHY WID BACK",
          "facility_code":"442",
          "facility_name":"CHEYENNE VAMC",
          "local_id":"2960112.0812",
          "other_information":"",
          "status_code":"2",
          "status_name":"CHECKED IN",
          "type_code":"9",
          "type_name":"REGULAR"
        }
      ]
    }
  }
}

@hpjaj hpjaj force-pushed the 12289-ihub-appointments-betamocks branch from 444b1bf to 36eae08 Compare August 15, 2018 16:50
@va-bot va-bot temporarily deployed to 12289-ihub-appointments-betamocks/master August 15, 2018 16:57 Inactive
@hpjaj
Copy link
Contributor Author

hpjaj commented Aug 16, 2018

@kreek @omgitsbillryan @bshyong - Morning. Can I pls get a review/approval on this? Thanks.

Copy link
Contributor

@kreek kreek left a comment

Choose a reason for hiding this comment

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

LGTM

@hpjaj hpjaj force-pushed the 12289-ihub-appointments-betamocks branch from 36eae08 to a17a1ce Compare August 16, 2018 18:59
@va-bot va-bot temporarily deployed to 12289-ihub-appointments-betamocks/master August 16, 2018 19:06 Inactive
This cannot currently be flexible to create unique beta mock data responses, based on the signed in user.  This is due to a regular expression matching rule in the beta mocks gem.

For now, a default for any logged in user will work just fine.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants