DEV: Move language detection out of the post serializer #193
Conversation
nattsw
force-pushed
the
lang-detection-on-post-event
branch
6 times, most recently
from
5909f4c to
4fe18db
Compare
Language detection is added to the queue every time the post is serialized, move this into the post_created and post_edited events instead.
nattsw
force-pushed
the
lang-detection-on-post-event
branch
from
4fe18db to
a5ccdb0
Compare
spec/lib/guardian_extension_spec.rb
Outdated
| let(:guardian) { Guardian.new(user) } | ||
|
|
||
| it "returns false when the post user is not in restrict_translation_by_poster_group" do | ||
| SiteSetting.restrict_translation_by_poster_group = "#{group.id + 1}" |
There was a problem hiding this comment.
Making artificial IDs can end up making flaky tests, we should either Fabricate(:group) here to get a new group or do group.remove(user) in this test
There was a problem hiding this comment.
Would it matter in this case if it is intentional that the group doesn't exist? Technically speaking, we don't update group-type site settings if a group is deleted.
There was a problem hiding this comment.
Yes it matters, I just know pretty much every time anyone's used an ID that wasn't from a DB model in specs, it has led to flakes especially in parallel testing environments. It's just safer to Fabricate a new one, as you've done now :)
spec/lib/guardian_extension_spec.rb
Outdated
| end | ||
|
|
||
| it "returns false when the post raw is empty" do | ||
| post.update(raw: "") |
There was a problem hiding this comment.
| post.update(raw: "") | |
| post.update!(raw: "") |
There was a problem hiding this comment.
Yeah this one needs to be not ! on purpose. I've updated the spec to make it clear
martin-brennan
left a comment
martin-brennan
left a comment
There was a problem hiding this comment.
Nice, much better than doing this in the serializer!
|
Thanks @martin-brennan! |
Language detection is added to the queue every time the post is serialized. We'll move this into the post_process_cooked event instead, since it is not a good idea to overload a serializer.
This is a follow-up from the move for queueing translations to redis here 8fce768.