Skip to content

Conversation

@RoPedro
Copy link
Contributor

@RoPedro RoPedro commented Dec 13, 2025

This address a problem where, if you create an article and try to access without signing, would throw an error.

Summary by CodeRabbit

  • New Features
    • Updated the article comment form to display intelligently based on user authentication status. Logged-in users who are not the article author can now comment directly, while others receive a prompt to sign up or log in.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 13, 2025

Walkthrough

The article show view now conditionally renders the comment form based on article existence, user authentication, and author status. Non-authenticated users or article authors see a sign-up/sign-in prompt instead of the comment form.

Changes

Cohort / File(s) Summary
Article Show View Comment Flow
app/views/articles/show.html.erb
Added conditional rendering for comment form: displays form only when article exists, user is logged in, and user is not the article author. Otherwise shows sign-up/sign-in prompt. Includes indentation adjustments for nested structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify conditional logic correctly identifies article author vs. commenter
  • Check that sign-up/sign-in prompt is appropriately positioned and styled
  • Ensure indentation and template syntax are correct

Poem

🐰 Comments flow like carrots in a row,
Yet authors can't on their own pieces grow—
Sign up, sign in, the prompt now gleams,
Guarding discourse with careful schemes! 🌟

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective: enabling guest users to read articles without errors, which matches the PR description and the guarded commenting flow changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
app/views/articles/show.html.erb (4)

72-86: Misleading message shown to authenticated article authors.

The current logic treats two distinct cases identically:

  1. Guest users (not logged in)
  2. Authenticated authors viewing their own article

Both see "Sign up/in to add a comment", but this is confusing for authors who are already signed in. Consider differentiating these cases.

Apply this diff to provide appropriate messaging:

-      <% if @article %>
-          <% if @current_user && @current_user.username != @article.author.username %>
+      <% if @article %>
+          <% if @current_user %>
+            <% if @current_user.username != @article.author.username %>
             <%= form_with(model: [@article, @comment], local: true, url: article_comments_path(@article.id), method: :post, class: 'card comment-form') do |form| %>
                 <div class="card-block">
                     <%= form.text_area :body, class: 'form-control', placeholder: 'Write a comment...', rows: 3 %>
                 </div>
                 <div class="card-footer">
                     <img src="<%= current_user.image %>" class="comment-author-img" />
                     <button class="btn btn-sm btn-primary">Post Comment</button>
                 </div>
-              <% end %>
+            <% end %>
+            <% else %>
+              <div class="card-block">
+                <p>Authors cannot comment on their own articles.</p>
+              </div>
+            <% end %>
           <% else %>
             <div class="card-block">
               <p>Sign up/in to add a comment.</p>
             </div>
           <% end %>

91-110: Critical: Comments loop will crash if article is nil.

The comments display loop on line 91 is positioned outside the <% if @article %> check (which ends at line 89). If @article is nil, @article.comments.each will raise a NoMethodError, defeating the purpose of this PR fix.

Apply this diff to move the comments loop inside the article check:

-        <% else %>
-            <p>Article not found.</p>
-        <% end %>
-
         <% @article.comments.each do |comment| %>
           <div class="card">
             <div class="card-block">
               <p class="card-text"><%= comment.body %></p>
             </div>
             <div class="card-footer">
               <%= link_to show_users_path(comment.author.username), class: 'comment-author' do %>
                 <%= image_tag comment.author.image, class: 'comment-author-img' %>
               <% end %>
               &nbsp;
               <%= link_to comment.author.username, show_users_path(comment.author.username), class: 'comment-author' %>
               <span class="date-posted"><%= comment.created_at.strftime('%B %d, %Y') %></span>
               <% if current_user == comment.author %>
                 <span class="mod-options">
                   <%= button_to '', article_comments_path(@article.id, comment), method: :delete, class: 'ion-trash-a' %>
                 </span>
               <% end %>
             </div>
           </div>
         <% end %>
+      <% else %>
+        <p>Article not found.</p>
+      <% end %>

20-24: Favorite buttons will fail for guest users.

The Favorite/Unfavorite buttons are displayed to all users, including guests, without authentication checks. When clicked by a guest user, these will trigger authentication errors.

Apply this diff to conditionally show these buttons only to authenticated users:

         &nbsp;&nbsp;
-        <% if @is_favorited %>
-          <%= button_to 'Unfavorite Post', unfavorite_article_path(@article.slug), method: :delete, class: 'btn btn-sm btn-outline-primary' %>
-        <% else %>
-          <%= button_to 'Favorite Post', favorite_article_path(@article.slug), method: :post, class: 'btn btn-sm btn-outline-primary' %>
-        <% end %>
+        <% if @current_user %>
+          <% if @is_favorited %>
+            <%= button_to 'Unfavorite Post', unfavorite_article_path(@article.slug), method: :delete, class: 'btn btn-sm btn-outline-primary' %>
+          <% else %>
+            <%= button_to 'Favorite Post', favorite_article_path(@article.slug), method: :post, class: 'btn btn-sm btn-outline-primary' %>
+          <% end %>
+        <% end %>

56-62: Follow and Favorite buttons will fail for guest users.

Similar to the issue above, the Follow and Favorite/Unfavorite buttons in the article actions section are displayed without authentication checks. Guest users will encounter errors when clicking these buttons.

Apply this diff to conditionally show these buttons only to authenticated users:

-        <%= button_to 'Follow', follow_profile_path(@article.author.username), method: :post, class: 'btn btn-sm btn-outline-secondary' %>
-        &nbsp;
-        <% if @is_favorited %>
-          <%= button_to 'Unfavorite Post', unfavorite_article_path(@article.slug), method: :delete, class: 'btn btn-sm btn-outline-primary' %>
-        <% else %>
-          <%= button_to 'Favorite Post', favorite_article_path(@article.slug), method: :post, class: 'btn btn-sm btn-outline-primary' %>
-        <% end %>
+        <% if @current_user %>
+          <%= button_to 'Follow', follow_profile_path(@article.author.username), method: :post, class: 'btn btn-sm btn-outline-secondary' %>
+          &nbsp;
+          <% if @is_favorited %>
+            <%= button_to 'Unfavorite Post', unfavorite_article_path(@article.slug), method: :delete, class: 'btn btn-sm btn-outline-primary' %>
+          <% else %>
+            <%= button_to 'Favorite Post', favorite_article_path(@article.slug), method: :post, class: 'btn btn-sm btn-outline-primary' %>
+          <% end %>
+        <% end %>
🧹 Nitpick comments (1)
app/views/articles/show.html.erb (1)

14-18: Inconsistent use of @current_user vs current_user.

The template mixes @current_user (instance variable) and current_user (helper method) throughout. Lines 14 and 72 use @current_user, while lines 15, 25, 63, 78, and 103 use current_user. This inconsistency can lead to subtle bugs if they're not always synchronized.

Standardize on the Rails convention of using the current_user helper method throughout the template. Replace all instances of @current_user with current_user for consistency.

Also applies to: 25-25, 63-63, 72-72, 78-78, 103-103

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d6bcf1 and 4edea2a.

📒 Files selected for processing (1)
  • app/views/articles/show.html.erb (2 hunks)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant