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

Type added manually gets erased when merging #238

Open
dgmora opened this issue Sep 24, 2024 · 3 comments
Open

Type added manually gets erased when merging #238

dgmora opened this issue Sep 24, 2024 · 3 comments

Comments

@dgmora
Copy link

dgmora commented Sep 24, 2024

Because of my specs, the following definition is generated:

  properties:
    first_name:
      nullable: true

To solve that, I manually edited the generated file (as indicated here) to have

  properties:
    name:
      nullable: true
      type: string

This however gets overwritten when re-generating the specs, the type gets lost. I've narrowed this down to this call:

# base = {:nullable=>true, :type=>"string"}
# spec = {:nullable=>true}
merge_schema!(base, spec)

This leaves you with {:nullable=>true}. Since you are iterating spec's keys and spec does not have type, the type gets removed. I wanted to confirm if this is a bug or not. I'm not sure if this somehow conflicts with 'removing' things that aren't present anymore in the API responses?

@exoego
Copy link
Owner

exoego commented Sep 25, 2024

Do you have a RSpec test where a string name is supplied?

@dgmora
Copy link
Author

dgmora commented Sep 25, 2024

The property being called name is not relevant, it can be any property, I updated the description. I can give a try to add a spec

@dgmora
Copy link
Author

dgmora commented Oct 2, 2024

Here's an example https://github.com/dgmora/rspec-openapi/tree/issue-238/type-gets-erased. The test I focused on was
bundle exec rspec spec/rspec/rails_spec.rb:46 which shows:

       Diff:

         {
           # ...
           "paths" => {
             # ...
             "/tables/{id}" => {
               "get" => {
                 # ...
                 "responses" => {
                   "200" => {
                     # ...
                     "content" => {
                       "application/json" => {
                         "schema" => {
                           # ...
                           "properties" => {
                             # ...
                             "nullable_w_manual_type" => {
                               # ...
       -                       "type" => "string"
                             },
                           },
                           # ...
                         },
                         # ...
                       }
                     }
                   }
                 }
               }
             },
             # ...
           },
           # ...
         }
     # ./spec/rspec/rails_spec.rb:52:in `block (3 levels) in <top (required)>'

Finished in 2.43 seconds (files took 0.5469 seconds to load)
1 example, 1 failure

I wanted to check further, but I was not sure what would be the right way to address it. Considering both the original and the new keys (instead of only the new as it's happening now) would probably change the way things are merged and it would not remove changes that had happened in the API.

Something else I noticed is that using binding.pry while running rspec doesn't seem to stop at the breakpoint, I'm not sure if there's any workaround for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants