Pull content for moratorium banner from Contentful. #2125
Conversation
| CONTENTFUL_SPACE_ID: str = "" | ||
|
|
||
| CONTENTFUL_ACCESS_TOKEN: str = "" |
There was a problem hiding this comment.
We should consider just using our actual space ID and access token as defaults here; because the access token is read-only and these credentials are intended for use in browser-side JS, they're effectively public (not secrets), and as such, just hard-coding them as defaults won't be a security hazard, and will also make it easier for developers to get up and running quickly.
There was a problem hiding this comment.
So this is kind of frustrating, I ended up adding the defaults to the WoW version of this PR, in JustFixNYC/who-owns-what#482, and immediately got this email:
ARGH. While I am pretty sure this isn't actually a security hole, I really, really wish it were possible to just tell Contentful to make a space publicly readable so we didn't have to go through this false alarm stuff.
| @@ -65,6 +66,7 @@ def create_initial_props_for_lambda( | |||
| "debug": settings.DEBUG, | |||
| "facebookAppId": settings.FACEBOOK_APP_ID, | |||
| "nycGeoSearchOrigin": settings.NYC_GEOSEARCH_ORIGIN, | |||
| "contentfulCommonStrings": contentful.get_common_strings(), | |||
There was a problem hiding this comment.
This could be bad, if contentful ever goes down. We don't cache anything when there's a network error, so if cdn.contentful.com happens to timeout, all requests to our server could take an unusually long time, effectively resulting in a denial of service.
That said, contentful does use a CDN so this request should be pretty darn fast and timeouts should be extremely rare. Lots of businesses depend on Contentful's CDN to be responsive so we could always just give this a shot and revisit if anything ever goes wrong.
There was a problem hiding this comment.
Cool! My understanding is that the org site already has infrastructure to pull from contentful, so that one would be easier to change over. So that would just leave us with WOW still pulling from react-common, and presumably we could do something similar with WOW.
Right, though there are a few wrinkles--more details at JustFixNYC/who-owns-what#482!
There was a problem hiding this comment.
Cool! My understanding is that the org site already has infrastructure to pull from contentful, so that one would be easier to change over. So that would just leave us with WOW still pulling from react-common, and presumably we could do something similar with WOW.
| @@ -65,6 +66,7 @@ def create_initial_props_for_lambda( | |||
| "debug": settings.DEBUG, | |||
| "facebookAppId": settings.FACEBOOK_APP_ID, | |||
| "nycGeoSearchOrigin": settings.NYC_GEOSEARCH_ORIGIN, | |||
| "contentfulCommonStrings": contentful.get_common_strings(), | |||
This factors out the common logic in JustFixNYC/tenants2#2125 and JustFixNYC/who-owns-what#482 into its own reusable package, `@justfixnyc/contentful-common-strings`.
This is analogous to JustFixNYC/tenants2#2125, but for our static sites (WoW's front-end is a static site). Note that pulling from Contentful for common strings for our static sites is a bit unusual because: * Not all our static sites use the same framework. For example, WoW uses Create React App, while our other sites use Gatsby. Each of these has their own particular way of accessing data from Contentful (WoW uses Contentful's API directly, while our other sites use a Gatsby plugin). * While all our static sites currently use Contentful in some capacity, they don't use the same Contentful _space_. The common strings are all stored in a separate Contentful space to keep things DRY. But our static sites are generally built assuming that there is only _one_ Contentful space to pull strings from (and even if they supported multiple spaces, which the Gatsby plugin does, we'd still have to write custom code for each framework). I've added a [`@justfixnyc/contentful-common-strings`](https://www.npmjs.com/package/@justfixnyc/contentful-common-strings) package which deals with these issues, and this PR now uses it. ## How it works 1. A command-line script is run to retrieve the Contentful common strings (see JustFixNYC/tenants2#2125 for more details) and write them out to a JSON file. This script is run whenever the site is built. 2. The front-end code loads the common strings JSON and renders it in a similar way to the tenant platform. Step 1 is the main place where the behavior of the tenant platform differs from that of our static sites: while the TP retrieves the strings dynamically and passes them on to the front-end during server-side rendering, our static sites will instead retrieve the content at build time. ## Why not do this in-browser? I decided to retrieve the strings at build time, rather than dynamically from the browser at page load time, for a few reasons: * If we fetched the strings dynamically using Contentful's CORS API, browsers that lack JS (or whose JS was broken for some reason) wouldn't be able to see the content. Since the common content we have right now is actually pretty important, it seemed more prudent to render it at build time. * Our static sites are already re-rendered whenever content changes, so this just follows the same pattern (well, we might need to manually re-run WoW's pulling of contentful data, but that one's not so bad because we don't change its contentful content very often). However, it should be noted that **we need to set up Webhooks on the Contentful common space** that trigger rebuilds of all our static sites whenever common strings are changed (we could alternatively manually trigger rebuilds, but it's likely that we'd forget to do this).
This is an attempt to try pulling content for the Covid moratorium banner text directly from Contentful, so that we don't need to push a change to this repository whenever we want to change the text of the banner.
It currently uses the new
@justfixnyc/contentful-common-stringspackage to help accomplish this.How it works
The tenant platform now optionally supports retrieving "common strings" from Contentful. By "common strings" we really mean "common rich text strings".
Each common string has an alphanumeric id and a value. The value is a localizable rich text value. For example, in this PR, the common string with id
covidMoratoriumBanneris expected to have a value containing the rich text to show users about the current state of the COVID Moratorium, localized in both English and Spanish.The platform retrieves all entries from its configured Contentful space tagged
common. Each of these entries is expected to have a short text field calledidand a localized rich text field calledvalue. For reference, here's what such a model looks like in the Contentful content model UI:The common strings are made available to front-end code,. A helper function on the front-end makes it convenient to retrieve a common string's rich text document representation by its id. The front-end can then render them using the
@contentful/rich-text-react-rendererpackage.Note that at present, all locales of the common strings are passed to the front-end, which means that there's a slight amount of bloat--but it shouldn't be too bad since we have very few common strings.
The back-end caches the common strings from Contentful for a short period of time--at the time of this writing, it's 5 seconds--to ensure minimal latency during periods of high load. (Note that the back-end is responsible for retrieving the Contentful strings to ensure that they are properly rendered during server-side rendering.)
To do
Consider spinning out some of the front-end code into a separate module under theSee Add contentful-common-strings package. justfix-ts#32.@justfixnycnamespace so our static sites can use it too (see Pull content for moratorium banner from Contentful. who-owns-what#482 for more details)