unmantained-activeadmin-plugins / activeadmin-globalize

MIT License
51 stars 188 forks source link

Avoid saving new translations that are empty, remove existing empty translations. #14

Open sepastian opened 11 years ago

sepastian commented 11 years ago

Activeadmin-globalize3 renders inputs for all possible translations. Consequently, params will contain translations for all attributes - even empty ones - and many empty translation records will be created.

This feature registers a before_filter in ActionController::Base that will

  1. remove new and empty translations from params before creating any empty records
  2. mark existing and empty translations in params for deletion

See https://github.com/stefanoverna/activeadmin-globalize3/issues/10, most credits go to codingluke and fabn, thanks!

codingluke commented 11 years ago

Hi Sebastian, I like your Idea but im not so sure if this is the right way, cause with your code every ActiveRecord update/create triggers the before filter FilterEmptyTranslations. Now this method just checks if in translations_attributes the First two attributes are empty. Is it like this it deletes the whole entry.

Now just imagine some guy has also an entry called "translations_attributes" but has not the structure form activeadmin-globalize3. Then it will delete the entries and the guy might be really confused ;)

I like to have the method centralized, but I think it should be configurable where the filter should trigger.

codingluke commented 11 years ago

oh, I didn't see that commit. Great! in this case I like your implementation :)

https://github.com/sepastian/activeadmin-globalize3/commit/334c8449ffe198b07b8ed951750df29e17727b4c

sepastian commented 11 years ago

I added this later. But you are right, adding the filter to ActionController::Base is probably not the best of all options.

I will look into adding the filter automatically only to classes which do active_admin_translates.

jdurand commented 10 years ago

This works pretty well. Any reason for this not being merged at this point?

sepastian commented 10 years ago

I found at least one reason: ActiveAdmin::Globalize3::FilterEmptyTranslations#filter_empty_translations does not handle nested, translated attributes correctly.

I do not know out of my head how exactly the Rails params hash will look, if nested, translated attributes are used. However, looking at my implementation of filter_empty_translations, it becomes obvious that params[model][:translation_attributes] will not work for nested translations.

However, this could be fixed easily; all that is required is parsing the params hash recursively for :translation_attributes.