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

Fixing back Redirectable #381

Merged
merged 4 commits into from
Dec 23, 2022
Merged

Fixing back Redirectable #381

merged 4 commits into from
Dec 23, 2022

Conversation

tsubik
Copy link
Collaborator

@tsubik tsubik commented Dec 12, 2022

Little hacky, but looks like it works.

Story: https://www.pivotaltracker.com/story/show/183760908

redirect_to redirect, notice: "#{resource.model_name.human} was successfully updated."
end
end
update! { params[:return_to] || collection_path }
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't return to the previous page on failure?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if block.present?
extended = Proc.new do |f|
return_to = request.params[:return_to] || request.referer
f.hidden_field :return_to, name: :return_to, value: return_to if return_to.present?
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it be also disabled so that it's not sent in the form params?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this must be sent back to controller (saved between requests) via params in this case. Otherwise, it won't work after validation errors. That's why I first check params[:return_to] if there is a value.

@tsubik
Copy link
Collaborator Author

tsubik commented Dec 22, 2022 via email

super
end

def self.extended(base)
Copy link
Contributor

Choose a reason for hiding this comment

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

this gets evaluated just by extending the module?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor

@santostiago santostiago left a comment

Choose a reason for hiding this comment

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

Not sure I understood it well :P

@tsubik
Copy link
Collaborator Author

tsubik commented Dec 22, 2022 via email

@tsubik
Copy link
Collaborator Author

tsubik commented Dec 23, 2022

Sorry looks like responding via email is not great as it responds to main thread.

@tsubik tsubik merged commit ae7f747 into develop Dec 23, 2022
@tsubik tsubik deleted the fix/fixing-backredirectable branch December 23, 2022 10:05
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.

None yet

2 participants