Automatically provide a language in diary entries factory

[pablobm]

When working on something the other day, I noticed that the default setting for the diary entries factory (create(:diary_entry)) requires that a Language record for the English language exists in DB.

Should this be simplified? I am providing an option that is perhaps slightly hacky, but I think it works well for the use it gets in this codebase.

[gravitystorm]

Can we use find_or_create_by here? It does side-step factory_bot entirely, but is perhaps easier to read.

[pablobm]

Hm, losing the factory feels a bit wrong. How about the following?

Suggested change

	language { Language.find_by(:code => "en") || create(:language, :code => "en") }
	language do
	Language.find_or_create_by(:code => "en") do |record|
	record.assign_attributes(attributes_for(:language))
	end
	end

Significantly more verbose though...

[tomhughes]

Personally, and after spending some time hunting through the factorybot documentation assuming that there must be a better way, it seems that @pablobm's original suggestion is probably best.

To be honest I'm struggling to understand how this has been working... Does factorybot really just associate to a random record if you don't tell it to do something else?

[pablobm]

It confused me too when I was looking into it. It's not FactoryBot, it's our SQL schema, which defaults to en:

    language_code character varying DEFAULT 'en'::character varying NOT NULL,

[tomhughes]

Possibly the tests in diary_entries_controller_test.rb should be included in this?

[pablobm]

Possibly the tests in diary_entries_controller_test.rb should be included in this?

I left that one out because it explicitly sets languages in some tests, so I felt like it made sense to allow the language being created separately. Eg: check out test_index_language or test_rss.

[tomhughes]

Yes I was aware of that but at least in some cases in might make sense but it can probably argued either way.

I think this is fine as far as it goes anyway, so there's no reason not to merge it now.