search

news-catalyst / next-tinynewsdemo

Frontend for Tiny News Collective organizations
https://next-tinynewsdemo.vercel.app
1 stars 0 forks source link

Add letterhead id to newsletter path #1325

Closed angel-tnc closed 1 year ago

angel-tnc commented 1 year ago

Fix for TNT-96.

Why

Newsletters were being routed using only a slug generated from the headline, which is commonly reused by members and has no uniqueness guarantees, which means a reliable unique URI cannot be derived from them alone.

What

Modify newsletter edition routing logic to take into account both the slug and the letterhead identifier used on the direct letterhead URI's, for example - https://read.letterhead.email/black-by-god/letter/11356

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
staging-tiny-news-sites ✅ Ready (Inspect) Visit Preview Mar 8, 2023 at 5:14PM (UTC)
9 Ignored Deployments | Name | Status | Preview | Updated | | :--- | :----- | :------ | :------ | | **ang-diaryo** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/ang-diaryo/6q1yw4w8zqcAFxyvNTMyW3dwCVFb)) | | Mar 8, 2023 at 5:14PM (UTC) | | **austin-vida** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/austin-vida/HnyffGqBmzy3PJU13RB5PpuRsHVi)) | | Mar 8, 2023 at 5:14PM (UTC) | | **black-by-god** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/black-by-god/6j6BrSQFMoQJdpJ54q7LtMAAowBQ)) | | Mar 8, 2023 at 5:14PM (UTC) | | **five-wards-media** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/five-wards-media/2U54pbHnWdgmYtVVPjLehavj9y37)) | | Mar 8, 2023 at 5:14PM (UTC) | | **harvey-world-herald** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/harvey-world-herald/9FVX5p3SWRyjYainWfCjSkbtqR3x)) | | Mar 8, 2023 at 5:14PM (UTC) | | **next-tinynewsdemo** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/next-tinynewsdemo/2Y3kZa7YywMjUchtpWVxu9QrfyyW)) | | Mar 8, 2023 at 5:14PM (UTC) | | **spotlight-schools** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/spotlight-schools/ELfyz8TF9G9agcZhUTL7mHaVuS7f)) | | Mar 8, 2023 at 5:14PM (UTC) | | **tiny-news-curriculum** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/tiny-news-curriculum/92hNcMkJDdUV6tPdWPbBgAXgnEvy)) | | Mar 8, 2023 at 5:14PM (UTC) | | **tiny-news-sites** | ⬜️ Ignored ([Inspect](https://vercel.com/news-catalyst/tiny-news-sites/5rd2iRm1oxtUbN85t9XrSUjRdLTM)) | | Mar 8, 2023 at 5:14PM (UTC) |
chelsea-tnc commented 1 year ago

@angel-tnc will this approach work for pre-existing duplicates or will we need to change those to fit this new solution?

angel-tnc commented 1 year ago

@chelsea-tnc yes, this approach is compatible with all existing newsletters and no database changes are required. Any old link with only the slug will work like now and point to the first newsletter found. Using the new link format will fetch the actual newsletter, as long as it has been fetched during build, which is an independent process and currently saves the letterhead_id.

angel-tnc commented 1 year ago

I'm checking that the old links with only the slug will get statically built. But after deploying this functionality, all links that show in the archive pages will have the new format and work, so that case would only apply if someone directly enters a link with only the slug in the browser.

angel-tnc commented 1 year ago

Ok, apologies for the many comments. Verified my assumptions to be certain. Summary:

  1. The newsletters with the new path that uses the letterhead id will be statically built on deploy and always point to a newsletter with that id, regardless of duplicate slugs.
  2. No database changes are needed. The letterhead_id DB column is a non-nullable integer and all current rows have a valid value.
  3. All links on archive list will be built using the new format and work.
  4. Old paths with only the slug will not be statically built but If anyone enters an older links with only the slug, it will trigger a fallback and show the page the first newsletter with that slug, so old links will work like it works now.
chelsea-tnc commented 1 year ago

@angel-tnc thanks for confirming. Let's ask members to understand the impact of 4, as it means existing links would be broken. I don't think most members are linking to their individual newsletters, but I asked on Slack to be sure.

angel-tnc commented 1 year ago

Thanks, makes sense to ask. In general existing slug-only links will work like now except they will take a second and show the "Loading..." message because they are not prebuilt. That's what I saw in my local production build. We can confirm on staging.

chelsea-tnc commented 1 year ago

@angel-tnc we can move this to staging. However, let's hold off on shipping to production until we better understand the impact to members first.