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

Leave unknown language as nil if account is remote #8861

Merged
merged 2 commits into from
Oct 5, 2018
Merged

Leave unknown language as nil if account is remote #8861

merged 2 commits into from
Oct 5, 2018

Conversation

tribela
Copy link
Contributor

@tribela tribela commented Oct 3, 2018

It is not reasonable to use Local instance's language setting for foreign user's toot.

@Quenty31
Copy link
Contributor

Quenty31 commented Oct 3, 2018

I was thinking of it this very morning!

@tribela
Copy link
Contributor Author

tribela commented Oct 3, 2018

Can someone help me to fix test code?

  1) LanguageDetector detect when language can't be detected remote user force use detector
     Failure/Error: let(:account_remote) { Fabricate(:user, domain: 'joinmastodon.org').account }
     
     ActiveModel::UnknownAttributeError:
       unknown attribute 'domain' for User.
     # ./spec/lib/language_detector_spec.rb:45:in `block (3 levels) in <top (required)>'
     # ./spec/lib/language_detector_spec.rb:111:in `block (5 levels) in <top (required)>'

@ClearlyClaire
Copy link
Contributor

I don't think we should use the language detector's result when it's unreliable. Maybe it's best to leave this field as nil?

@tribela
Copy link
Contributor Author

tribela commented Oct 3, 2018

When language field is nil, Is it never shown to user who filtered some languages?

@Gargron
Copy link
Member

Gargron commented Oct 3, 2018

The language detector is super inaccurate for short messages. Also, you can't ignore when people set a posting language preference (which is hopefully more correct than the detector, although vandalism is also possible)

@tribela
Copy link
Contributor Author

tribela commented Oct 3, 2018

So.. It's better to set nil on foreign toot which not having language field?

@tribela tribela changed the title Force use language detector if account is remote Leave unknown language as nil if account is remote Oct 3, 2018
@ClearlyClaire
Copy link
Contributor

@Kjwon15 When a status' language is set to nil, it is always shown.

@Gargron afaict this change doesn't ignore the posting language setting, it just drops the fallback to default user / instance setting on receiving remote toots for which a language wasn't defined and for which the detection has been deemed unreliable.

@tribela
Copy link
Contributor Author

tribela commented Oct 5, 2018

Yes, It actually fixes a bug

@Gargron Gargron merged commit 144d737 into mastodon:master Oct 5, 2018
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 this pull request may close these issues.

4 participants