diff --git a/README.md b/README.md index b01160f..18a6637 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,9 @@ Run the tests locally (against development environment): ## TODOs -1. Report 404s better (thread by id and remark by id) +1. Clean-up 404 stuff and variable checking (what's up with + `$last_page` checking?) +1. Don't allow non-mods to view hidden posts at all 1. "All new posts flagged" mode (require approval for new posts) 1. Tests for mod-only user? diff --git a/lib/PostText/Controller/Remark.pm b/lib/PostText/Controller/Remark.pm index 8441949..07a07cc 100644 --- a/lib/PostText/Controller/Remark.pm +++ b/lib/PostText/Controller/Remark.pm @@ -6,9 +6,15 @@ sub by_id($self) { my $remark_id = $self->param('remark_id'); my $remark = $self->remark->by_id($remark_id); - $self->stash(status => 404) unless $remark->{'id'}; - - $self->stash(remark => $remark); + if (my $remark_id = $remark->{'id'}) { + $self->stash(remark => $remark) + } + else { + $self->stash( + remark => {}, + error => 'Remark not found 🤷' + ) + } $self->render; } diff --git a/lib/PostText/Controller/Thread.pm b/lib/PostText/Controller/Thread.pm index 43d0b02..f1ae968 100644 --- a/lib/PostText/Controller/Thread.pm +++ b/lib/PostText/Controller/Thread.pm @@ -64,13 +64,19 @@ sub by_id($self) { $self->stash( thread => {}, remarks => [], - status => 404 + status => 404, + error => 'Thread not found 🤷' ) } # Check for remarks or thread page number to make sure # remark->by_page_for did its job - $self->stash(status => 404) unless $remarks->[0] || 1 >= $this_page; + unless ((my $first_remark = $remarks->[0]) || 1 >= $this_page) { + $self->stash( + status => 404, + error => 'Page not found 🕵️' + ) + } $self->render; } @@ -81,14 +87,24 @@ sub by_page($self) { my $last_page = $self->thread->last_page; my $threads = $self->thread->by_page($this_page); - $self->stash(status => 404) unless $threads->[0]; - - $self->stash( - threads => $threads, - this_page => $this_page, - last_page => $last_page, - base_path => $base_path - ); + if (my $first_thread = $threads->[0]) { + $self->stash( + threads => $threads, + this_page => $this_page, + last_page => $last_page, + base_path => $base_path + ) + } + else { + $self->stash( + threads => [], + this_page => undef, + last_page => undef, + base_path => undef, + status => 404, + error => 'Page not found 🕵️' + ) + } $self->render; } diff --git a/t/thread.t b/t/thread.t index 191abc0..da9e6ff 100644 --- a/t/thread.t +++ b/t/thread.t @@ -38,7 +38,7 @@ subtest 'View single thread', sub { ->text_like(h2 => qr/Thread #1/); $t->get_ok('/thread/single/65536')->status_is(404) - ->text_like(h2 => qr/Thread #/); + ->text_like(p => qr/Thread not found/); }; subtest 'Threads feed', sub { diff --git a/templates/remark/by_id.html.ep b/templates/remark/by_id.html.ep index 445004e..c9f6a98 100644 --- a/templates/remark/by_id.html.ep +++ b/templates/remark/by_id.html.ep @@ -1,11 +1,12 @@ % layout 'default'; -% title "Remark #$remark->{'id'}"; +% title my $remark_id = $remark->{'id'} ? "Remark #$remark_id" : '?';

<%= title %>

+<% if (my $remark_id = $remark->{'id'}) { =%>

<%= $remark->{'date'} %> - #<%= $remark->{'id'} %> + #<%= $remark_id %>

@@ -31,3 +32,4 @@ <% } =%>
+<% } =%> diff --git a/templates/thread/by_id.html.ep b/templates/thread/by_id.html.ep index 6ab4efa..cdbf912 100644 --- a/templates/thread/by_id.html.ep +++ b/templates/thread/by_id.html.ep @@ -1,11 +1,12 @@ % layout 'default'; -% title "Thread #$thread->{'id'}"; +% title my $thread_id = $thread->{'id'} ? "Thread #$thread_id" : '?';

<%= title %>

+<% if (my $thread_id = $thread->{'id'}) { =%>

<%= $thread->{'title'} %> - #<%= $thread->{'id'} %> + #<%= $thread_id %>

@@ -36,6 +37,7 @@ <% } =%>
+<% } =%> <% if (my $first_remark = $remarks->[0]) { =%>

Remarks

diff --git a/templates/thread/by_page.html.ep b/templates/thread/by_page.html.ep index 3256d6e..1d60e20 100644 --- a/templates/thread/by_page.html.ep +++ b/templates/thread/by_page.html.ep @@ -1,6 +1,7 @@ % layout 'default'; % title 'Threads List';

<%= title %>

+<% if (my $first_thread = $threads->[0]) { =%>
<% for my $thread (@{$threads}) { =%>
@@ -36,5 +37,6 @@ -<% } =%> + <% } =%>
+<% } =%>