[e.cash] Add media link to blog post
ClosedPublic
Actions

Authored by johnkuney on Sep 5 2023, 22:27.

Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

rABC0c55d75253dd: [e.cash] Add media link to blog post

Summary

Some of our blog post were originally authored/posted by other outlets. Adding a link to these posts based on if its present and a valid url in the strapi data

Test Plan

Run the app and find some blog posts that have this link. Check out the line under the title looks good and the link works
Also check it is not rendered on normal posts
Here are some for example that have a media link

/blog/how-blockchain-will-change-the-way-we-work-play-and-stay-healthy-in-the-future
/blog/gnc-shines-amid-high-profile-bitcoin-dev-resignations
/blog/is-there-a-future-for-cryptocurrencies-without-privacy-features

also run npm run test to run new tests

Diff Detail

Repository

rABC Bitcoin ABC

Branch

ecash-blog-media-links

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 24974
Build 49536: Build Diff
Build 49535: arc lint + arc unit

Event Timeline

johnkuney created this revision.Sep 5 2023, 22:27

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 5 2023, 22:27

johnkuney requested review of this revision.Sep 5 2023, 22:27

Harbormaster completed remote builds in B24951: Diff 42078.Sep 5 2023, 22:28

bytesofman added a subscriber: bytesofman.Sep 5 2023, 22:51

bytesofman added inline comments.

web/e.cash/pages/blog/[slug].js
42 ↗	(On Diff #42078)	what is `post.attributes.media_link` for posts that don't have this? does the key not exist, does it have a value of `undefined`?

bytesofman requested changes to this revision.Sep 5 2023, 22:52

bytesofman added inline comments.

web/e.cash/pages/blog/[slug].js
42 ↗	(On Diff #42078)	we want to be specific as possible with this stuff -- a lot of things can evaluate to `true` and thus render `post.attributes.media_link &&` if the ones that "exist" are all strings, `typeof post.attributes.media_link === 'string' &&` would be better

This revision now requires changes to proceed.Sep 5 2023, 22:52

add a function to test url validity, and some tests to go with it.

Harbormaster completed remote builds in B24968: Diff 42099.Sep 7 2023, 02:55

For sure. Strapi does only allow strings in this field (and null), and the key is always there for every entry, but yeah should be more robust

johnkuney edited the summary of this revision. (Show Details)Sep 7 2023, 02:59

johnkuney edited the test plan for this revision. (Show Details)

bytesofman requested changes to this revision.Sep 7 2023, 03:42

bytesofman added inline comments.

web/e.cash/data/blog.js
81 ↗	(On Diff #42099)	haven't seen this convention before -- why `_` and not `err`? I get it's not used, but seem to get a linting error if you use `catch()`, whereas `catch (err)` is ok. I don't think this convention is used in any other JS apps in the monorepo, so best to stick with `(err)` unless there is something I'm overlooking.