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

1.3.0.rc1 Can't assign an translated attribute with a column fallback #635

Open
ScotterC opened this issue Mar 6, 2024 · 2 comments
Open

Comments

@ScotterC
Copy link

ScotterC commented Mar 6, 2024

I'm just getting started with Mobility. I found that to use the container strategy on postgres and preserve my existing columns it was best to use 1.3.0.rc1. Rails 7.1.3

Context

  plugins do
    backend :jsonb
    active_record
    column_fallback true
  end

My Document model has an existing title column with a default locale of english.

# Migration
add_column :documents, :translations, :jsonb, default: {}

class Document < ApplicationRecord
  extend Mobility
  translates :title
end

Expected Behavior

Document.last.title #=> "Document Title in fallback column"
 title_es = "hablas espagnol"
  Mobility.with_locale(:es) {
    document.title = title_es
    puts document.title
  } #=>  "hablas espagnol"

and to have title_es stored in the translations column of the Document record.

Actual Behavior

Here's my test script to reproduce

  document = Document.last
  puts document.title
  title_es = "hablas espagnol"
  Mobility.with_locale(:es) {
    document.title = title_es
    puts document.title
  }
# => mobility-1.3.0.rc1/lib/mobility/backends/active_record/pg_hash.rb:26:in `[]=': string not matched (IndexError)

Possible Fix

Spelunking in pg_hash, here's what I find

# ...
        def write(locale, value, _options = nil)
          if value.nil?
            translations.delete(locale.to_s)
          else
            translations[locale.to_s] = value
          end
        end
# ...
        def translations
          model[column_name]
        end
# ...

translations[locale.to_s] = value has an IndexError because translations is returning the value of the column fallback :title attribute.

translations #=> "Document Title in fallback column"
column_name #=> "title"
model.translations #=> {}

A very naive fix would be to change translations but I know there's a lot more going on here.

@ScotterC
Copy link
Author

ScotterC commented Mar 7, 2024

The good news for me is I should be using :container backend and not :jsonb backend which resolves my issue.
I'm not 100% sure but it seems that the above still holds true for a :jsonb backend with a column_fallback true plugin.

This failing test reveals the issue which could be added if this is intended behavior.

# spec/mobility/plugins/active_record/column_fallback_spec.rb
  context "column_fallback: true with jsonb backend" do
    plugin_setup :slug, column_fallback: true
    
    before do
      translates model_class, :slug, backend: :jsonb, column_fallback: true
    end

    it "reads/writes from/to backend if locale is not I18n.default_locale" do
      instance.send(:write_attribute, :slug, "foo")

      Mobility.with_locale(:fr) do
        expect(listener).to receive(:read).with(:fr, any_args).and_return("bar")
        expect(instance.slug).to eq("bar")

        expect(listener).to receive(:write).with(:fr, "baz", any_args)
        instance.slug = "baz"
      end
    end
  end

@JacobAlexander
Copy link

JacobAlexander commented Jun 24, 2024

Correct me but in documentation jsonb column for for your
translates :title
should be added as
add_column(:documents, :title, :jsonb, default: {})
not the
add_column :documents, :translations, :jsonb, default: {}

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

2 participants