Skip to content

Commit

Permalink
explicit expose locale field
Browse files Browse the repository at this point in the history
globalize gem add a validation for locale field in translation table
we should be explicit expose locale

more details see refinery#140
  • Loading branch information
Deshi Xiao committed Dec 12, 2013
1 parent 06c3755 commit 1299638
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions app/models/refinery/news/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ class Item < Refinery::Core::BaseModel
extend FriendlyId

translates :title, :body, :slug

before_save do |m|

This comment has been minimized.

Copy link
@shioyama

shioyama Dec 12, 2013

Why are you deleting this before_save code?

This comment has been minimized.

Copy link
@xiaods

xiaods Dec 12, 2013

Owner

i go through the globalize source code, don't have see any test code for it purpose. i suppose globalize can do it automation.
we don't need add a guard for before_save callback. is it correct understand?

This comment has been minimized.

Copy link
@shioyama

shioyama Dec 12, 2013

This is refinery-specific code. I originally wrote it for refinerycms in refinery/refinerycms#2462 to solve refinery/refinerycms#2450. As long as you're sure that it's not needed here, it's probably fine to remove it, but I assume it was put there for a reason.

If you leave it and just add the attr_accessible part, does it not work?

This comment has been minimized.

Copy link
@xiaods

xiaods Dec 12, 2013

Owner

if i left here, it's fail by spec sample case.

1) Refinery::News::ItemsController#index assigns items and page
     Failure/Error: let!(:item) { FactoryGirl.create(:news_item) }
     ActiveRecord::StatementInvalid:
       SQLite3::ConstraintException: refinery_news_item_translations.refinery_news_item_id may not be NULL: INSERT INTO "refinery_new
s_item_translations" ("created_at", "locale", "updated_at") VALUES (?, ?, ?)
     # ./app/models/refinery/news/item.rb:16:in `block in <class:Item>'
     # ./app/models/refinery/news/item.rb:16:in `block in <class:Item>'
     # ./spec/controllers/refinery/news/items_controller_spec.rb:6:in `block (2 levels) in <module:News>'

this error log is very like report by refinery/refinerycms#2450.

if you remove the before_save code, all relative spec sample case is pass. I think currently globalize already support it to generate foreign key id.

This comment has been minimized.

Copy link
@shioyama

shioyama Dec 12, 2013

Ah, sorry now I remember. Those lines of code were needed to work correctly with the AR3 version of globalize, but not for AR4. So you're right, probably they're not needed here.

This comment has been minimized.

Copy link
@parndt

parndt Dec 12, 2013

gotcha.

m.translation.globalized_model = self
m.translation.save if m.translation.new_record?
class Translation
attr_accessible :locale
end

attr_accessible :title, :body, :content, :source, :publish_date, :expiration_date
Expand Down

0 comments on commit 1299638

Please sign in to comment.