Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add path for downloading translations with a hierarchy instead of a flat #27

Closed
wants to merge 4 commits into from

5 participants

@cmavromoustakos

The reason for this change is that most javascript I18N plugins expect the data in the same hierarchy that it is stored on as the locale yaml files.

Currently, we have to come up with a conversion to store and retrieve our keys differently to match the key to the translation. With this change we allow the flexibility of allowing the end user (CopyCopter client or some other javascript client) to pass a optional argument formart with a type of hierarchy to get the data in the format of:

  {
    "en": {
      "test": {
        "one": "expected one",           
        "two": "expected two"
      }
    }
  }
Gemfile
@@ -24,10 +24,11 @@ end
group :test do
gem 'bourne', '1.1.1'
gem 'capybara-webkit', '0.3.0'
- gem 'cucumber-rails', '0.4.1'

Please keep gem versions explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
.rvmrc
@@ -1 +1 @@
-rvm 1.9.2
+rvm use 1.9.2-p180

1.9.2's current patchlevel is 315. Enforcing an old one may not be advisable.

This was removed by Dan on master and I will remove the file completely here also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Gemfile
((6 lines not shown))
gem 'database_cleaner', '0.7.1'
gem 'factory_girl_rails', '1.7.0'
gem 'fakeweb', '1.3.0'
+ gem 'ruby-debug19', '~> 0.11.6'

This is the same mention for locked version numbers.

@croaky Owner
croaky added a note

Is there a way to use Ruby debug without including it in the repo? I've been on a 'try not to include anything machine- or developer-specific' kick lately.

copycopter/style-guide@1bda2fc#L0R112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
app/controllers/api/v2/draft_blurbs_controller.rb
@@ -1,7 +1,11 @@
class Api::V2::DraftBlurbsController < Api::V2::BaseController
def index
if stale? :etag => current_project.etag
- render :json => current_project.draft_json
+ if params[:hierarchy]
@kareemk Owner
kareemk added a note

shouldn't the be params[:format] == "hierarchy", isn't that clearer and more flexible

@theycallmeswift Collaborator

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Gemfile
@@ -24,10 +24,11 @@ end
group :test do
gem 'bourne', '1.1.1'
gem 'capybara-webkit', '0.3.0'
- gem 'cucumber-rails', '0.4.1'
+ gem 'cucumber-rails'

add the version back, was seeing an error originally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kareemk kareemk commented on the diff
spec/models/blurb_spec.rb
@@ -32,25 +32,66 @@
one = Factory(:blurb, :key => 'test.one', :project => project)
two = Factory(:blurb, :key => 'test.two', :project => project)
- Factory :localization, :blurb => one, :locale => en,
- :draft_content => 'draft one', :published_content => 'published one'
- Factory :localization, :blurb => two, :locale => en,
- :draft_content => 'draft two', :published_content => 'published two'
- Factory :localization, :blurb => two, :locale => fr,
- :draft_content => 'ébauche', :published_content => 'publié'
+ Factory :localization, :blurb => one,
@kareemk Owner
kareemk added a note

alignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kareemk
Owner

Please detail the reason for this change in the pull request. Otherwise LGTM.

app/controllers/api/v2/draft_blurbs_controller.rb
@@ -1,7 +1,11 @@
class Api::V2::DraftBlurbsController < Api::V2::BaseController
def index
if stale? :etag => current_project.etag
- render :json => current_project.draft_json
+ if params[:hierarchy]
+ render :json => current_project.draft_json(:hierarchy => true)
@theycallmeswift Collaborator

:format => :hierarchy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
app/controllers/api/v2/published_blurbs_controller.rb
@@ -1,7 +1,11 @@
class Api::V2::PublishedBlurbsController < Api::V2::BaseController
def index
if stale? :etag => current_project.etag
- render :json => current_project.published_json
+ if params[:hierarchy]
@theycallmeswift Collaborator

same as above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@theycallmeswift theycallmeswift commented on the diff
app/models/blurb.rb
((8 lines not shown))
key = [locale_key, blurb_key].join(".")
result.update key => content
end
+
+ hierarchichal_data = blurbs.inject({}) do |result, (blurb_key, locale_key, content)|
+ keys = []
+ keys = blurb_key.split('.') if blurb_key
+ result.deep_merge!({ locale_key => create_hierarchichal_hash_from_array(keys + [content]) })
+ end
+
+ { :data => data, :hierarchichal_data => hierarchichal_data }
@theycallmeswift Collaborator

why do you have to send back both? shouldn't this only come back as one or the other?

Swift,

