Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ROAD-546: Add comments to stories #304

Merged
merged 25 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7d6c141
Adds comment model and controller
torresga Sep 11, 2023
896603a
Adds routes
torresga Sep 11, 2023
107490f
Adds view
torresga Sep 11, 2023
25c331d
Adds empty comment and comments to stories controller
torresga Sep 11, 2023
ea4a476
Adds has_many comment association
torresga Sep 11, 2023
081f690
ROAD-547 adjusting edit and delete actions for comments
hmdros Sep 12, 2023
af95ed5
cleanup of controller due to test failures
torresga Sep 13, 2023
bb3866c
Adds specs for comments
torresga Sep 13, 2023
e06a4e8
ROAD-548 add support to export comments optionally
hmdros Sep 13, 2023
b10f301
[wip] adding controller specs
hmdros Sep 14, 2023
c5bc1c3
Add user to comment
hmdros Sep 15, 2023
49b4ed4
Adds specs to test that a user cannot edit, delete, or update another…
torresga Sep 15, 2023
c9a7282
Address review's feedback
hmdros Sep 18, 2023
f8fbb6e
Add flash for delete - add redirect projects when not found
hmdros Sep 19, 2023
3c65b4a
Removes respond_to blocks
torresga Sep 19, 2023
4cb6c5b
adds comments_headers to avoid appending headers to constant
torresga Sep 19, 2023
ddd943a
ROAD-546 comment_headers
hmdros Sep 20, 2023
165950f
Fix partial usage with local variables, and fix csv export - add spec…
hmdros Sep 20, 2023
9779ea8
Adjust partials
hmdros Sep 21, 2023
c76c595
Fixes typo
torresga Sep 21, 2023
cbcb3e7
fix issue of comments containing comments of previous story
torresga Sep 21, 2023
c1d13a6
improve csv export test case
hmdros Sep 22, 2023
dd31304
ROAD-548 adjust csv header comment loop
hmdros Sep 25, 2023
f8575b8
Remove unused comment_id
hmdros Sep 25, 2023
8c0a211
Undisables the submit button
torresga Sep 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions app/assets/stylesheets/stories.scss
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,36 @@
border: 1px solid #9b054d;
padding: 5px;
}

.comments-section {
margin: 16px 0;

.comment-card {
margin: 8px 0;

.bold {
font-weight: bold;
}

.link-blue {
color: blue;
}
}
}

.comment-form-container {
margin: 8px 0;
width: 50%;

.bold {
font-weight: bold;
}
}

#comment_body {
margin: 10px 0;
}

input.button.green {
width: auto;
}
59 changes: 59 additions & 0 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
class CommentsController < ApplicationController
before_action :authenticate_user!
before_action :load_story_and_project
before_action :find_comment, only: [:edit, :update, :destroy]

def edit
end

def create
@comment = current_user.comments.build(story: @story)
@comment.attributes = comment_params
saved = @comment.save
if saved
flash[:success] = "Comment created!"
else
flash[:error] = @comment.errors.full_messages
end

redirect_to project_story_path(@comment.story.project_id, @comment.story_id)
end

def update
updated = @comment.update(comment_params)
if updated
flash[:success] = "Comment updated!"
redirect_to project_story_path(@comment.story.project_id, @comment.story_id)
else
flash[:error] = @comment.errors.full_messages
render :edit
end
end

def destroy
@comment.destroy
flash[:success] = "Comment deleted!"
redirect_to project_story_path(@comment.story.project_id, @comment.story_id)
end

private

def find_comment
@comment = current_user.comments.where(story_id: params[:story_id]).find(params[:id])
rescue ActiveRecord::RecordNotFound
flash[:error] = "Comment not found"
redirect_to project_story_path(params[:project_id], params[:story_id])
end

def load_story_and_project
@project = Project.find(params[:project_id])
@story = Story.find(params[:story_id])
torresga marked this conversation as resolved.
Show resolved Hide resolved
rescue ActiveRecord::RecordNotFound
flash[:error] = "Project or Story not found"
redirect_to projects_path
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should handle the 2 possible exceptions individually, so in the cases where the project exists and the story doesn't we can redirect to the project instead of redirecting to the index

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but should we do that in this ticket or create another issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know, this is adding this code, we could create another story though, it's not a blocker so I'm fine ignoring it for now

end

