FEATURE: Translate categories and tags #269
Conversation
nattsw
force-pushed
the
category-translations
branch
from
422a3ed to
0e8c3e2
Compare
nattsw
changed the title
| AND m.user_id > 0 | ||
| #{max_age_clause} | ||
| WHERE m.#{content_column} != '' | ||
| #{not_deleted_clause(model)} |
There was a problem hiding this comment.
Unlike posts and topics, categories and tags do not have a deleted_at nor can be created by a bot
| [ | ||
| [Topic, "title"], | ||
| [Post, "raw"], | ||
| [Category, "name"], |
There was a problem hiding this comment.
There is Category#description too. Is that something we want to consider in this PR too?
There was a problem hiding this comment.
Not yet. Category descriptions are currently topics so we will ignore for now.
| #{not_deleted_clause(model)} | ||
| #{non_bot_clause(model)} | ||
| #{max_age_clause(model)} |
There was a problem hiding this comment.
I'm wondering if it makes sense to just make this backfill_clause(model) or something instead of having the logic of model == Post || model == Topic present over 3 methods. This also makes the complete query much easier to read/understand IMO instead of having 3 parts to combine mentally.
There was a problem hiding this comment.
Actually the current query looks very neat instead of having to repeat the clause for every model
SELECT m.id
FROM #{model.table_name} m
#{limit_to_public_clause(model)}
WHERE m.#{content_column} != ''
#{not_deleted_clause(model)}
#{non_bot_clause(model)}
#{max_age_clause(model)}
ORDER BY m.updated_at DESC
I will prefer optimizing next time if it gets more complex.
| # frozen_string_literal: true | ||
|
|
||
| module DiscourseTranslator | ||
| class CategoryTranslation < ActiveRecord::Base |
There was a problem hiding this comment.
Do we need to differentiate between Category#name and Category#description? It might be useful to think about how we can expand this table to store the translation for more than a single column since some models may have multiple columns that requires us to translate.
There was a problem hiding this comment.
No.
This current implementation makes use of the translatable concern. Category descriptions point to a topic so it will use topic translations instead.
Adding categories and tags into the fray.
This PR only adds translations for the categories and tags, without displaying them yet. That will come in a separate PR as we figure out caching and appropriate APIs in core.