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

bug(addresses): fix address schema displays #133

Merged
merged 8 commits into from
Jul 9, 2021
Merged

Conversation

sudoku-lord
Copy link
Contributor

@sudoku-lord sudoku-lord commented Jul 8, 2021

In short:

  • The discriminator is unnecessary: it adds complexity & prevents the schema from displaying properly. Getting rid of it doesn't actually negatively affect the functionality, but it allows the schema to display.

  • Redocly doesn't understand how to deal with allOf at the beginning of the address.yml file in shared/models. This format totally messes it up:

allOf:
  - $ref: address_editable_base.yml
  - $ref: address_fields_us.yml
  - $ref: address_generated_base.yml
  - type: object

Given that, the DRY format doesn't quite work for this. I manually included all the fields in address_us and address_intl, and that solved the issues I was seeing in to and from for mailpieces, wherein I kept seeing Recursive instead of the expected schema. Note that this isn't the first time Redocly has had issues with false-positive recursion.

Weirdly, allOf at the beginning of a file DOES work elsewhere, but at this point I'm ready to throw up my hands and implement whatever works.

PLEASE go through this with a fine-tooth comb, including generating the docs and poring over them, to see if I broke anything 😓 I think there are some files I might be able to get rid of, if they're not being referenced at all, but I haven't consolidated that information yet.

Copy link
Contributor

@jho44 jho44 left a comment

Choose a reason for hiding this comment

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

The generated redoc from this looks good. I saw that you fixed how data in the responses portions of "List all"...of the addresses and checks resources wasn't being rendered properly. I also checked that all the properties were there by comparing with the complete examples I'd made for every resource in another branch.

These are the only refs I saw you delete. From a quick finder search, address_generated_base isn't referenced by anyone so that can be safely deleted. (Tested locally and by checking List all Checks > 200 Responses > data > to renders properly.
image
image
But honestly I don't mind if you leave it; we might need it later, given how often we have to refactor things. So if you delete it, I'll reapprove.

Copy link
Contributor

@SidneyAllen SidneyAllen left a comment

Choose a reason for hiding this comment

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

Reviewed output of Redoc and address object displaying correctly.

@sudoku-lord sudoku-lord merged commit bf08ab4 into main Jul 9, 2021
@sudoku-lord sudoku-lord deleted the fix_address branch July 9, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants