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

New controller-side paradigm not working for me #92

Open
jasonperrone opened this issue Oct 18, 2017 · 10 comments
Open

New controller-side paradigm not working for me #92

jasonperrone opened this issue Oct 18, 2017 · 10 comments

Comments

@jasonperrone
Copy link

We've been using the delocalize gem since Rails 3.1. I just upgraded to Rails 4.2 and delocalize 0.4.0 was broke so I decided to go ahead and upgrade to the latest branch. I noticed, and I read your dissertation on why you undid delocalize's "magic". I'm not sure that moving from convention to configuration is the way to go, but in this case, this is causing me some serious problems. I have one use case where we have an incredibly complex parameters hash coming through, with nested attributes within nested attributes, and with 0.4.0, this was "magically" handled. All dates entered magically delocalized because they came right from the view that way. Now I have to explicitly configure them, which is difficult with this nested parameters hash, but my bigger problem is we very often add additional fields to this hash, and the programmers responsible never had to worry about "oh, the field I am adding is a date, better add it to the delocalize config hash".

Anyway, thought I'd just mention that sometimes that "magic" solves problems elegantly for us. That magic is why Ruby is better than Java.

@clemens
Copy link
Owner

clemens commented Oct 18, 2017

Hey Jason,

I fully understand and I can see that this was a controversial change. If you look at the history, that's exactly the reason why I sought discussion and the merge happened after almost 2 years (!) – if you look at the history, you can see that I made the first commits to the new approach in August 2013 and merged the new stuff into master in November 2015.

That being said, I'm happy to merge any solution you might have that lets you run the original gem with Rails 4.2. The two options are:

  • Build something that's backwards compatible and still works with Rails 3.x => I could release this as 0.4.1 if it keeps working with a Rails version that worked fine with 0.4.0.
  • Build something that only works on Rails 4.x (or even just 4.2 and not 4.0/4.1) => I could release this as 0.5.0 despite it strictly speaking not being fully compliant with Semantic Versioning.

Let me know if you want to do this and if so, I'm happy to accept a pull request.

Cheers,

  • C.

@jasonperrone
Copy link
Author

Thanks, Clemens.

Well, pretty quickly after 4.2 we'll be upgrading to 5.x. So, I'd be right back in this boat again.

Correct me if I'm wrong, but the 0.4.0 version tried to delocalize every text input, is that correct?

@jasonperrone
Copy link
Author

Here's where I am going with this... so in 0.4.0 it looks like it tried to delocalize every input field. You didn't have to put a marker or a class or anything on the text_field_tag. So what if you had a delocalize! method which didn't take any configuration, much like strong parameters permit! method. It then goes deeply through the entire parameters hash and attempts to delocalize every field, pretty much like 0.4.0 did. And like 0.4.0 you don't have to tell it whether it's a date or a time.

Therefore, you'd keep your current delocalize(delocalize_config) method, but add a delocalize! which pretty much just does in the controller what it used to do in the view. Since all my controllers have to handle strong_parameters anyway, it's no big deal to just add a delocalize! call to them.

Thoughts?

@jasonperrone
Copy link
Author

I've spent hours re-engineering my app for the new delocalize version, but it seems to me that Rails' date_field form helper renders this whole gem unnecessary.

@fmluizao
Copy link
Collaborator

@jasonperrone I'm not familiar with date_field, but I think it doesn't allow you to change the input format. Am I wrong?

@jasonperrone
Copy link
Author

What I said was a tad misleading. It's really input type="date" and then HTML5's new date field functionality taking over, which does appear to do all the transformation stuff. Problem is it is not yet at all implemented by IE, and looks different in Edge than it does in Chrome. I am told the HTML5 date functionality is still young yet. If you were supporting only Chrome, it's feature rich and comes with a beautiful date picker widget that blows jQuery's away. Keep your eye on that feature.

@fmluizao
Copy link
Collaborator

Although chrome displays a date picker, the HTML5 specifications does not mention anything about displaying a calendar... it would be nice if we could rely on this. I'm using delocalize + datepicker plugin for some years, and accepting multiple input formats, according to the user's locale, works great.

I'm not saying it's perfect, but I think that parameter conversion should be done at controller level, not model (as Rails itself does). I know it's more convenient, but if we think for a moment about responsabilities, data conversion doesn't seem to belongs to the model...

Just my 2 cents 😄

@jasonperrone
Copy link
Author

Understood :)

I wrote a simple method in my ApplicationController that automatically delocalizes all permitted parameters with keys of /^date$|date$|^date/ so as long as my developers keep to our naming conventions, they'll never have to worry about delocalizing in the controllers. Unfortunately, in the views I have to use value: f.object.whatever.try(:to_s,:app_date) to get the date from the database to go back to our desired format. Luckily, out of our extensive app I only had three html.erbs with date fields that get saved into the DB, so not too bad.

@fmluizao
Copy link
Collaborator

fmluizao commented Nov 1, 2017

For the views, if you are using a FormBuilder like simple_form or formtastic, you can create a custom input to automatic format the value. This way, you don't need to change every view, all logic related to formatting will be centralized in the form builder.

Examples here:

https://github.com/plataformatec/simple_form#custom-inputs

I have a fairly large Rails app (120+ forms), and update was quite easy, thanks to the use of FormBuilder. In fact, I recommend using a custom FormBuilder even without delocalize, as it can reduce maintenance efforts, by centralizing your form logic in one place.

@jasonperrone
Copy link
Author

Thanks, Fernand!

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

No branches or pull requests

3 participants