def comment_params
params.require(:comment).permit(:body)
end
end
25 changes: 20 additions & 5 deletions app/controllers/stories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ def bulk_destroy

def show
@estimate = Estimate.find_by(story: @story, user: current_user)
@comments = @story.comments.includes(:user).order(:created_at)
@comment = Comment.new
end

def update
Expand Down Expand Up @@ -85,12 +87,25 @@ def import
end

def export
csv = CSV.generate(headers: true) { |csv|
csv << CSV_HEADERS
hmdros marked this conversation as resolved.
Show resolved Hide resolved
@project.stories.by_position.each do |story|
csv << story.attributes.slice(*CSV_HEADERS)
csv = if params[:export_with_comments] == "1"
CSV.generate(headers: true) do |csv|
csv << CSV_HEADERS + ["comment"]
@project.stories.includes(:comments).by_position.each do |story|
arielj marked this conversation as resolved.
Show resolved Hide resolved
comments = []
story.comments.each do |comment|
comments << "#{comment.user.name}: #{comment.body}"
end
csv << [story.id, story.title, story.description, story.position] + comments
end
end
}
else
CSV.generate(headers: true) do |csv|
csv << CSV_HEADERS
@project.stories.by_position.each do |story|
csv << story.attributes.slice(*CSV_HEADERS)
end
end
end
filename = "#{@project.title.gsub(/[^\w]/, "_")}-#{Time.now.to_formatted_s(:short).tr(" ", "_")}.csv"
send_data csv, filename: filename
end
Expand Down
5 changes: 5 additions & 0 deletions app/models/comment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class Comment < ApplicationRecord
belongs_to :story
belongs_to :user
validates :body, presence: true
end
1 change: 1 addition & 0 deletions app/models/story.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Story < ApplicationRecord
belongs_to :project
has_many :estimates
has_many :users, through: :estimates
has_many :comments

before_create :add_position

Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ class User < ApplicationRecord
include OmbuLabsAuthenticable

has_many :estimates
has_many :comments
end
7 changes: 7 additions & 0 deletions app/views/comments/_comment.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<div class="comment-card">
<p class="bold"><%= comment.user.name %>: <%= markdown(comment.body) %> <%= comment.created_at %><p>
<% if current_user == comment.user %>
<%= link_to "Edit Comment", edit_project_story_comment_path(project, story, comment), class: "link-blue" %> |
<%= link_to "Delete", project_story_comment_path(project, story, comment), method: :delete, data: { confirm: "Are you sure?" }, title: "Delete" %>
<% end %>
</div>
5 changes: 5 additions & 0 deletions app/views/comments/_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<%= form_with model: [project, story, comment] do |form| %>
<%= form.text_area :body, rows: 6 %>
<%= form.submit class: "button green"%>
<%= link_to "Back", project_story_path(project, story), id: "back", class: "button" if ["edit", "update"].include?(action_name) %>
<% end %>
4 changes: 4 additions & 0 deletions app/views/comments/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="container">
<h2 class="new-edit-title">Edit Comment</h2>
<%= render partial: "form", locals: {story: @story, comment: @comment, project: @project} %>
</div>
9 changes: 8 additions & 1 deletion app/views/projects/_import_export.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@
<h4 class="my-0 font-weight-normal">Export CSV</h4>
</div>
<div class="card-body">
<%= link_to 'Export', export_project_stories_path(@project), class: "button green" %>
<%= form_with url: export_project_stories_path(@project), method: :get do |f| %>
<%= f.submit "Export", class: "button green", data: { disable_with: false } %>
<br/>
<%= f.label :export_with_comments do %>
<%= f.check_box :export_with_comments %>
Export with comments
<% end %>
<% end %>
</div>
</div>
</div>
Expand Down
14 changes: 13 additions & 1 deletion app/views/stories/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="container">
<h1 class="dashboard-title"><%= render "shared/project_title", project: @project %></h1>

<%= render "shared/story", story: @story %>
<%= render partial: "shared/story", locals: { story: @story } %>

<div class="btn-group">
<%= link_to 'Back', project_path(@project), id: "back", class: "button" %>
Expand All @@ -10,4 +10,16 @@
<%= link_to "Delete", project_story_path(@project.id, @story), method: :delete, data: { confirm: "Are you sure?", story_id: @story.id }, class: "button red", remote: true , title: "Delete" %>
<% end %>
</div>

