TechAndCheck / tech-and-check-alerts

Daily tip sheet for fact checkers
MIT License
13 stars 6 forks source link

Implement dynamic newsletters #360

Closed slifty closed 4 years ago

slifty commented 4 years ago

As per #359 we want to move to a model where newsletters content and distribution is driven by the database, not by constants and implementation.

This means the NationalNewsletter and NorthCarolinaNewsletter obejcts would be replaced with a single Newsletter class.

It is unclear to me yet if we want to have an AbstractNewsletter at all, or if we just want the Newsletter to be its own class. This should be determined as part of the implementation of this issue.

slifty commented 4 years ago

@reefdog I can't decide... should I create a seed file that populates with the national + NC newsletters as part of this issue? Or should that be completely up to the deployment to set up.

That would probably be an abuse of seed data since it isn't actually universal to anybody who might want to run this project.

OK I've convinced myself nevermind!

slifty commented 4 years ago

With dynamic newsletters we have a naming clash -- we'll have a Newsletter model (sequelize) but also the concept of the Newsletter (the code that actually uses the model data to construct and send the newsletter via templates / etc)

I don't think we can (or should) have the model do that work. Should the "Newsletter" we had before actually be called a NewsletterController, or maybe NewsletterGenerator / MailingGenerator / EpisodeGenerator (what do we call a specific instance of a newsletter email...)

reefdog commented 4 years ago

what do we call a specific instance of a newsletter email

We don't have a name yet. I'd recommend "Issue".

Given that, perhaps NewsletterIssueGenerator?

slifty commented 4 years ago

That's perfect!

slifty commented 4 years ago

I'm thinking of a slight restructure to newsletter code and want to run it by the team before implementing.

Right now we have

- queues
- newsletters
- workers

inside newsletters we have the code for generating / sending newsletters, and templates.

Wondering if we should move to:

- queues
- templates
- workers
| - newsletterIssueGenerators (or newsletterGenerators)

This fits the pattern of "business logic in workers directory" while still keeping template data in its own folder. Thoughts?

slifty commented 4 years ago

So for making the template dynamic I'm doing the following:

  1. Template (and text template) name are both DB fields, so the template itself is still handlebars files and in theory we could have all newsletters use the same template. If we have an edge case down the line we're able to fulfill it by creating a new handlebars template (dev level change)

  2. There is a new DB field that stores a JSON object of "template parameters" -- which means we can have templates that pull fields from the database.

  3. I found a markdown helper which I think I'd like to use.

So now I need to decide if the "default" dynamic template has the footer boilerplate hard coded, or if the footer for a template should be stored in DB + markdown and passed to the template using the DB template parameters field.

reefdog commented 4 years ago

I'm thinking of a slight restructure to newsletter code and want to run it by the team before implementing.

Go team go!

I think this restructuring is great. It didn't sit well with me that newsletter was a sibling to queues and workers, even though I did it, and I think your restructure reflects a more coherent taxonomy.

So for making the template dynamic I'm doing the following…

I like it all!

So now I need to decide…

I decided to err on the side of giving the copywriters maximum flexibility here. I didn't want to pressure them to make language overly-generic or comprehensively verbose just to allow more boilerplate re-use. So I believe that argues for not hard-coding it into the template, even if current variations between newsletters are minimal.

Any non-"copy" snippets like compliance/provenance/identification could (and in fact should, to minimize getting out of date) be hard-coded.

slifty commented 4 years ago

Thanks @reefdog I'll go with that.