Skip to content

Conversation

@Spence1115
Copy link
Contributor

@Spence1115 Spence1115 commented Nov 24, 2025

Summary

Fixes #270

When a nested entry inside a response contained multiple possible types, we were only ever returning the first one. For example:

          {
            "fields" => [
              {
                "id" => "country_code",
                "options" => [
                  {
                    "id" => "us",
                    "label" => "United States"
                  },
                  {
                    "id" => "ca",
                    "label" => "Canada"
                  }
                ]
              },
              {
                "id" => "region_id",
                "options" => [
                  {
                    "id" => 1,
                    "label" => "New York"
                  },
                  {
                    "id" => 2,
                    "label" => "California"
                  }
                ]
              }
            ]
          }

For this, the id in options was being marked as type: string, when it should have been oneOf: string | number

This PR changes that, so now we get

{
    "options": {
      "type": "array",
      "items": {
        "type": "object",
        "properties": {
          "id": {
            "oneOf": [
              {
                "type": "string"
              },
              {
                "type": "integer"
              }
            ]
          },
          "label": {
            "type": "string"
          }
        },
        "required": [
          "id",
          "label"
        ]
      }
    }
}
image

Additionally I've added more tests to cover different response shapes to try and ensure any further complexities are covered (along with simple versions), and to ensure all paths of the new code are covered. These may be excessive and I'm happy to cut back if desired.

merged_schema
end

def build_merged_schema_from_variations(variations)

Check notice

Code scanning / Rubocop

A calculated magnitude based on number of assignments, branches, and conditions. Note

Metrics/AbcSize: Assignment Branch Condition size for build_merged_schema_from_variations is too high. [<26, 59, 44> 78.06/46]
merged_schema
end

def build_merged_schema_from_variations(variations)

Check notice

Code scanning / Rubocop

A complexity metric that is strongly correlated to the number of test cases needed to validate a method. Note

Metrics/CyclomaticComplexity: Cyclomatic complexity for build_merged_schema_from_variations is too high. [31/13]
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.68%. Comparing base (204a75e) to head (9beb59d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
- Coverage   97.26%   94.68%   -2.58%     
==========================================
  Files          20       20              
  Lines         695      734      +39     
  Branches      163      185      +22     
==========================================
+ Hits          676      695      +19     
- Misses         19       39      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Spence1115 Spence1115 marked this pull request as draft November 24, 2025 19:45
@Spence1115 Spence1115 force-pushed the better-handling-of-one-of branch from 2ee508c to 48c8dd4 Compare November 24, 2025 20:21
@Spence1115 Spence1115 force-pushed the better-handling-of-one-of branch from 48c8dd4 to 6bf759a Compare November 24, 2025 20:24
@Spence1115 Spence1115 marked this pull request as ready for review November 24, 2025 20:53
@Spence1115
Copy link
Contributor Author

Codecov Report

✅ All modified and coverable lines are covered by tests. ✅ Project coverage is 94.68%. Comparing base (204a75e) to head (9beb59d).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
- Coverage   97.26%   94.68%   -2.58%     
==========================================
  Files          20       20              
  Lines         695      734      +39     
  Branches      163      185      +22     
==========================================
+ Hits          676      695      +19     
- Misses         19       39      +20     

☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

I'm not sure why overall project coverage is down when my branch has 100% coverage...

Copy link
Owner

@exoego exoego left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you

@exoego exoego merged commit d7c2eaa into exoego:master Nov 26, 2025
13 of 14 checks passed
@exoego exoego added the enhancement New feature or request label Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: hashes inside an array with different types are not picked up properly

2 participants