Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.

69916 1010 ez ezr update insurance schema#814

Draft
JoshingYou1 wants to merge 10 commits into
masterfrom
69916_1010_ez_ezr_update_insurance_schema
Draft

69916 1010 ez ezr update insurance schema#814
JoshingYou1 wants to merge 10 commits into
masterfrom
69916_1010_ez_ezr_update_insurance_schema

Conversation

@JoshingYou1
Copy link
Copy Markdown
Contributor

@JoshingYou1 JoshingYou1 commented Nov 14, 2023

What Does This Code Do?

  • Updates the 10-10EZR schema to use a new insurance provider schema

Pull Requests to update the schema in related repositories

After you've merged your changes to vets-json-schema you'll need to make PR's to vets-website and vets-api. Please link them here.

@JoshingYou1 JoshingYou1 marked this pull request as ready for review November 15, 2023 16:11
@JoshingYou1 JoshingYou1 requested review from a team as code owners November 15, 2023 16:11
Copy link
Copy Markdown
Contributor

@longmd longmd left a comment

Choose a reason for hiding this comment

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

Can you do this change without even touching the EZ schema?? This is going to break the EZ again with this setup and we just don't want to be touching that right now.

@JoshingYou1
Copy link
Copy Markdown
Contributor Author

Can you do this change without even touching the EZ schema?? This is going to break the EZ again with this setup and we just don't want to be touching that right now.

Yes, I could, but that would mean keeping shared code that is no longer shared. I basically moved the shared code into the EZ schema, so it should be identical.

@longmd
Copy link
Copy Markdown
Contributor

longmd commented Nov 15, 2023

I'd be okay with that in the short term. Its the $ref that is going to break the FE on the EZ.

@JoshingYou1 JoshingYou1 deleted the 69916_1010_ez_ezr_update_insurance_schema branch November 15, 2023 16:27
@JoshingYou1 JoshingYou1 restored the 69916_1010_ez_ezr_update_insurance_schema branch November 15, 2023 16:29
@JoshingYou1 JoshingYou1 reopened this Apr 28, 2025
@JoshingYou1 JoshingYou1 requested a review from a team as a code owner April 28, 2025 15:40
@JoshingYou1 JoshingYou1 requested review from longmd and removed request for jadon1979 and lihanli April 28, 2025 16:38
@rmtolmach rmtolmach requested a review from Copilot April 28, 2025 19:41
Copy link
Copy Markdown

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

This PR updates the 10-10EZR schema to incorporate the new insurance provider schema and corresponding test validations.

  • Introduces an insuranceProvider helper and extended tests for the providers field in the schema.
  • Updates the schema definition by validating insuranceProvider objects with required properties via an anyOf construct.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/schemas/10-10EZR/schema.spec.js Added test cases for various valid and invalid insuranceProvider object scenarios.
src/schemas/10-10EZR/schema.js Updated the providers property to support the new insurance provider schema using conditional requirements.
Files not reviewed (1)
  • package.json: Language not supported

providers: {
type: 'array',
items: definitions.insuranceProvider,
items: {
Copy link

Copilot AI Apr 28, 2025

Choose a reason for hiding this comment

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

Consider adding 'additionalProperties: false' within the insuranceProvider object schema to prevent acceptance of unexpected properties.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think of this comment @JoshingYou1? I do see some other schemas use it. It's also mentioned in the README. I'll leave the decision up to you.

@rjohnson2011
Copy link
Copy Markdown

Can you address Rebecca's comment above please?

@longmd
Copy link
Copy Markdown
Contributor

longmd commented May 14, 2025

@rjohnson2011 , Josh is OOO until the 27th, I believe. We will have him address that feedback when he returns.

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.

6 participants