The only method that calls this is the Project#generate_json which is in turn used by updated_caches, so if you follow that flow you will see that it takes the data that is returned from this method and stores it in the cache. When we cache we want to cache both versions. We do not need to split out that path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kareemk
Owner

Need to update docs

...te/20120315145922_add_hierarchy_text_to_text_cache.rb
@@ -0,0 +1,11 @@
+class AddHierarchyTextToTextCache < ActiveRecord::Migration
+ def up
+ change_table :text_caches do |t|
+ t.text :hierarchichal_data
+ end
+ end
+
+ def down
+ remove_column :text_cahces, :hierarchichal_data
@kareemk Owner
kareemk added a note

typo

@croaky Owner
croaky added a note

Stylistically, I also like to see add_column when there is only one column changing and change_table do..end for multiple columns. Same style guideline as { ... } for single-line blocks and do...end for multi-line blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@theycallmeswift
Collaborator

lgtm

@cmavromoustakos

If everyone is ok with this can I get at least one more person to give it a lgtm so I can merge please.

@croaky
Owner

lgtm

@cmavromoustakos

Deployed with hash commit 8a753c0

@cmavromoustakos cmavromoustakos referenced this pull request from a commit
@cmavromoustakos cmavromoustakos [#27] Download translations with a hierarchy
The reason for this change is that most Javascript I18n plugins expect
the data in the same hierarchy that it is stored on as the locale yaml
files.

Currently, we have to come up with a conversion to store and retrieve
our keys differently to match the key to the translation. With this
change we allow the flexibility of allowing the end user (CopyCopter
client or some other javascript client) to pass a optional argument
formart with a type of hierarchy to get the data in the format of:

  {
    "en": {
      "test": {
        "one": "expected one",
        "two": "expected two"
      }
    }
  }
895499a
@croaky croaky referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@croaky croaky referenced this pull request from a commit
@croaky croaky [#27] Use host in install instructions
* Determine it programmatically instead of statically.
b543a2a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
17 Gemfile.lock
@@ -30,6 +30,7 @@ GEM
activesupport (3.1.1)
multi_json (~> 1.0)
addressable (2.2.7)
+ archive-tar-minitar (0.5.2)
arel (2.2.3)
bourbon (1.4.0)
sass (>= 3.1)
@@ -50,6 +51,7 @@ GEM
celerity (0.9.2)
childprocess (0.3.1)
ffi (~> 1.0.6)
+ columnize (0.3.6)
cucumber (1.1.9)
builder (>= 2.1.2)
diff-lcs (>= 1.1.2)
@@ -61,7 +63,7 @@ GEM
nokogiri (>= 1.4.4)
rack-test (>= 0.5.7)
culerity (0.2.15)
- daemons (1.1.4)
+ daemons (1.1.8)
database_cleaner (0.7.1)
diff-lcs (1.1.3)
dynamic_form (1.1.4)
@@ -91,6 +93,8 @@ GEM
json (1.6.5)
launchy (2.0.5)
addressable (~> 2.2.6)
+ linecache19 (0.5.12)
+ ruby_core_source (>= 0.1.4)
mail (2.3.3)
i18n (>= 0.4.0)
mime-types (~> 1.16)
@@ -143,6 +147,16 @@ GEM
activesupport (>= 3.0)
railties (>= 3.0)
rspec (~> 2.8.0)
+ ruby-debug-base19 (0.11.25)
+ columnize (>= 0.3.1)
+ linecache19 (>= 0.5.11)
+ ruby_core_source (>= 0.1.4)
+ ruby-debug19 (0.11.6)
+ columnize (>= 0.3.1)
+ linecache19 (>= 0.5.11)
+ ruby-debug-base19 (>= 0.11.19)
+ ruby_core_source (0.1.5)
+ archive-tar-minitar (>= 0.5.2)
rubyzip (0.9.6.1)
sass (3.1.15)
selenium-webdriver (2.20.0)
@@ -193,6 +207,7 @@ DEPENDENCIES
pg (= 0.13.2)
rails (= 3.1.1)
rspec-rails (= 2.8.1)
+ ruby-debug19 (= 0.11.6)
shoulda-matchers (= 1.0.0)
spork (= 0.9.0)
thin (= 1.3.1)
View
6 app/controllers/api/v2/draft_blurbs_controller.rb
@@ -1,7 +1,11 @@
class Api::V2::DraftBlurbsController < Api::V2::BaseController
def index
if stale? :etag => current_project.etag
- render :json => current_project.draft_json
+ if params[:format] == "hierarchy"
+ render :json => current_project.draft_json(:hierarchy => true)
+ else
+ render :json => current_project.draft_json
+ end
end
end
View
6 app/controllers/api/v2/published_blurbs_controller.rb
@@ -1,7 +1,11 @@
class Api::V2::PublishedBlurbsController < Api::V2::BaseController
def index
if stale? :etag => current_project.etag
- render :json => current_project.published_json
+ if params[:format] == "hierarchy"
+ render :json => current_project.published_json(:hierarchy => true)
+ else
+ render :json => current_project.published_json
+ end
end
end
end
View
31 app/models/blurb.rb
@@ -20,10 +20,20 @@ def self.ordered
def self.to_hash(attribute)
scope = joins(:localizations => :locale).
select("blurbs.key AS blurb_key, locales.key AS locale_key, #{attribute} AS content")
- connection.select_rows(scope.to_sql).inject({}) do |result, (blurb_key, locale_key, content)|
+ blurbs = connection.select_rows(scope.to_sql)
+
+ data = blurbs.inject({}) do |result, (blurb_key, locale_key, content)|
key = [locale_key, blurb_key].join(".")
result.update key => content
end
+
+ hierarchichal_data = blurbs.inject({}) do |result, (blurb_key, locale_key, content)|
+ keys = []
+ keys = blurb_key.split('.') if blurb_key
+ result.deep_merge!({ locale_key => create_hierarchichal_hash_from_array(keys + [content]) })
+ end
+
+ { :data => data, :hierarchichal_data => hierarchichal_data }
@theycallmeswift Collaborator

why do you have to send back both? shouldn't this only come back as one or the other?

Swift,

The only method that calls this is the Project#generate_json which is in turn used by updated_caches, so if you follow that flow you will see that it takes the data that is returned from this method and stores it in the cache. When we cache we want to cache both versions. We do not need to split out that path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
def self.keys
@@ -31,6 +41,25 @@ def self.keys
end
private
+ def self.create_hierarchichal_hash_from_array(array_hierarchy, hash_hierarchy = {})
+ return hash_hierarchy if array_hierarchy.empty?
+
+ # The last 2 values in the array are the most drilled down part, so given:
+ # [d,c,b,a,1]
+ # The first Iteration you would get:
+ # { "a" => 1 }
+ # Second iteration:
+ # { "b" => { "a" => 1 } }, etc..
+ #
+ if hash_hierarchy.empty?
+ value = array_hierarchy.pop
+ hash_hierarchy.merge!(array_hierarchy.pop => value)
+ else
+ hash_hierarchy = { array_hierarchy.pop => hash_hierarchy }
+ end
+
+ return create_hierarchichal_hash_from_array(array_hierarchy, hash_hierarchy)
+ end
def update_project_caches
project.update_caches
View
25 app/models/project.rb
@@ -51,8 +51,12 @@ def deploy!
update_caches
end
- def draft_json
- draft_cache.data
+ def draft_json(options = { :hierarchy => false })
+ if options[:hierarchy]
+ draft_cache.hierarchichal_data
+ else
+ draft_cache.data
+ end
end
def etag
@@ -71,8 +75,12 @@ def lock_key_for_creating_defaults
"project-#{id}-create-defaults"
end
- def published_json
- published_cache.data
+ def published_json(options = { :hierarchy => false })
+ if options[:hierarchy]
+ published_cache.hierarchichal_data
+ else
+ published_cache.data
+ end
end
def self.regenerate_caches
@@ -82,8 +90,8 @@ def self.regenerate_caches
end
def update_caches
- draft_cache.update_attributes! :data => generate_json(:draft_content)
- published_cache.update_attributes! :data => generate_json(:published_content)
+ draft_cache.update_attributes!(generate_json(:draft_content))
+ published_cache.update_attributes!(generate_json(:published_content))
touch
end
@@ -111,6 +119,9 @@ def generate_api_key
end
def generate_json(content)
- Yajl::Encoder.encode blurbs.to_hash(content)
+ blurbs_hash = blurbs.to_hash(content)
+ blurbs_hash[:data] = Yajl::Encoder.encode blurbs_hash[:data]
+ blurbs_hash[:hierarchichal_data] = Yajl::Encoder.encode blurbs_hash[:hierarchichal_data]
+ blurbs_hash
end
end
View
9 db/migrate/20120315145922_add_hierarchy_text_to_text_cache.rb
@@ -0,0 +1,9 @@
+class AddHierarchyTextToTextCache < ActiveRecord::Migration
+ def up
+ add_column :text_caches, :hierarchichal_data, :text
+ end
+
+ def down
+ remove_column :text_caches, :hierarchichal_data
+ end
+end
View
38 features/api_v2_download.feature
@@ -54,3 +54,41 @@ Feature: Download blurbs for a project through API
Then I should receive the following as a JSON object:
| en.test.one | update |
+ Scenario: download published blurbs with a json hieratchy for a known project
+ Given a project exists with a name of "Breakfast"
+ When I POST the v2 API URI for "Breakfast" draft blurbs:
+ | en.test.one | expected one |
+ | en.test.two | expected two |
+ And I POST the v2 API URI for "Breakfast" deploys
+ When I GET the v2 API URI for "Breakfast" published blurbs with a hierarchy param
+ Then I should receive a HTTP 200
+ And I should receive the following JSON response:
+ """
+ {
+ "en": {
+ "test": {
+ "one": "expected one",
+ "two": "expected two"
+ }
+ }
+ }
+ """
+
+ Scenario: download draft blurbs with a json hierarchy for a known project
+ Given a project exists with a name of "Breakfast"
+ When I POST the v2 API URI for "Breakfast" draft blurbs:
+ | en.test.one | expected one |
+ | en.test.two | expected two |
+ And I GET the v2 API URI for "Breakfast" draft blurbs with a hierarchy param
+ Then I should receive a HTTP 200
+ And I should receive the following JSON response:
+ """
+ {
+ "en": {
+ "test": {
+ "one": "expected one",
+ "two": "expected two"
+ }
+ }
+ }
+ """
View
21 features/step_definitions/api_v2_steps.rb
@@ -1,11 +1,19 @@
-When /^I GET the v2 API URI for "([^"]*)" draft blurbs$/ do |project_name|
+When /^I GET the v2 API URI for "([^"]*)" draft blurbs( with a hierarchy param)?$/ do |project_name, hierarchy|
project = Project.find_by_name!(project_name)
- get_with_etag "/api/v2/projects/#{project.api_key}/draft_blurbs"
+ if hierarchy
+ get_with_etag "/api/v2/projects/#{project.api_key}/draft_blurbs?format=hierarchy"
+ else
+ get_with_etag "/api/v2/projects/#{project.api_key}/draft_blurbs"
+ end
end
-When /^I GET the v2 API URI for "([^"]*)" published blurbs$/ do |project_name|
+When /^I GET the v2 API URI for "([^"]*)" published blurbs( with a hierarchy param)?$/ do |project_name, hierarchy|
project = Project.find_by_name!(project_name)
- get_with_etag "/api/v2/projects/#{project.api_key}/published_blurbs"
+ if hierarchy
+ get_with_etag "/api/v2/projects/#{project.api_key}/published_blurbs?format=hierarchy"
+ else
+ get_with_etag "/api/v2/projects/#{project.api_key}/published_blurbs"
+ end
end
When /^I GET the v2 API URI for "([^"]*)" published blurbs twice$/ do |project_name|
@@ -46,3 +54,8 @@
actual_result = Yajl::Parser.parse(page.source)
actual_result.should == expected_hash
end
+
+Then /^I should receive the following JSON response:$/ do |string|
+ actual_result = JSON.parse(page.source)
+ actual_result.should == JSON.parse(string)
+end
View
65 spec/models/blurb_spec.rb
@@ -32,25 +32,66 @@
one = Factory(:blurb, :key => 'test.one', :project => project)
two = Factory(:blurb, :key => 'test.two', :project => project)
- Factory :localization, :blurb => one, :locale => en,
- :draft_content => 'draft one', :published_content => 'published one'
- Factory :localization, :blurb => two, :locale => en,
- :draft_content => 'draft two', :published_content => 'published two'
- Factory :localization, :blurb => two, :locale => fr,
- :draft_content => 'ébauche', :published_content => 'publié'
+ Factory :localization, :blurb => one,
@kareemk Owner
kareemk added a note

alignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ :locale => en,
+ :draft_content => 'draft one',
+ :published_content => 'published one'
+ Factory :localization, :blurb => two,
+ :locale => en,
+ :draft_content => 'draft two',
+ :published_content => 'published two'
+ Factory :localization, :blurb => two,
+ :locale => fr,
+ :draft_content => 'ébauche',
+ :published_content => 'publié'
end
it 'returns draft hash' do
- Blurb.to_hash(:draft_content).should == {
- 'en.test.one' => 'draft one', 'en.test.two' => 'draft two',
+ Blurb.to_hash(:draft_content).should include(:data => {
+ 'en.test.one' => 'draft one',
+ 'en.test.two' => 'draft two',
'fr.test.two' => 'ébauche'
- }
+ })
end
it 'returns published hash' do
- Blurb.to_hash(:published_content).should == {
- 'en.test.one' => 'published one', 'en.test.two' => 'published two',
+ Blurb.to_hash(:published_content).should include(:data => {
+ 'en.test.one' => 'published one',
+ 'en.test.two' => 'published two',
'fr.test.two' => 'publié'
- }
+ })
+ end
+
+ it 'returns a draft hash maintaining hierarchy' do
+ Blurb.to_hash(:draft_content).should include(:hierarchichal_data => {
+ 'en' => {
+ 'test' => {
+ 'one' => 'draft one',
+ 'two' => 'draft two'
+ }
+ },
+ 'fr' => {
+ 'test' => {
+ 'two' => 'ébauche'
+ }
+ }
+ })
+ end
+
+ it 'returns a published hash maintaining hierarchy' do
+ Blurb.to_hash(:published_content).should include(:hierarchichal_data => {
+ 'en' => {
+ 'test' => {
+ 'one' => 'published one',
+ 'two' => 'published two'
+ }
+ },
+ 'fr' => {
+ 'test' => {
+ 'two' => 'publié'
+ }
+ }
+ })
+
end
end
View
27 spec/models/default_creator_spec.rb
@@ -15,26 +15,25 @@ def draft_hash
it 'sets draft content for a list of blurbs' do
locale = project.locales.first
one = Factory(:blurb, :project => project, :key => 'test.one')
- Factory :localization, :blurb => one,
- :locale => locale,
- :draft_content => 'draft one',
- :published_content => 'published one'
+ Factory :localization, :blurb => one,
+ :locale => locale,
+ :draft_content => 'draft one',
+ :published_content => 'published one'
+
two = Factory :blurb, :project => project, :key => 'test.two'
- Factory :localization, :blurb => two,
- :locale => locale,
- :draft_content => 'draft two',
- :published_content => 'published two'
+ Factory :localization, :blurb => two,
+ :locale => locale,
+ :draft_content => 'draft two',
+ :published_content => 'published two'
+
create_defaults 'en.test.one' => 'new one', 'en.test.three' => 'new three'
- project.localizations(true).map(&:draft_content).
- should =~ ['draft one', 'draft two', 'new three']
- project.localizations.map(&:published_content).
- should =~ ['', 'published one', 'published two']
+ project.localizations(true).map(&:draft_content).should =~ ['draft one', 'draft two', 'new three']
+ project.localizations.map(&:published_content).should =~ ['', 'published one', 'published two']
end
it 'ignores blank keys' do
create_defaults 'en.test.one' => 'not blank', '' => 'blank'
- project.localizations(true).map(&:draft_content).
- should =~ ['not blank']
+ project.localizations(true).map(&:draft_content).should =~ ['not blank']
end
it "only updates the project once when creating several defaults" do
View
37 spec/models/project_spec.rb
@@ -118,12 +118,24 @@
end
describe Project, 'draft_json' do
- it 'returns draft hash' do
- project = Factory(:project)
- project.create_defaults('en.test.key' => 'value')
+ let(:project) { Factory(:project) }
+
+ before { project.create_defaults('en.test.key' => 'value') }
+ it 'returns draft hash' do
project.reload.draft_json.should == Yajl::Encoder.encode('en.test.key' => 'value')
end
+
+ it 'returns draft hash with hierarchy' do
+ hierarchichal_representation = {
+ 'en' => {
+ 'test' => {
+ 'key' => 'value'
+ }
+ }
+ }
+ project.reload.draft_json(:hierarchy => true).should == Yajl::Encoder.encode(hierarchichal_representation)
+ end
end
describe Project, 'etag' do
@@ -205,15 +217,30 @@
end
describe Project, 'published_json' do
- it 'returns published hash' do
- project = Factory(:project)
+ let(:project) { Factory(:project) }
+
+ before do
project.create_defaults('en.test.key' => 'value')
project.deploy!
project.blurbs.first.localizations.first.revise(:content => 'new value',
:published => false).save!
+ end
+ it 'returns published hash' do
project.reload.published_json.should == Yajl::Encoder.encode('en.test.key' => 'value')
end
+
+ it 'returns draft hash with hierarchy' do
+ hierarchichal_representation = {
+ 'en' => {
+ 'test' => {
+ 'key' => 'value'
+ }
+ }
+ }
+ project.reload.published_json(:hierarchy => true).should == Yajl::Encoder.encode(hierarchichal_representation)
+ end
+
end
describe Project, '.regenerate_caches' do
Something went wrong with that request. Please try again.