Open jace opened 2 years ago
Handlebars templating appears to be a comparable choice with Mustache for our needs. References:
However, our choice will depend on the better performing and better maintained option between the Python implementations – pybars3 (Handlebars) vs Chevron (Mustache) – and it appears Chevron is the better maintained choice here.
On templates: this decision shouldn't only be on the basis of implementations in Chevron vs pybars3. The template languages are a more important factor.
Mustache is great for enforcing logic-less usage, supporting only content insertion. But it can also become a limitation rather quickly. For instance, {#var}
is overloaded with three meanings: if
, for each
and with
. The conflation of if
with for
means that a non-empty list cannot be preceded with text introducing the list.
Handlebars is a better choice here, and we should aim to find a performant implementation as secondary criteria.
Counterpoint:
Change announcements as distinct Python classes in the filesystem may be a bad idea:
The better approach, therefore, is to replace announcements as described in this ticket with a messaging product. Perhaps:
@help
account.<account>@hasgeek.com
, but we'll have to deal with the pain of namespace separation from corporate email (maybe a distinct hasgeek.email TLD) and also email reputation risk from a single domain representing a diversity of users.The recipient CSV system used in Hasmail will need to become a nested JSON structure in this implementation because change announcements will need to use lists, for when multiple pieces of user data are affected and need to be enumerated.
The Notification system as introduced in #835 (originally defined in #605) does not host any content, by design. Instead it has a type defined as a Python class, a set of templates for various transports, and a reference document from which the notification finds target users, and the templates draw content. Due to this design, the notification system cannot be used to send one-off content to specific users. A source document must provide both. This ticket describes one such requirement, an announcement of change.
The app's functionality changes from time to time as we respond to product usage and business needs. These changes can affect a user's data and experience, and affected users must be notified. This has multiple parts that must be aligned:
The announcement must typically accompany the actual change in functionality, or must precede it by a notice period. Changes are usually introduced via a database migration, which is currently performed via Alembic. While migrations are technically meant for schema changes, we have also used it for data migrations in the past, and at the moment it serves us well as the framework to build this on.
Changes are one-off events. The code that performs the change does not need continuous maintenance, and is therefore expected to be (a) discardable and (b) isolated from the main code. Alembic enforces this convention by expecting each migration to be standalone, not dependent on imports for schema or logic. However, if we use the notification system to communicate the change, a corresponding artifact document must be long-lived and must supply the explanation of change. This necessitates splitting the concept of the announcement into two parts: a one-off migration that performs the change and records the affected users, and a database-hosted document that enumerates these users.
A third part may also be necessary. An announcement will need review before release, translation into supported languages, and formatting as per the expectations of each transport – rich HTML for web, simple HTML for email, simpler HTML for Telegram, plain text for SMS, etc. This requirement is identical to that of the notification rendering system, which defines multiple formats. The existing Babel-based notification system provides translation support, and code review is an established practice alongside the Git version control system. However, a notification renderer is meant for recurring use and sources its dynamic content from the document. Each announcement needs distinct content, but the infrastructure it needs is present in the source repository, not in the database. We can resolve this by creating a new type of view called a
RenderAnnouncement
, derived fromRenderNotification
, that works on a 1:1 mapping with theAnnouncement
database model. This is similar to how the Alembic version in the database corresponds to a migration file. TheAnnouncement
model only hosts a list of users, with no content. Keeping it simple will allow its table structure to be repeatedly re-defined in migrations, which must continue to be standalone (although we may allow for one shared import in this case). The view contains all the content but is not responsible for discovery of affected users. The migration does that.However, since announcements are one-off, their long life in the source repo also creates trouble. Should the class structure of
RenderAnnouncement
change, all existing announcement subclasses will also need revision even though the announcement is obsolete with no one accessing it. Should announcements proliferate, each of them will be loaded into memory as part of the app runtime. A similar problem exists for old migrations. Our periodic introduction of linters and code conventions has required maintenance of migrations that are no longer required. The solution may also be similar: delete them after they are deemed obsolete: the file, the db record, and the linked notifications (another migration!). This requires a repository maintenance process, but that can taken up in the future, and we may likely also evolve a mechanism to define such data migrations separately from schema migrations.Announcement translations present a similar problem: old announcements are unlikely to need translation into newly supported languages, but if the Babel system extracts all strings into a single
messages.pot
file, the translator does not get a prioritized list. We can address this by configuring Babel to use a separate POT file for announcement translations (a single file for all announcements to start with).If web templates for announcements continue to be specified in Jinja2 files, as is done for notification renderers, this too will likely create an unmaintainable situation with boilerplate markup in templates (
{% extends %}
at the top of each file,{% trans %}
around each paragraph inside the<p>
tag). We can instead standardize on Markdown files and either use Markdown+Jinja2, allowing a single{% trans%}
tag to wrap the entire content, or drop Jinja2 and use something like Mustache, which still allows enough logic to insert content from source documents. In this case, (a) the template can be a multi-line string inside the class, wrapped in gettext for Babel's benefit, or (b) template files can be specified with language tags, dropping Babel altogether. (Smaller formats like SMS are currently strings within the notification render classes and use Babel.)A change may affect multiple user documents. For instance, when the follow feature migration is introduced, the user will become a follower of multiple profiles, and the user must be presented this list. The
Announcement
database model cannot contain this data as it is restricted to a user list. TheRenderAnnouncement
view should not contain query logic beyond a stable API. Fortunately, theNotification
class records two sources: adocument
and afragment
, and consolidates multiple fragments into a single notification. The relationship between document and fragment is only defined in the notification renderer, and in this case can be in the announcement-specificRenderAnnouncement
class.Announcements may need to be staggered. Since they are for announcements of change, the change may be staggered, introduced to some users before all, and the notifications need to be sent out correspondingly. Fortunately, we have some of the pieces in place: the announcement as a long-term artifact will be present throughout the stagger period and beyond. The notifications system is already capable of not sending duplicate notifications. The dispatch of an announcement's notification must be understood as a distinct activity from the migration that discovered the users and added them to the announcement. This dispatch may be triggered either over the web or through a command line interface. Using the CLI will allow it to be performed right after the data migration. However, the staggering will also need to be performed as distinct database migrations, with (currently) no mechanism to define them together. If we are to introduce a shared import for announcement schema into the migrations system, we may also introduce a shared import for the staggered stages of a change. However, each stage will continue to be an independent migration as far as Alembic is concerned.
Announcements need preview during the development cycle. A schema migration has presumably already been performed, but a data migration has not, and so the underlying fragment will not be available to supply data for the templates. To afford preview, the announcement renderer must (a) include a dummy document, or (b) show preview against a reference document specified the user (simplest form: as a URL parameter with the UUID). This preview mechanism should be access restricted on the production website, or should use a settings flag to disable it entirely. Since data migrations are distinct from notification dispatch, it may also be possible to preview with live data between these stages, but this will only be available in a development environment, and bears the risk of accidentally triggering dispatch to non-developers. (The transport mechanisms should support an allow-list to limit recipients in dev environments.)