<div class="comments-section">
<h4>Comments</h4>
<% @comments.each do |comment| %>
<%= render partial: "comments/comment", locals: { story: @story, project: @project, comment: comment } %>
<% end %>
</div>

<div class="comment-form-container">
<p class="bold">Add a new comment</p>
<%= render partial: "comments/form", locals: { story: @story, project: @project, comment: @comment } %>
</div>
</div>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
get :export, on: :collection
resources :estimates, except: [:index, :show]
put :move
resources :comments, only: [:create, :edit, :update, :destroy]
end
resource :action_plan, only: [:show]
end
Expand Down
10 changes: 10 additions & 0 deletions db/migrate/20230908142819_create_comments.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class CreateComments < ActiveRecord::Migration[7.0]
def change
create_table :comments do |t|
t.text :body
t.integer :story_id
t.integer :user_id
t.timestamps
end
end
end
10 changes: 9 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,18 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2023_08_31_175732) do
ActiveRecord::Schema[7.0].define(version: 2023_09_08_142819) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

create_table "comments", force: :cascade do |t|
t.text "body"
t.integer "story_id"
t.integer "user_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

create_table "estimates", force: :cascade do |t|
t.integer "best_case_points"
t.integer "worst_case_points"
Expand Down
113 changes: 113 additions & 0 deletions spec/controllers/comments_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
require "rails_helper"

RSpec.describe CommentsController, type: :controller do
render_views

let!(:user) { FactoryBot.create(:user) }
let!(:project) { FactoryBot.create(:project) }
let!(:story) { FactoryBot.create(:story, project: project) }
let!(:comment) { FactoryBot.create(:comment, story: story, user: user) }

before do
@request.env["devise.mapping"] = Devise.mappings[:user]
sign_in user
end

describe "#create" do
context "with valid attributes" do
let(:valid_params) { FactoryBot.attributes_for(:comment) }

it "creates a new comment" do
expect {
post :create, params: {project_id: project.id, story_id: story.id, comment: valid_params}
}.to change(Comment, :count).by(1)
end

it "redirects to the story path" do
post :create, params: {project_id: project.id, story_id: story.id, comment: valid_params}

expect(response).to redirect_to project_story_path(project.id, story.id)
end
end

context "with invalid attributes" do
let(:invalid_params) { {body: ""} }

it "redirects back to the story page" do
post :create, params: {project_id: project.id, story_id: story.id, comment: invalid_params}
expect(response).to redirect_to project_story_path(project.id, story.id)
end
end
end

describe "#destroy" do
it "deletes the comment" do
delete :destroy, params: {project_id: project.id, story_id: story.id, id: comment.id}
expect(Comment.exists?(comment.id)).to be_falsey
expect(response).to redirect_to project_story_path(project.id, story.id)
end

it "disallows destroying another users' comment" do
user2 = FactoryBot.create(:user)
comment2 = FactoryBot.create(:comment, story: story, user: user2)

delete :destroy, params: {id: comment2.id, story_id: story.id, project_id: project.id}

expect(response).to redirect_to project_story_path(project.id, story.id)
expect(flash[:error]).to eq "Comment not found"
end
end

describe "#edit" do
before do
get :edit, params: {id: comment.id, story_id: story.id, project_id: project.id}
end

it "redirects to the edit page" do
expect(response).to render_template :edit
end

it "shows the fields for the comment" do
expect(assigns(:comment)).to eq comment
end
end

describe "#edit as other user" do
it "disallows editing another users' comment" do
user2 = FactoryBot.create(:user)
comment2 = FactoryBot.create(:comment, story: story, user: user2)

get :edit, params: {id: comment2.id, story_id: story.id, project_id: project.id}

expect(response).to redirect_to project_story_path(project.id, story.id)
expect(flash[:error]).to eq "Comment not found"
end
end

describe "#update" do
it "updates the body for the comment" do
put :update, params: {
id: comment.id,
story_id: story.id,
project_id: project.id,
comment: {
body: "test123"
}
}
expect(comment.reload.body).to eq "test123"
end

it "disallows updating another users' comment" do
user2 = FactoryBot.create(:user)
comment2 = FactoryBot.create(:comment, story: story, user: user2)

put :update, params: {id: comment2.id,
story_id: story.id,
project_id: project.id,
comment: {body: "test123"}}

expect(response).to redirect_to project_story_path(project.id, story.id)
expect(flash[:error]).to eq "Comment not found"
end
end
end
Loading
Loading