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

Fixes for Rails 5 issues discovered in testing by NU #1719

Merged
merged 2 commits into from Oct 2, 2018

Conversation

vandrijevik
Copy link
Contributor

Release Notes

Fixes for Rails 5 issues discovered in testing

Additional Context

  1. Due to the datetime column changes, a TO_TIMESTAMP type cast is necessary to make the query in Oracle comparisons
  2. Rails 5 automatically disables form submit buttons when they are clicked. On the bulk email form, this is undesirable for the Export button, since clicking the button doesn’t leave the page (the file is just downloaded).

Copy link
Contributor

@jhanggi jhanggi left a comment

Choose a reason for hiding this comment

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

Looks good. I wonder if there are any other places where we to disable the disabling?

@vandrijevik
Copy link
Contributor Author

@jhanggi I had the same thought, but I think we can tackle those if they come up—the new default behavior is good, and would prevent accidental double-submissions. It’s just on this page that it doesn’t quite fit because the Export link leaves the browser on the page.

If it turns out there are many other places where we want to flip the behavior, we can then think about setting config.action_view.automatically_disable_submit_tag globally.

@jhanggi
Copy link
Contributor

jhanggi commented Oct 2, 2018

Sounds good.

I know there are a few places that we use disable_with that already did the disabling previously. Maybe we can remove those as we come across them. I didn't realize this was a new default, but I support it.

@vandrijevik vandrijevik merged commit 82a3acc into master Oct 2, 2018
@vandrijevik vandrijevik deleted the rails-5-fixes-from-nu branch October 2, 2018 17:33
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