Skip to content

Conversation

@run-o
Copy link
Contributor

@run-o run-o commented Oct 25, 2017

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

Description of the PR

Modify build_from_hash to compare the given value against the enum's value rather than the enum's constant name.
Unless I'm missing something, it seems to me that this should be the desired behavior for build_from_hash.

For instance, consider the following Ruby enum class:

class Enum
  ENUM1 = "enum 1"
  ENUM2 = "enum 2"
  ENUM3 = "enum 3"
  def build_from_hash(value)
    ...
  end
end

If we run the following:

Enum.new.build_from_hash("enum 2")

It'll return "enum 2".
Before this fix, it would return an error since we were comparing "enum 2" against the constant names "ENUM1", "ENUM2" and "ENUM3".

Hope this makes sense!

@run-o
Copy link
Contributor Author

run-o commented Oct 25, 2017

@wing328 Here's my PR for the Ruby enum fix. Let me know if it makes sense. Thanks!

@wing328
Copy link
Contributor

wing328 commented Oct 26, 2017

@run-o thanks for the PR. Can you add a test case in a new file named "enum_spec.rb" under the spec folder for the following enum class:

 module Petstore
   class EnumClass
     
     ABC = "_abc".freeze
     EFG = "-efg".freeze
     XYZ = "(xyz)".freeze

@wing328
Copy link
Contributor

wing328 commented Oct 26, 2017

cc @NicoEigenmannCW

Linked to #6131

@wing328 wing328 changed the title Fix build_from_hash function for Ruby enums [Ruby] Fix "build_from_hash" function for Ruby enums Nov 1, 2017
@wing328
Copy link
Contributor

wing328 commented Nov 1, 2017

@run-o please add the test case later when you've time. Thanks.

The fix looks good to me so merging it into master.

@wing328 wing328 merged commit 9272b3e into swagger-api:master Nov 1, 2017
run-o pushed a commit to TheJumpCloud/jcapi-ruby that referenced this pull request Jun 13, 2018
Fix instructions. The manual patch is not needed anymore after I submitted a proper patch in the swagger-codegen repo: swagger-api/swagger-codegen#6812
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.

2 participants