diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 768aa26d..9527ec60 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -8,8 +8,6 @@ class StoriesController < ApplicationController include ApplicationHelper - CSV_HEADERS = %w[id title description position] - def new @story = Story.where(id: params[:story_id]).first_or_initialize.dup end @@ -87,25 +85,19 @@ def import end def export - csv = if params[:export_with_comments] == "1" - CSV.generate(headers: true) do |csv| - csv << CSV_HEADERS + ["comment"] - @project.stories.includes(:comments).approved.by_position.each do |story| - comments = [] - story.comments.each do |comment| - comments << "#{display_name(comment.user)}: #{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.approved.by_position.each do |story| - csv << story.attributes.slice(*CSV_HEADERS) - end - end - end + with_comments = params[:export_with_comments] == "1" + # Only admins may export non-approved stories. Enforce it here rather than + # relying on the checkbox being hidden in the view, so the param can't be + # forged by a non-admin. + export_all = params[:export_all] == "1" && current_user.admin? + + stories = export_all ? @project.stories : @project.stories.approved + # Eager-load the comments and their authors; generate_csv reads + # comment.user for every comment, which would otherwise be an N+1. + stories = stories.includes(comments: :user) if with_comments + + csv = generate_csv(stories, with_comments: with_comments) + filename = "#{@project.title.gsub(/[^\w]/, "_")}-#{Time.now.to_formatted_s(:short).tr(" ", "_")}.csv" send_data csv, filename: filename end @@ -150,6 +142,27 @@ def pending private + def generate_csv(stories, with_comments: false) + CSV.generate(headers: true) do |csv| + headers = %w[id position status title description] + headers << "comments" if with_comments + csv << headers + + stories.by_position.each do |story| + row = [story.id, story.position, story.status, story.title, story.description] + + # Keep every story on a single, uniform-width row: collapse all comments + # into one cell instead of spilling into a variable number of trailing, + # unlabeled columns. + if with_comments + row << story.comments.map { |comment| "#{display_name(comment.user)}: #{comment.body}" }.join("\n") + end + + csv << row + end + end + end + def find_project @project = Project.find(params[:project_id]) end diff --git a/app/views/projects/_import_export.html.erb b/app/views/projects/_import_export.html.erb index d3549268..dac9bcb4 100644 --- a/app/views/projects/_import_export.html.erb +++ b/app/views/projects/_import_export.html.erb @@ -33,7 +33,13 @@
<%= form_with url: export_project_stories_path(@project), method: :get do |f| %> <%= f.submit "Export", class: "button green", data: { disable_with: false } %> -
+ <% if current_user.admin? %> +
+ <%= f.label :export_all do %> + <%= f.check_box :export_all %> + Export all stories + <% end %> + <% end %> <%= f.label :export_with_comments do %> <%= f.check_box :export_with_comments %> Export with comments diff --git a/spec/controllers/stories_controller_spec.rb b/spec/controllers/stories_controller_spec.rb index c9e677dc..15f0c5fb 100644 --- a/spec/controllers/stories_controller_spec.rb +++ b/spec/controllers/stories_controller_spec.rb @@ -181,12 +181,50 @@ csv_data = CSV.parse(response.body) expected_csv_content = [ - ["id", "title", "description", "position"], - [story.id.to_s, story.title, story.description, story.position.to_s] + ["id", "position", "status", "title", "description"], + [story.id.to_s, story.position.to_s, story.status, story.title, story.description] ] expect(csv_data).to eq(expected_csv_content) end + context "when an admin" do + it "exports a CSV file with all stories" do + sign_in FactoryBot.create(:user, :admin) + + story2 = FactoryBot.create(:story, project: project, status: :rejected) + story3 = FactoryBot.create(:story, project: project, status: :pending) + get :export, params: {project_id: project.id, export_all: "1"} + expect(response).to have_http_status(:ok) + + csv_data = CSV.parse(response.body) + expected_csv_content = [ + ["id", "position", "status", "title", "description"], + [story.id.to_s, story.position.to_s, story.status, story.title, story.description], + [story2.id.to_s, story2.position.to_s, story2.status, story2.title, story2.description], + [story3.id.to_s, story3.position.to_s, story3.status, story3.title, story3.description] + ] + expect(csv_data).to eq(expected_csv_content) + end + end + + context "when not an admin" do + it "ignores export_all and exports only approved stories" do + # The signed-in user from the before block is not an admin, so a + # forged export_all param must not leak non-approved stories. + FactoryBot.create(:story, project: project, status: :rejected) + FactoryBot.create(:story, project: project, status: :pending) + get :export, params: {project_id: project.id, export_all: "1"} + expect(response).to have_http_status(:ok) + + csv_data = CSV.parse(response.body) + expected_csv_content = [ + ["id", "position", "status", "title", "description"], + [story.id.to_s, story.position.to_s, story.status, story.title, story.description] + ] + expect(csv_data).to eq(expected_csv_content) + end + end + context "with comments" do it "exports a CSV file with only approved stories" do user = FactoryBot.create(:user) @@ -206,16 +244,39 @@ csv_data = CSV.parse(response.body) expected_csv_content = [ - ["id", "title", "description", "position", "comment"], - [story.id.to_s, story.title, story.description, story.position.to_s, "#{comment1.user.name}: #{comment1.body}", "#{comment1_2.user.name}: #{comment1_2.body}"], - [story2.id.to_s, story2.title, story2.description, story2.position.to_s, "#{comment2_1.user.name}: #{comment2_1.body}", "#{comment2_2.user.name}: #{comment2_2.body}"], - [story3.id.to_s, story3.title, story3.description, story3.position.to_s, "#{comment3_1.user.name}: #{comment3_1.body}"], - [story4.id.to_s, story4.title, story4.description, story4.position.to_s] + ["id", "position", "status", "title", "description", "comments"], + [story.id.to_s, story.position.to_s, story.status, story.title, story.description, "#{comment1.user.name}: #{comment1.body}\n#{comment1_2.user.name}: #{comment1_2.body}"], + [story2.id.to_s, story2.position.to_s, story2.status, story2.title, story2.description, "#{comment2_1.user.name}: #{comment2_1.body}\n#{comment2_2.user.name}: #{comment2_2.body}"], + [story3.id.to_s, story3.position.to_s, story3.status, story3.title, story3.description, "#{comment3_1.user.name}: #{comment3_1.body}"], + [story4.id.to_s, story4.position.to_s, story4.status, story4.title, story4.description, ""] ] expect(csv_data).to eq(expected_csv_content) end end + + context "when an admin exports all stories with comments" do + it "includes comments for non-approved stories too" do + sign_in FactoryBot.create(:user, :admin) + commenter = FactoryBot.create(:user) + + rejected = FactoryBot.create(:story, project: project, status: :rejected) + pending = FactoryBot.create(:story, project: project, status: :pending) + approved_comment = FactoryBot.create(:comment, user: commenter, story: story) + rejected_comment = FactoryBot.create(:comment, user: commenter, story: rejected) + + get :export, params: {project_id: project.id, export_all: "1", export_with_comments: "1"} + expect(response).to have_http_status(:ok) + + csv_data = CSV.parse(response.body) + expect(csv_data).to eq([ + ["id", "position", "status", "title", "description", "comments"], + [story.id.to_s, story.position.to_s, story.status, story.title, story.description, "#{commenter.name}: #{approved_comment.body}"], + [rejected.id.to_s, rejected.position.to_s, rejected.status, rejected.title, rejected.description, "#{commenter.name}: #{rejected_comment.body}"], + [pending.id.to_s, pending.position.to_s, pending.status, pending.title, pending.description, ""] + ]) + end + end end end end