Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sharing the Hearing on facebook does not display Hearing image and description in the link. #317

Open
Rikuoja opened this issue Apr 26, 2017 · 5 comments

Comments

@Rikuoja
Copy link
Contributor

Rikuoja commented Apr 26, 2017

No description provided.

@akx
Copy link
Contributor

akx commented Oct 25, 2017

As discussed in Slack, this could be implemented by way of augmenting renderHTMLSkeleton to look something like

function renderHTMLSkeleton(req, res, settings) {
  const hearingSlugMatch = /^\/([a-zA-Z0-9-]+)\/?$/.exec(req.url);
  const hearingDataPromise = hearingSlugMatch
    ? // TODO: cache this for a (short enough) while!
      fetch(`${settings.BÄKKÄRIN_URLI}/api/v0/hearing/${hearingSlugMatch[1]}`)
        .then(r => (r.status == 404 ? null : r.json())) // Not found? Okay, never mind, must've been a false positive
        .catch(err => null) // Other error? Meh, render w/o metatags then
    : Promise.resolve(null); // No match, no hearing data
  return hearingDataPromise.then(hearingData => {
    const html = renderToStaticMarkup(
      <Html
        bundleSrc={settings.bundleSrc || "/app.js"}
        apiBaseUrl={settings.apiBaseUrl}
        uiConfig={settings.uiConfig}
        hearingData={hearingData}
      />
    );
    res.status(200).send(html);
  });
}

and teaching the Html component to read hearingData.
The caching, which is marked as "TODO" is really a prerequisite IMNSHO; otherwise every request might attempt a secondary request to the backend.

@Rikuoja
Copy link
Contributor Author

Rikuoja commented Oct 25, 2017

+1 to the caching, if it works sensibly then this looks good.

@akx
Copy link
Contributor

akx commented Oct 25, 2017

Quick dry-coded sketch of what the simple cache might look like.

const hearingDataCache = {};
function getHearingDataCached(settings, hearingSlug, ttl = 30 * 60 * 1000) {
  const now = +new Date(); // current time as milliseconds
  if (
    hearingDataCache[hearingSlug] && // in cache
    now < hearingDataCache[hearing].ts + ttl // not expired yet
  ) {
    return hearingDataCache[hearingSlug].data;
  }
  return fetch(
    `${settings.BÄKKÄRIN_URLI}/api/v0/hearing/${hearingSlugMatch[1]}`
  )
    .then(r => (r.status == 404 ? null : r.json())) // Not found? Okay, never mind, must've been a false positive
    .catch(err => null) // Other error? Meh, render w/o metatags then
    .then(data => {
      // Save in cache while we're here. (Note that 404s and errors are also cached.)
      hearingDataCache[hearingSlug] = { data, ts: now };
      return data;
    });
}

// ...

const hearingSlugMatch = /^\/([a-zA-Z0-9-]+)\/?$/.exec(req.url);
const hearingDataPromise = hearingSlugMatch
  ? getHearingDataCached(settings, hearingSlugMatch[1])
  : Promise.resolve(null);

@jonnetanninen
Copy link
Contributor

This is implemented now, but there seems to be an issue with Facebook itself. When linkin the hearing to facebook, it doesn't actually load the image if the link is with https, e.g. https://kerrokantasi.hel.ninja/dfdsa?lang=fi.

This seems to be an issue with https, something like described here: https://stackoverflow.com/questions/8855361/fb-opengraph-ogimage-not-pulling-images-possibly-https

If the hearing is linked with http like this http://kerrokantasi.hel.ninja/dfdsa?lang=fi, the image will load correctly like.

@petterill
Copy link

Tested. Works as jonne described.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants