Handle symbol argument in find_or_initialize_with_errors#5840
Open
MahmoudBakr23 wants to merge 1 commit intoheartcombo:mainfrom
Open
Handle symbol argument in find_or_initialize_with_errors#5840MahmoudBakr23 wants to merge 1 commit intoheartcombo:mainfrom
MahmoudBakr23 wants to merge 1 commit intoheartcombo:mainfrom
Conversation
The method documented required_attributes as accepting an array, but did not guard against a bare symbol being passed directly. When a symbol was given, the subsequent .each call on required_attributes raised NoMethodError and .size returned the character count of the symbol name rather than 1, so the size comparison against attributes.size always failed. Wrapping required_attributes with Array() at the entry point normalises all valid inputs (symbol, string, array) without changing behaviour for existing callers that already pass an array. Fixes heartcombo#5588
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5588
What
find_or_initialize_with_errorsnow accepts a bare symbol (or string) forrequired_attributes, in addition to the documented array form.Why
The method's contract required an array, but didn't enforce it. When a caller passed a bare symbol — as
reset_password_by_tokendoes internally when delegating throughfind_or_initialize_with_error_by— two things broke silently:required_attributes.sizereturns the character count of the symbol name (e.g.5for:email) instead of1, so the size comparison againstattributes.sizealways fails and the record is never returned even when it exists.required_attributes.eachiterates over the symbol as an enumerator of its characters (Ruby 2) or raisesNoMethodError(Ruby 3), so the error-building block at the end crashes before adding any validation errors.The root cause is that the method does no normalisation before using
required_attributesas a collection.Fix
One
Array()call at the entry point:Array()is the standard Ruby idiom for this normalisation:Array([:email])→[:email](existing array callers are unaffected)Array(:email)→[:email](bare symbol now works)Array("email")→["email"](bare string now works)Array(nil)→[](nil is safe)All existing callers already pass arrays, so there is zero behaviour change for the current codebase.
Tests
Two new cases added to
test/models/authenticatable_test.rb, covering the symbol path for both the found and not-found branches:Full test run: 9 runs, 10 assertions, 0 failures, 0 errors, 0 skips.