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

Fix passing unpacked options #637

Closed

Conversation

t0biasz
Copy link

@t0biasz t0biasz commented Mar 13, 2024

The define_handler_methods method in the Mobility gem's plugin was not properly handling method invocation with additional parameters, causing issues with methods like will_save_change_to_attribute?(from:,to:). This fix modifies the else block to correctly pass additional parameters to the super method, ensuring proper functionality.

The define_handler_methods method in the Mobility gem's plugin was not properly handling method invocation with additional parameters, causing issues with methods like `will_save_change_to_attribute?(from:,to:)`. This fix modifies the else block to correctly pass additional parameters to the super method, ensuring proper functionality.
@shioyama
Copy link
Owner

Can you rebase (I fixed some failing sequel tests) and add a failing test to show what you are fixing?

@@ -142,7 +142,8 @@ def #{method_name}(attr_name, *rest#{kwargs})
mutations_from_mobility.attribute_previously_changed?(attr_name))
mutations_from_mobility.send(#{method_name.inspect}, attr_name, *rest#{kwargs})
else
super
options = rest.first || {}
super(attr_name, **options)
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, this doesn't look right. There is already some code handling keyword arguments above. Are you sure this fixes the issue? Please add a failing test.

@doits
Copy link
Contributor

doits commented Mar 26, 2024

I got something similar with *_previously_changed? methods and fixed it in #639. This feels similar to it. I can take a look.

@doits
Copy link
Contributor

doits commented Mar 26, 2024

I think I got it fixed correctly in #639.

@shioyama
Copy link
Owner

shioyama commented Mar 31, 2024

Merged @doits's fix and shipped as 1.3.0.rc3, thanks!

@shioyama shioyama closed this Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants