Initial room_type work, MSC3288 implementation#375
Conversation
clokep
left a comment
There was a problem hiding this comment.
Unfortunately due to the l10n concern I think we need to think through this a bit more.
There might be other ways to solve it, but I gave a suggestion on the comment.
| substitutions["room_type_name"] = ( | ||
| "space" if substitutions["room_type"] == "m.space" else "room" | ||
| ) |
There was a problem hiding this comment.
The issue with this approach is that it is not localizable -- if someone updates their templates to be in a different language the replacements would still be in English here.
I think the solution to this is to replace our home-grown templates with Jinja2 templates (probably in a separate PR). It may or may not be feasible to continue supporting the current templates as an upgrade path. Alternately we could require updating the templates at the same time.
Then we'll want to rebase this PR and do the logic of whether to include space or room in the template itself.
|
Could I get a second pair of eyes on this since I've now done a bunch of work to it? |
reivilibre
left a comment
There was a problem hiding this comment.
Is it worth calling out (in the changelog?) that only Jinja2 templates can properly customise themselves for room types / spaces?
| # MSC3288 | ||
| substitutions["room_type"] = substitutions.pop( | ||
| "org.matrix.msc3288.room_type", "" | ||
| ) |
There was a problem hiding this comment.
I'm guessing these lines will go away when we move out of unstable
There was a problem hiding this comment.
Something like that. We'll probably have Synapse send both for a while and update Sydent to use just room_type? Or fallback to org.matrix.msc3288.room_type. 🤷
Related matrix-org/synapse#10435
Implementation of matrix-org/matrix-spec-proposals#3288
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.