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

Fix issue 2450 globalize #2462

Merged
merged 3 commits into from
Nov 18, 2013
Merged

Conversation

shioyama
Copy link
Contributor

@shioyama shioyama commented Nov 6, 2013

This is needed to get refinery to work with the latest version of globalize for ActiveRecord 3.x, because globalize no-longer autosaves translations (see #2450). It requires globalize/globalize#296 to be released and the refinery gemspecs to also be updated from globalize3 ~> 0.3.0 to globalize ~> 3.0.0.

@parndt
Copy link
Member

parndt commented Nov 6, 2013

@shioyama seems sane to me. So this will fail Travis until Globalize is released, is that correct? Should the gemspecs be updated in this pull request too? I like going from green to green if possible.

Love your work.

@shioyama
Copy link
Contributor Author

shioyama commented Nov 7, 2013

Yes, I merged the change to globalize 3-0-stable so all we need to do is release that as 3.0.1 and then update refinery to point to globalize ~> 3.0.0 rather than globalize3. Can you release globalize 3.0.1? Then I can add those changes in this PR, and Travis should go green.

p.s. globalize needs a changelog.

@parndt
Copy link
Member

parndt commented Nov 7, 2013

@shioyama I think I gave you gem push access for globalize.. I see your face: http://rubygems.org/gems/globalize

@shioyama
Copy link
Contributor Author

shioyama commented Nov 7, 2013

I know I was just splitting the (very small) load 😉

@cwise
Copy link

cwise commented Nov 7, 2013

+1 for this. Bit me in the tail today, but was happy to see so much activity on it. Thanks for your hard work.

@shioyama
Copy link
Contributor Author

shioyama commented Nov 7, 2013

😖 (pulling hair out)

Ok, one failing test which I don't see locally:

1) Pages with translations add a page with title only for secondary locale uses id instead of slug in admin

Failure/Error: page.find_link('Edit this page')[:href].should include(ru_page_id.to_s)

expected "/refinery/pages/%D0%BD%D0%BE%D0%B2%D0%BE%D1%81%D1%82%D0%B8/edit?switch_locale=ru" to include "38"

# ./pages/spec/features/refinery/admin/pages_spec.rb:527:in `block (5 levels) in <module:Admin>'
# ./pages/spec/features/refinery/admin/pages_spec.rb:526:in `block (4 levels) in <module:Admin>'

@parndt any clue what's going on?

@parndt
Copy link
Member

parndt commented Nov 7, 2013

Ugh. I don't know sorry. This stuff is too magical in Refinery.. Sorry.

@shioyama
Copy link
Contributor Author

shioyama commented Nov 7, 2013

Problem is that if I don't see the failure with the same environment locally, I have no way to debug it... can you run the tests and see if you get the same? At least that might provide a clue where the difference is.

@shioyama
Copy link
Contributor Author

shioyama commented Nov 7, 2013

p.s. I've run the tests on two different environments on a linux pc and a mac, and see no failures...

@parndt
Copy link
Member

parndt commented Nov 7, 2013

It may just be Travis.. I'll check this out over the next few days.. I'll be at RubyConf so a little preoccupied.

@parndt parndt merged commit 1366824 into refinery:2-1-stable Nov 18, 2013
@parndt
Copy link
Member

parndt commented Nov 18, 2013

Merged! Thanks, Chris

shioyama referenced this pull request in xiaods/refinerycms-news Dec 12, 2013
globalize gem add a validation for locale field in translation table
we should be explicit expose locale

more details see refinery#140
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.

4 participants