fix(legal): fix rendering of legal pages when loaded directly via url

[zaach]

I refactored the template fetching so that we can use it from the route handler when rendering pages dynamically and from the grunt task when generating pages statically.

Fixes #1372.

@shane-tomlinson r?

[shane-tomlinson]

Since getTemplate returns a promise, could you do something like:

   return Promise.all([
     getTemplate('terms', lang, defaultLang),
     getTemplate('privacy', lang, defaultLang)
   ]).then(allTemplates) {
    templates[lang] = {
       terms: allTemplates[0],
       privacy: allTemplates[1]
    }
   }).then(...

It might be marginally faster since the runtime loop can continue while waiting for the filesystem?

[shane-tomlinson]

never mind, my idea blows up.

[zaach]

Really? It seems to work for me. Thanks for the suggestion!

[shane-tomlinson]

@zaach - Huurah! It works, but I am concerned at the amount of voodoo that is going on with our l10n build step, or at least lack of documented steps. Wanna put our heads together and come up with a legacy new developers can use to learn what is going on, or just as a refresher for 6 months from now when both of us are scratching our heads saying WTF?

[zaach]

@shane-tomlinson I added more comments! I began mapping out all of the flows in a diagram. It's not pretty, but I'll try to create a lucid chart next week.

[shane-tomlinson]

@zaach - Thanks for all the documentation. Now we have one final strangeness - requesting non-english locales always serves english privacy policy, even though the templates are clearly translated to to language. Here is German:

Rendered text

Template that should be served

Reproducible when running the server using grunt server:dist

[shane-tomlinson]

@zaach - I believe the above noted problem is because server/lib/localized-render replaces res.render with a function that renders the template for the locale specified in req.locale, which for me is set to en.

If I add:

            req.locale = i18n.localeFrom(lang);
            req.lang = lang;

just before

            // the HTML page removes the header to allow embedding.
            res.removeHeader('X-FRAME-OPTIONS');

everything works as expected. I'm not positive that's correct? Possibly.

[shane-tomlinson]

@zaach - another problem is the user receives a 500 error if browsing directly to a locale that does not exist:

This is noted in #1377

[shane-tomlinson]

@zaach - I have a proposal in #1377 for the failed to lookup view problem

[zaach]

@shane-tomlinson I believe your comment #1376 (comment) is issue #1337. I'm working on a PR for that which turns on abide's feature to recognize the lang from the URL.

[shane-tomlinson]

@zaach - hot.

[zaach]

@shane-tomlinson I'm just going to roll the fix for #1337 into this PR

[shane-tomlinson]

@zaach - do it!

[zaach]

These are now run as part of l10n-generate-pages, since it depends on them.

[zaach]

Fixes #1377.

[shane-tomlinson]

@zaach - still seeing some instances of strangeness.

1. I set the browser language to en and browsed to en_US:

2. If you browse to es_AR with en set as the browser language, similar error, but browsing to es-AR does not.

[zaach]

@shane-tomlinson Yep, I think we need to just remove en from the supported lang list (it's not present in production fwiw). Then it will default to en-US like other unsupported locales. To your second point, es_AR is unrecognized because of the underscore, so it follows the unrecognized languages path, which is working as intended.

[zaach]

Okay, I've removed en from the default supported languages array. The cases caught by @shane-tomlinson are working correctly but I'd like someone else to confirm.