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

human_state cached across locales in 5.1.1 release #709

Closed
niciliketo opened this issue Sep 20, 2020 · 3 comments · Fixed by #716
Closed

human_state cached across locales in 5.1.1 release #709

niciliketo opened this issue Sep 20, 2020 · 3 comments · Fixed by #716

Comments

@niciliketo
Copy link

Describe the bug
The method aasm.human_state returns the state name using a translation. Once returned for one locale, the same text is subsequently returned for other locales too.

To Reproduce
Steps to reproduce the behaviour:

    it 'shows text in 2 different locales' do
      ep = described_class.new
      I18n.locale = :fr
      fr_text = ep.aasm.human_state
      I18n.locale = :de
      de_text = ep.aasm.human_state
      expect(fr_text).not_to eq(de_text)
    end

Expected behavior
A clear and concise description of what you expected to happen.
I would expect the above spec to pass
Screenshots
n/a

Additional context
I have only tested this in the context of our application.

@the-spectator
Copy link
Contributor

I got the reproduction script below:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "~> 6.0"
  gem "sqlite3"
  gem "aasm", '~> 5.1'
  gem "byebug"
end

require "byebug"
require "aasm"
require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :assets, force: true do |t|
    t.string :aasm_state
  end
end

class Asset < ActiveRecord::Base
  include AASM

  aasm do
    state :available, initial: true
    state :nonoperational, display: "non-operational"

    event :move do
      transitions from: :available, to: :nonoperational
    end
  end
end

class BugTest < Minitest::Test

  def setup
    # I18n.load_path << Dir[File.expand_path("locales") + "/*.yml"]
    I18n.backend.store_translations(:en, {
      activerecord: {
        attributes: {
          asset: {
            aasm_state: {
              available: "Lavailable"
            }
          }
        }
      }
    })
    I18n.backend.store_translations(:fr, {
      activerecord: {
        attributes: {
          asset: {
            aasm_state: {
              available: "Disponible"
            }
          }
        }
      }
    })
    I18n.default_locale = :en
  end


  def test_for_human_state
    asset = Asset.new

    I18n.locale = :en
    assert_equal "Lavailable", asset.aasm.human_state

    I18n.locale = :fr
    assert_equal "Disponible", asset.aasm.human_state
  end
end

@anilmaurya I am able to scope it down to

def display_name
@display_name ||= begin
if Module.const_defined?(:I18n)
localized_name
else
name.to_s.gsub(/_/, ' ').capitalize
end
end
end

This behavior is due to memoization. To my understanding, by removing it we can resolve the issue reported. Am I on right track?

@the-spectator
Copy link
Contributor

I would like to get clarification for the following scenario:

If :display option and localization for the state are present, then what should aasm#human_state should return? Localized value or the display value.

example:

class Asset < ActiveRecord::Base
  include AASM

  aasm do
    state :nonoperational, initial: true, display: "non-operational"
    state :available
  end
end

asset = Asset.new
asset.aasm.human_state #=> ?
# en.yml
en:
  activerecord:
    attributes:
      asset:
        aasm_state: 
          available: Lavailable
          nonoperational: Fr-Non-Operational


# fr.yml
fr:
  activerecord:
    attributes:
      asset:
        aasm_state: 
          available: Disponible
          nonoperational: Fr-Non-Operational

the-spectator added a commit to the-spectator/aasm that referenced this issue Oct 12, 2020
Remove memoization to avoid stale localized state name
@anilmaurya
Copy link
Member

anilmaurya commented Oct 13, 2020

Hi @the-spectator
Thank you for the reproduction script.

Ideally one should not use both display & localization at the same time.
If both are used then localization should take precedence over display option.

anilmaurya pushed a commit that referenced this issue Oct 15, 2020
Remove memoization to avoid stale localized state name
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 a pull request may close this issue.

3 participants