From f05a12a880cbb1399925d9dc12317f6f5ce8a285 Mon Sep 17 00:00:00 2001
From: Edward Loveall <edward@edwardloveall.com>
Date: Fri, 17 Jun 2022 13:26:35 -0400
Subject: [PATCH] Add support for missing posts
Posts, like 8661f4724aa9, can go missing if the account or post was
removed. In this case, the API returns data like this:
```json
{
"data": {
"post": null
}
}
```
When this happens, we can detect it because the parsed response now has
a nil value: `response.data.post == nil` and construct an `EmptyPage`
instead of a `Page`. The `Articles::Show` action can then render
conditionally based on if the response from `PageConverter` is a `Page`
or an `EmptyPage`.
---
CHANGELOG | 4 ++
spec/actions/articles/show_spec.cr | 15 ++++++
spec/classes/page_converter_spec.cr | 82 +++++++++++++----------------
spec/setup/clean_database.cr | 3 --
spec/support/action_helpers.cr | 12 +++++
src/actions/articles/show.cr | 12 ++++-
src/actions/errors/show.cr | 4 ++
src/classes/page_converter.cr | 20 ++++---
src/models/missing_page.cr | 5 ++
src/models/post_response.cr | 2 +-
src/version.cr | 2 +-
11 files changed, 102 insertions(+), 59 deletions(-)
create mode 100644 spec/actions/articles/show_spec.cr
delete mode 100644 spec/setup/clean_database.cr
create mode 100644 spec/support/action_helpers.cr
create mode 100644 src/models/missing_page.cr
diff --git a/CHANGELOG b/CHANGELOG
index a06f831..293d70d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,9 @@
2022-05-21
+* Show error page for missing posts
+
+2022-05-21
+
* Remove the need for a fake DATABASE_URL
2022-04-04
diff --git a/spec/actions/articles/show_spec.cr b/spec/actions/articles/show_spec.cr
new file mode 100644
index 0000000..88d6bc0
--- /dev/null
+++ b/spec/actions/articles/show_spec.cr
@@ -0,0 +1,15 @@
+require "../../spec_helper"
+
+include ActionHelpers
+
+describe Articles::Show do
+ context "if the article is missing" do
+ it "should raise a MissingPageError" do
+ context = action_context(path: "/abc123")
+
+ expect_raises(MissingPageError) do
+ Articles::Show.new(context, params).call
+ end
+ end
+ end
+end
diff --git a/spec/classes/page_converter_spec.cr b/spec/classes/page_converter_spec.cr
index 39f89e0..ccb07db 100644
--- a/spec/classes/page_converter_spec.cr
+++ b/spec/classes/page_converter_spec.cr
@@ -17,8 +17,8 @@ describe PageConverter do
}
]
JSON
- data_json = default_data_json(title, paragraph_json)
- data = PostResponse::Data.from_json(data_json)
+ data_json = default_post_json(title, paragraph_json)
+ data = PostResponse::Post.from_json(data_json)
page = PageConverter.new.convert(data)
@@ -26,52 +26,48 @@ describe PageConverter do
end
it "sets the author" do
- data_json = <<-JSON
+ post_json = <<-JSON
{
- "post": {
- "title": "This is a story",
- "createdAt": 0,
- "creator": {
- "id": "abc123",
- "name": "Author"
- },
- "content": {
- "bodyModel": {
- "paragraphs": []
- }
+ "title": "This is a story",
+ "createdAt": 0,
+ "creator": {
+ "id": "abc123",
+ "name": "Author"
+ },
+ "content": {
+ "bodyModel": {
+ "paragraphs": []
}
}
}
JSON
- data = PostResponse::Data.from_json(data_json)
+ post = PostResponse::Post.from_json(post_json)
- page = PageConverter.new.convert(data)
+ page = PageConverter.new.convert(post)
page.author.name.should eq "Author"
page.author.id.should eq "abc123"
end
it "sets the publish date/time" do
- data_json = <<-JSON
+ post_json = <<-JSON
{
- "post": {
- "title": "This is a story",
- "createdAt": 1000,
- "creator": {
- "id": "abc123",
- "name": "Author"
- },
- "content": {
- "bodyModel": {
- "paragraphs": []
- }
+ "title": "This is a story",
+ "createdAt": 1000,
+ "creator": {
+ "id": "abc123",
+ "name": "Author"
+ },
+ "content": {
+ "bodyModel": {
+ "paragraphs": []
}
}
}
JSON
- data = PostResponse::Data.from_json(data_json)
+ post = PostResponse::Post.from_json(post_json)
- page = PageConverter.new.convert(data)
+ page = PageConverter.new.convert(post)
page.created_at.should eq Time.utc(1970, 1, 1, 0, 0, 1)
end
@@ -98,8 +94,8 @@ describe PageConverter do
}
]
JSON
- data_json = default_data_json(title, paragraph_json)
- data = PostResponse::Data.from_json(data_json)
+ post_json = default_post_json(title, paragraph_json)
+ data = PostResponse::Post.from_json(post_json)
page = PageConverter.new.convert(data)
@@ -115,23 +111,21 @@ def default_paragraph_json
"[]"
end
-def default_data_json(
+def default_post_json(
title : String = "This is a story",
paragraph_json : String = default_paragraph_json
)
<<-JSON
{
- "post": {
- "title": "#{title}",
- "createdAt": 1628974309758,
- "creator": {
- "id": "abc123",
- "name": "Author"
- },
- "content": {
- "bodyModel": {
- "paragraphs": #{paragraph_json}
- }
+ "title": "#{title}",
+ "createdAt": 1628974309758,
+ "creator": {
+ "id": "abc123",
+ "name": "Author"
+ },
+ "content": {
+ "bodyModel": {
+ "paragraphs": #{paragraph_json}
}
}
}
diff --git a/spec/setup/clean_database.cr b/spec/setup/clean_database.cr
deleted file mode 100644
index a1bc631..0000000
--- a/spec/setup/clean_database.cr
+++ /dev/null
@@ -1,3 +0,0 @@
-Spec.before_each do
- AppDatabase.truncate
-end
diff --git a/spec/support/action_helpers.cr b/spec/support/action_helpers.cr
new file mode 100644
index 0000000..70358b3
--- /dev/null
+++ b/spec/support/action_helpers.cr
@@ -0,0 +1,12 @@
+module ActionHelpers
+ private def action_context(path = "/")
+ io = IO::Memory.new
+ request = HTTP::Request.new("GET", path)
+ response = HTTP::Server::Response.new(io)
+ HTTP::Server::Context.new(request, response)
+ end
+
+ private def params
+ {} of String => String
+ end
+end
diff --git a/src/actions/articles/show.cr b/src/actions/articles/show.cr
index 15b65a7..9a09acc 100644
--- a/src/actions/articles/show.cr
+++ b/src/actions/articles/show.cr
@@ -6,8 +6,8 @@ class Articles::Show < BrowserAction
case post_id
in Monads::Just
response = client_class.post_data(post_id.value!)
- page = PageConverter.new.convert(response.data)
- html ShowPage, page: page
+ page = PageConverter.new.convert(response.data.post)
+ render_page(page)
in Monads::Nothing, Monads::Maybe
html(
Errors::ParseErrorPage,
@@ -18,6 +18,14 @@ class Articles::Show < BrowserAction
end
end
+ def render_page(page : Page)
+ html ShowPage, page: page
+ end
+
+ def render_page(page : MissingPage)
+ raise MissingPageError.new
+ end
+
def client_class
if use_local?
LocalClient
diff --git a/src/actions/errors/show.cr b/src/actions/errors/show.cr
index 0afdd0b..82c70b6 100644
--- a/src/actions/errors/show.cr
+++ b/src/actions/errors/show.cr
@@ -4,6 +4,10 @@ class Errors::Show < Lucky::ErrorAction
default_format :html
dont_report [Lucky::RouteNotFoundError, Avram::RecordNotFoundError]
+ def render(error : MissingPageError)
+ error_html message: "This article is missing.", status: 404
+ end
+
def render(error : Lucky::RouteNotFoundError | Avram::RecordNotFoundError)
if html?
error_html "Sorry, we couldn't find that page.", status: 404
diff --git a/src/classes/page_converter.cr b/src/classes/page_converter.cr
index a277f96..fb454cd 100644
--- a/src/classes/page_converter.cr
+++ b/src/classes/page_converter.cr
@@ -1,20 +1,24 @@
class PageConverter
- def convert(data : PostResponse::Data) : Page
- title, content = title_and_content(data)
- author = data.post.creator
- created_at = Time.unix_ms(data.post.createdAt)
+ def convert(post : PostResponse::Post) : Page
+ title, content = title_and_content(post)
+ author = post.creator
+ created_at = Time.unix_ms(post.createdAt)
gist_store = gist_store(content)
Page.new(
title: title,
author: author,
- created_at: Time.unix_ms(data.post.createdAt),
+ created_at: Time.unix_ms(post.createdAt),
nodes: ParagraphConverter.new.convert(content, gist_store)
)
end
- def title_and_content(data : PostResponse::Data) : {String, Array(PostResponse::Paragraph)}
- title = data.post.title
- paragraphs = data.post.content.bodyModel.paragraphs
+ def convert(post : Nil) : MissingPage
+ MissingPage.new
+ end
+
+ def title_and_content(post : PostResponse::Post) : {String, Array(PostResponse::Paragraph)}
+ title = post.title
+ paragraphs = post.content.bodyModel.paragraphs
non_content_paragraphs = paragraphs.reject { |para| para.text == title }
{title, non_content_paragraphs}
end
diff --git a/src/models/missing_page.cr b/src/models/missing_page.cr
new file mode 100644
index 0000000..72ce452
--- /dev/null
+++ b/src/models/missing_page.cr
@@ -0,0 +1,5 @@
+class MissingPage
+end
+
+class MissingPageError < Exception
+end
diff --git a/src/models/post_response.cr b/src/models/post_response.cr
index 478884b..b295ffd 100644
--- a/src/models/post_response.cr
+++ b/src/models/post_response.cr
@@ -8,7 +8,7 @@ class PostResponse
end
class Data < Base
- property post : Post
+ property post : Post?
end
class Post < Base
diff --git a/src/version.cr b/src/version.cr
index a4c4b6c..a1ac6c2 100644
--- a/src/version.cr
+++ b/src/version.cr
@@ -1,3 +1,3 @@
module Scribe
- VERSION = "2022-05-21"
+ VERSION = "2022-06-17"
end