From cd5a7354579a9e3777f68c9eb725af2b685703a9 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Thu, 27 Jun 2019 12:16:54 +0300 Subject: [PATCH 01/12] Add Bootstrap. Style. Fix tests. --- Gemfile | 2 ++ Gemfile.lock | 25 ++++++++++++++++--- app/assets/javascripts/application.js | 2 ++ .../{application.css => application.scss} | 3 +++ app/views/answers/_answer.html.slim | 14 ++++++----- app/views/layouts/application.html.erb | 18 ------------- app/views/layouts/application.html.slim | 17 +++++++++++++ .../questions/_question_for_show.html.slim | 16 +++++++----- app/views/questions/show.html.slim | 9 ++++--- app/views/shared/_answer_links.html.slim | 7 +++--- app/views/shared/_question_links.html.slim | 5 ++-- app/views/shared/_user_nav.html.slim | 13 +++++----- spec/features/answer/create_spec.rb | 8 +++--- 13 files changed, 82 insertions(+), 57 deletions(-) rename app/assets/stylesheets/{application.css => application.scss} (88%) delete mode 100644 app/views/layouts/application.html.erb create mode 100644 app/views/layouts/application.html.slim diff --git a/Gemfile b/Gemfile index 259c4f2..31cb56e 100644 --- a/Gemfile +++ b/Gemfile @@ -19,6 +19,8 @@ gem 'uglifier', '>= 1.3.0' gem 'devise' # JS gem 'jquery-rails' +# UI +gem 'bootstrap', '~> 4.3.1' # Use CoffeeScript for .coffee assets and views gem 'coffee-rails', '~> 4.2' diff --git a/Gemfile.lock b/Gemfile.lock index b398e95..7e25859 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,10 +45,16 @@ GEM addressable (2.6.0) public_suffix (>= 2.0.2, < 4.0) arel (9.0.0) + autoprefixer-rails (9.6.0) + execjs bcrypt (3.1.13) bindex (0.7.0) bootsnap (1.4.4) msgpack (~> 1.0) + bootstrap (4.3.1) + autoprefixer-rails (>= 9.1.0) + popper_js (>= 1.14.3, < 2) + sassc-rails (>= 2.0.0) builder (3.2.3) byebug (11.0.1) capybara (3.24.0) @@ -119,7 +125,8 @@ GEM mini_portile2 (~> 2.4.0) orm_adapter (0.5.0) pg (1.1.4) - public_suffix (3.1.0) + popper_js (1.14.5) + public_suffix (3.1.1) puma (3.12.1) rack (2.0.7) rack-test (1.1.0) @@ -157,9 +164,9 @@ GEM rb-inotify (0.10.0) ffi (~> 1.0) regexp_parser (1.5.1) - responders (2.4.1) - actionpack (>= 4.2.0, < 6.0) - railties (>= 4.2.0, < 6.0) + responders (3.0.0) + actionpack (>= 5.0) + railties (>= 5.0) rspec-core (3.8.1) rspec-support (~> 3.8.0) rspec-expectations (3.8.4) @@ -190,6 +197,15 @@ GEM sprockets (>= 2.8, < 4.0) sprockets-rails (>= 2.0, < 4.0) tilt (>= 1.1, < 3) + sassc (2.0.1) + ffi (~> 1.9) + rake + sassc-rails (2.1.2) + railties (>= 4.0.0) + sassc (>= 2.0) + sprockets (> 3.0) + sprockets-rails + tilt selenium-webdriver (3.142.3) childprocess (>= 0.5, < 2.0) rubyzip (~> 1.2, >= 1.2.2) @@ -246,6 +262,7 @@ PLATFORMS DEPENDENCIES bootsnap (>= 1.1.0) + bootstrap (~> 4.3.1) byebug capybara (>= 2.15) coffee-rails (~> 4.2) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index a55d271..69941fe 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -14,4 +14,6 @@ //= require activestorage //= require turbolinks //= require jquery3 +//= require popper +//= require bootstrap-sprockets //= require_tree . diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.scss similarity index 88% rename from app/assets/stylesheets/application.css rename to app/assets/stylesheets/application.scss index d05ea0f..f8c1d73 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.scss @@ -13,3 +13,6 @@ *= require_tree . *= require_self */ + +// Custom bootstrap variables must be set or imported *before* bootstrap. +@import "bootstrap"; diff --git a/app/views/answers/_answer.html.slim b/app/views/answers/_answer.html.slim index d1062ba..99e5be5 100644 --- a/app/views/answers/_answer.html.slim +++ b/app/views/answers/_answer.html.slim @@ -1,12 +1,14 @@ -div id="answer_#{answer.id}" +div id="answer_#{answer.id}" class="mb-3" - if answer.persisted? - if answer.best? div id="best-answer" - h4 The best answer - p= answer.body - p= render(partial: 'shared/answer_links', locals: { answer: answer }) + h5 The best answer + p class="mb-1"= answer.body + p class="mb-1"= render(partial: 'shared/answer_links', locals: { answer: answer }) = form_with model: answer, class: 'hidden', html: { id: "edit-answer-#{answer.id}" } do |f| - = f.label :body, 'Your answer' - = f.text_area :body + = f.label :body, 'Your answer', class: "mb-0" + br + = f.text_area :body, cols: 32, rows: 3 + br = f.submit 'Update' diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb deleted file mode 100644 index d6f50f6..0000000 --- a/app/views/layouts/application.html.erb +++ /dev/null @@ -1,18 +0,0 @@ - - - - QnA - <%= csrf_meta_tags %> - <%= csp_meta_tag %> - - <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %> - <%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %> - - - - <%= render 'shared/user_nav' %> -

<%= notice %>

-

<%= alert %>

- <%= yield %> - - diff --git a/app/views/layouts/application.html.slim b/app/views/layouts/application.html.slim new file mode 100644 index 0000000..b244244 --- /dev/null +++ b/app/views/layouts/application.html.slim @@ -0,0 +1,17 @@ +doctype html +html + head + title QnA + = csrf_meta_tags + = csp_meta_tag + = stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' + = javascript_include_tag 'application', 'data-turbolinks-track': 'reload' + body + div class="navbar navbar-expand-sm navbar-light bg-light" + = render 'shared/user_nav' + div class="container" + div class="mt-2 row justify-content-center" + div class="col col-sm-6" + p.notice= notice + p.alert= alert + = yield diff --git a/app/views/questions/_question_for_show.html.slim b/app/views/questions/_question_for_show.html.slim index fe7272f..4379104 100644 --- a/app/views/questions/_question_for_show.html.slim +++ b/app/views/questions/_question_for_show.html.slim @@ -1,11 +1,15 @@ h4= @question.title -p= @question.body -=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) +p class="mb-1"= @question.body +p class="mb-1"=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) .question-errors =render 'shared/errors', resource: @question = form_with model: @question, class: 'hidden', html: { id: "edit-question-#{@question.id}" } do |f| - = f.label :title - = f.text_field :title - = f.label :body, 'Body' - = f.text_area :body + = f.label :title, class: "mb-0" + br + = f.text_field :title, size: 32, class: "mb-2" + br + = f.label :body, 'Body', class: "mb-0" + br + = f.text_area :body, cols: 32, rows: 3 + br = f.submit 'Update' diff --git a/app/views/questions/show.html.slim b/app/views/questions/show.html.slim index 05a8929..9c38f7c 100644 --- a/app/views/questions/show.html.slim +++ b/app/views/questions/show.html.slim @@ -1,12 +1,13 @@ div id="question_#{@question.id}" = render 'question_for_show', resource: @question -h4 Answers +h5 class="mt-3" Answers div.answers = render @question.answers -p Your answer .answer-errors = render 'shared/errors', resource: @answer = form_with model: [@question, @answer], class: 'new-answer' do |f| - = f.label :body - = f.text_area :body + = f.label :body, 'Your answer', class: "mb-0" + br + = f.text_area :body, cols: 32, rows: 3 + br = f.submit 'Leave' diff --git a/app/views/shared/_answer_links.html.slim b/app/views/shared/_answer_links.html.slim index 6d20456..4cc1dda 100644 --- a/app/views/shared/_answer_links.html.slim +++ b/app/views/shared/_answer_links.html.slim @@ -1,7 +1,6 @@ -p - => link_to('Mark as the best', mark_answer_path(answer), method: :post, remote: true) \ +=> link_to('Mark as the best', mark_answer_path(answer), method: :post, remote: true) \ if current_user&.owner?(answer.question) && !answer.best? - => link_to('Edit', '#', class: 'edit-answer-links', data: { answer_id: answer.id }) \ +=> link_to('Edit', '#', class: 'edit-answer-links', data: { answer_id: answer.id }) \ if current_user&.owner?(answer) - = link_to('Delete', answer_path(answer), method: :delete, remote: true) \ += link_to('Delete', answer_path(answer), method: :delete, remote: true) \ if current_user&.owner?(answer) diff --git a/app/views/shared/_question_links.html.slim b/app/views/shared/_question_links.html.slim index 881b8bf..b45c98a 100644 --- a/app/views/shared/_question_links.html.slim +++ b/app/views/shared/_question_links.html.slim @@ -1,3 +1,2 @@ -p - => link_to 'Edit', '#', class: 'edit-question-link', data: { question_id: question.id } - = link_to 'Delete', question_path(question), method: :delete +=> link_to 'Edit', '#', class: 'edit-question-link', data: { question_id: question.id } += link_to 'Delete', question_path(question), method: :delete diff --git a/app/views/shared/_user_nav.html.slim b/app/views/shared/_user_nav.html.slim index 074341a..d3aa0ab 100644 --- a/app/views/shared/_user_nav.html.slim +++ b/app/views/shared/_user_nav.html.slim @@ -1,7 +1,6 @@ -- if current_user - p= link_to 'Log out', destroy_user_session_path, method: :delete -- else - p - = link_to 'Log in', new_user_session_path - | |  - = link_to 'Sign up', new_user_registration_path +p class="mb-0" + - if current_user + => link_to 'Log out', destroy_user_session_path, method: :delete + - else + => link_to 'Log in', new_user_session_path + = link_to 'Sign up', new_user_registration_path diff --git a/spec/features/answer/create_spec.rb b/spec/features/answer/create_spec.rb index e1d76a0..cee51f8 100644 --- a/spec/features/answer/create_spec.rb +++ b/spec/features/answer/create_spec.rb @@ -14,7 +14,7 @@ end scenario 'answers to a question', js: true do - fill_in 'Body', with: "#{"body" * 25}" + fill_in 'answer_body', with: "#{"body" * 25}" click_on 'Leave' expect(page).to have_content "#{"body" * 25}" @@ -38,10 +38,8 @@ scenario 'unauthenticated user tries to get an answer' do visit question_path(question) - within ".new-answer" do - fill_in 'Body', with: "#{"body" * 25}" - click_on 'Leave' - end + fill_in 'answer_body', with: "#{"body" * 25}" + click_on 'Leave' expect(page).to have_content 'You need to sign in or sign up before continuing.' end From 92b3123d8ca754d655823d32a02920b9722ec922 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Thu, 27 Jun 2019 15:37:02 +0300 Subject: [PATCH 02/12] Style. --- app/views/layouts/application.html.slim | 6 ++++-- app/views/questions/_question.html.slim | 3 ++- app/views/questions/_question_for_show.html.slim | 2 +- app/views/questions/index.html.slim | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/views/layouts/application.html.slim b/app/views/layouts/application.html.slim index b244244..4319e1b 100644 --- a/app/views/layouts/application.html.slim +++ b/app/views/layouts/application.html.slim @@ -12,6 +12,8 @@ html div class="container" div class="mt-2 row justify-content-center" div class="col col-sm-6" - p.notice= notice - p.alert= alert + - if flash[:notice].present? + p.notice= notice + - if flash[:alert].present? + p.alert= alert = yield diff --git a/app/views/questions/_question.html.slim b/app/views/questions/_question.html.slim index deb0b83..2f7e2b7 100644 --- a/app/views/questions/_question.html.slim +++ b/app/views/questions/_question.html.slim @@ -1 +1,2 @@ -p= question.title +p + = link_to question.title, question_path(question) diff --git a/app/views/questions/_question_for_show.html.slim b/app/views/questions/_question_for_show.html.slim index 4379104..c209622 100644 --- a/app/views/questions/_question_for_show.html.slim +++ b/app/views/questions/_question_for_show.html.slim @@ -1,4 +1,4 @@ -h4= @question.title +h4 class="mt-2"= @question.title p class="mb-1"= @question.body p class="mb-1"=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) .question-errors diff --git a/app/views/questions/index.html.slim b/app/views/questions/index.html.slim index c7fcf13..d0c9d6b 100644 --- a/app/views/questions/index.html.slim +++ b/app/views/questions/index.html.slim @@ -1,2 +1,2 @@ -p= render @questions +p.questions= render @questions p= link_to 'Ask a question', new_question_path From 5fa3d0fb72532702bdba76467d39a67b28b4c033 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Thu, 27 Jun 2019 18:37:48 +0300 Subject: [PATCH 03/12] Implement scenario 'Authenticated user asks a question with attached files'. --- app/assets/javascripts/questions.js | 2 +- app/controllers/questions_controller.rb | 4 +-- app/models/question.rb | 2 ++ app/views/answers/_answer.html.slim | 8 ++--- .../questions/_question_for_show.html.slim | 36 +++++++++++-------- app/views/questions/new.html.slim | 15 ++++---- app/views/questions/show.html.slim | 14 ++++---- app/views/questions/update.js.erb | 2 +- ...te_active_storage_tables.active_storage.rb | 27 ++++++++++++++ db/schema.rb | 24 ++++++++++++- spec/features/question/create_spec.rb | 10 ++++++ spec/models/question_spec.rb | 4 +++ spec/rails_helper.rb | 3 ++ 13 files changed, 114 insertions(+), 37 deletions(-) create mode 100644 db/migrate/20190627131211_create_active_storage_tables.active_storage.rb diff --git a/app/assets/javascripts/questions.js b/app/assets/javascripts/questions.js index 44b3cdb..9fb60c8 100644 --- a/app/assets/javascripts/questions.js +++ b/app/assets/javascripts/questions.js @@ -1,5 +1,5 @@ $(document).on('turbolinks:load', function() { - $('.edit-question-link').on('click', function(e) { + $('.question').on('click', '.edit-question-link', function(e) { e.preventDefault() $(this).hide() var questionId = $(this).data('questionId') diff --git a/app/controllers/questions_controller.rb b/app/controllers/questions_controller.rb index f23fa9d..671a466 100644 --- a/app/controllers/questions_controller.rb +++ b/app/controllers/questions_controller.rb @@ -42,10 +42,10 @@ def destroy private def set_question - @question = Question.find(params[:id]) + @question = Question.with_attached_files.find(params[:id]) end def question_params - params.require(:question).permit(:title, :body) + params.require(:question).permit(:title, :body, files: []) end end diff --git a/app/models/question.rb b/app/models/question.rb index 19dcf3b..4a22b57 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -2,6 +2,8 @@ class Question < ApplicationRecord belongs_to :user has_many :answers, dependent: :destroy + has_many_attached :files + validates :title, presence: true, length: { in: 15..75 }, uniqueness: { case_sensitive: false } diff --git a/app/views/answers/_answer.html.slim b/app/views/answers/_answer.html.slim index 99e5be5..dc64f69 100644 --- a/app/views/answers/_answer.html.slim +++ b/app/views/answers/_answer.html.slim @@ -7,8 +7,6 @@ div id="answer_#{answer.id}" class="mb-3" p class="mb-1"= render(partial: 'shared/answer_links', locals: { answer: answer }) = form_with model: answer, class: 'hidden', html: { id: "edit-answer-#{answer.id}" } do |f| - = f.label :body, 'Your answer', class: "mb-0" - br - = f.text_area :body, cols: 32, rows: 3 - br - = f.submit 'Update' + div= f.label :body, 'Your answer', class: "mb-0" + div= f.text_area :body, cols: 32, rows: 3 + div= f.submit 'Update' diff --git a/app/views/questions/_question_for_show.html.slim b/app/views/questions/_question_for_show.html.slim index c209622..601cb4b 100644 --- a/app/views/questions/_question_for_show.html.slim +++ b/app/views/questions/_question_for_show.html.slim @@ -1,15 +1,21 @@ -h4 class="mt-2"= @question.title -p class="mb-1"= @question.body -p class="mb-1"=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) -.question-errors - =render 'shared/errors', resource: @question -= form_with model: @question, class: 'hidden', html: { id: "edit-question-#{@question.id}" } do |f| - = f.label :title, class: "mb-0" - br - = f.text_field :title, size: 32, class: "mb-2" - br - = f.label :body, 'Body', class: "mb-0" - br - = f.text_area :body, cols: 32, rows: 3 - br - = f.submit 'Update' +div id="question_#{@question.id}" + h4 class="mt-2"= @question.title + p class="mb-1"= @question.body + + - if @question.files.attached? + - @question.files.each do |file| + p= link_to file.filename.to_s, url_for(file) + + p class="mb-1"=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) + + .question-errors + =render 'shared/errors', resource: @question + + = form_with model: @question, class: 'hidden', html: { id: "edit-question-#{@question.id}" } do |f| + div= f.label :title, class: "mb-0" + div= f.text_field :title, size: 29, class: "mb-2" + div= f.label :body, 'Body', class: "mb-0" + div= f.text_area :body, cols: 32, rows: 3 + div= f.label :files, 'Files', class: "mb-0" + div= f.file_field :files, multiple: true, class: "mb-3" + div= f.submit 'Update' diff --git a/app/views/questions/new.html.slim b/app/views/questions/new.html.slim index 6469bfe..bfe727e 100644 --- a/app/views/questions/new.html.slim +++ b/app/views/questions/new.html.slim @@ -1,8 +1,11 @@ = render 'shared/errors', resource: @question -= form_with model: @question, local: true do |f| - = f.label :title - = f.text_field :title - = f.label :body - = f.text_area :body - = f.submit 'Ask' += form_with model: @question do |f| + div class="form-group" + div= f.label :title, class: "mb-0" + div= f.text_field :title, size: 29, class: "mb-2" + div= f.label :body, 'Body', class: "mb-0" + div= f.text_area :body, cols: 32, rows: 3 + div= f.label :files, 'Files', class: "mb-0" + div= f.file_field :files, multiple: true, class: "mb-3" + div= f.submit 'Ask' diff --git a/app/views/questions/show.html.slim b/app/views/questions/show.html.slim index 9c38f7c..dba436c 100644 --- a/app/views/questions/show.html.slim +++ b/app/views/questions/show.html.slim @@ -1,13 +1,15 @@ -div id="question_#{@question.id}" +.question = render 'question_for_show', resource: @question + h5 class="mt-3" Answers + div.answers = render @question.answers + .answer-errors = render 'shared/errors', resource: @answer + = form_with model: [@question, @answer], class: 'new-answer' do |f| - = f.label :body, 'Your answer', class: "mb-0" - br - = f.text_area :body, cols: 32, rows: 3 - br - = f.submit 'Leave' + div= f.label :body, 'Your answer', class: "mb-0" + div= f.text_area :body, cols: 32, rows: 3 + div= f.submit 'Leave' diff --git a/app/views/questions/update.js.erb b/app/views/questions/update.js.erb index a085def..02dec75 100644 --- a/app/views/questions/update.js.erb +++ b/app/views/questions/update.js.erb @@ -1,4 +1,4 @@ $('.question-errors').html('<%= render 'shared/errors', resource: @question %>') <% if !@question.errors.present? %> - $("#question_" + <%= @question.id %>).html('<%= j render 'question_for_show', resource: @question %>') + $("#question_" + <%= @question.id %>).replaceWith('<%= j render 'question_for_show', resource: @question %>') <% end %> diff --git a/db/migrate/20190627131211_create_active_storage_tables.active_storage.rb b/db/migrate/20190627131211_create_active_storage_tables.active_storage.rb new file mode 100644 index 0000000..0b2ce25 --- /dev/null +++ b/db/migrate/20190627131211_create_active_storage_tables.active_storage.rb @@ -0,0 +1,27 @@ +# This migration comes from active_storage (originally 20170806125915) +class CreateActiveStorageTables < ActiveRecord::Migration[5.2] + def change + create_table :active_storage_blobs do |t| + t.string :key, null: false + t.string :filename, null: false + t.string :content_type + t.text :metadata + t.bigint :byte_size, null: false + t.string :checksum, null: false + t.datetime :created_at, null: false + + t.index [ :key ], unique: true + end + + create_table :active_storage_attachments do |t| + t.string :name, null: false + t.references :record, null: false, polymorphic: true, index: false + t.references :blob, null: false + + t.datetime :created_at, null: false + + t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true + t.foreign_key :active_storage_blobs, column: :blob_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 199c4e9..06a8ee7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,11 +10,32 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_06_24_142356) do +ActiveRecord::Schema.define(version: 2019_06_27_131211) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" + create_table "active_storage_attachments", force: :cascade do |t| + t.string "name", null: false + t.string "record_type", null: false + t.bigint "record_id", null: false + t.bigint "blob_id", null: false + t.datetime "created_at", null: false + t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id" + t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true + end + + create_table "active_storage_blobs", force: :cascade do |t| + t.string "key", null: false + t.string "filename", null: false + t.string "content_type" + t.text "metadata" + t.bigint "byte_size", null: false + t.string "checksum", null: false + t.datetime "created_at", null: false + t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true + end + create_table "answers", force: :cascade do |t| t.text "body" t.datetime "created_at", null: false @@ -47,6 +68,7 @@ t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end + add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "answers", "questions" add_foreign_key "answers", "users" add_foreign_key "questions", "users" diff --git a/spec/features/question/create_spec.rb b/spec/features/question/create_spec.rb index 3708d26..df949e0 100644 --- a/spec/features/question/create_spec.rb +++ b/spec/features/question/create_spec.rb @@ -30,6 +30,16 @@ expect(page).to have_content "Title can't be blank" end + + scenario 'asks a question with attached files' do + fill_in 'Title', with: 'Test question title' + fill_in 'Body', with: "#{"body" * 25}" + attach_file 'Files', ["#{Rails.root}/spec/rails_helper.rb", "#{Rails.root}/spec/spec_helper.rb"] + click_on 'Ask' + + expect(page).to have_link "rails_helper.rb" + expect(page).to have_link "spec_helper.rb" + end end scenario 'unauthenticated user tries to ask a question' do diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb index fc42938..171724b 100644 --- a/spec/models/question_spec.rb +++ b/spec/models/question_spec.rb @@ -12,4 +12,8 @@ it { should validate_length_of(:body).is_at_least(50) } it { should validate_uniqueness_of(:title).case_insensitive } + + it 'has many attached files' do + expect(Question.new.files).to be_an_instance_of(ActiveStorage::Attached::Many) + end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index a56e492..4842a3f 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -65,6 +65,9 @@ config.filter_rails_from_backtrace! # arbitrary gems may also be filtered via: # config.filter_gems_from_backtrace("gem name") + config.after(:all) do + FileUtils.rm_rf("#{Rails.root}/tmp/storage") + end end Shoulda::Matchers.configure do |config| From 4ad3c017cc3216c239daacc921ad0cc6012deba2 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Thu, 27 Jun 2019 18:41:34 +0300 Subject: [PATCH 04/12] Fix New view for Questions. --- app/views/questions/new.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/questions/new.html.slim b/app/views/questions/new.html.slim index bfe727e..d422231 100644 --- a/app/views/questions/new.html.slim +++ b/app/views/questions/new.html.slim @@ -1,6 +1,6 @@ = render 'shared/errors', resource: @question -= form_with model: @question do |f| += form_with model: @question, local: true do |f| div class="form-group" div= f.label :title, class: "mb-0" div= f.text_field :title, size: 29, class: "mb-2" From 3dbd869e87ea33ac4d9e7057495c0279df933da9 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Fri, 28 Jun 2019 15:50:41 +0300 Subject: [PATCH 05/12] Implement scenario 'user answers to a question with attached files'. --- app/controllers/answers_controller.rb | 2 +- app/models/answer.rb | 2 ++ app/models/question.rb | 2 ++ app/views/answers/_answer.html.slim | 7 +++++++ app/views/questions/show.html.slim | 2 ++ spec/features/answer/create_spec.rb | 9 +++++++++ spec/models/answer_spec.rb | 4 ++++ 7 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb index 31aeb26..5a2cf55 100644 --- a/app/controllers/answers_controller.rb +++ b/app/controllers/answers_controller.rb @@ -33,7 +33,7 @@ def mark private def answer_params - params.require(:answer).permit(:body) + params.require(:answer).permit(:body, files: []) end def set_answer diff --git a/app/models/answer.rb b/app/models/answer.rb index 871d79c..42ddfa4 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -2,6 +2,8 @@ class Answer < ApplicationRecord belongs_to :user belongs_to :question + has_many_attached :files + default_scope { order(best: :desc, created_at: :asc) } scope :best, -> { where(best: true) } diff --git a/app/models/question.rb b/app/models/question.rb index 4a22b57..b760a36 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -4,6 +4,8 @@ class Question < ApplicationRecord has_many_attached :files + default_scope { order(:created_at) } + validates :title, presence: true, length: { in: 15..75 }, uniqueness: { case_sensitive: false } diff --git a/app/views/answers/_answer.html.slim b/app/views/answers/_answer.html.slim index dc64f69..a0a7763 100644 --- a/app/views/answers/_answer.html.slim +++ b/app/views/answers/_answer.html.slim @@ -1,9 +1,16 @@ div id="answer_#{answer.id}" class="mb-3" - if answer.persisted? + - if answer.best? div id="best-answer" h5 The best answer + p class="mb-1"= answer.body + + - if answer.files.attached? + - answer.files.each do |file| + p= link_to file.filename.to_s, url_for(file) + p class="mb-1"= render(partial: 'shared/answer_links', locals: { answer: answer }) = form_with model: answer, class: 'hidden', html: { id: "edit-answer-#{answer.id}" } do |f| diff --git a/app/views/questions/show.html.slim b/app/views/questions/show.html.slim index dba436c..62d8a07 100644 --- a/app/views/questions/show.html.slim +++ b/app/views/questions/show.html.slim @@ -12,4 +12,6 @@ div.answers = form_with model: [@question, @answer], class: 'new-answer' do |f| div= f.label :body, 'Your answer', class: "mb-0" div= f.text_area :body, cols: 32, rows: 3 + div= f.label :files, 'Files', class: "mb-0" + div= f.file_field :files, multiple: true, class: "mb-3" div= f.submit 'Leave' diff --git a/spec/features/answer/create_spec.rb b/spec/features/answer/create_spec.rb index cee51f8..e6729f9 100644 --- a/spec/features/answer/create_spec.rb +++ b/spec/features/answer/create_spec.rb @@ -26,6 +26,15 @@ expect(page).to have_content "Body can't be blank" expect(page).to have_content 'Body is too short (minimum is 50 characters)' end + + scenario 'answers to a question with attached files', js: true do + fill_in 'answer_body', with: "#{"body" * 25}" + attach_file 'Files', ["#{Rails.root}/spec/rails_helper.rb", "#{Rails.root}/spec/spec_helper.rb"] + click_on 'Leave' + + expect(page).to have_link "rails_helper.rb" + expect(page).to have_link "spec_helper.rb" + end end feature 'only authenticated user can create answers', %q{ diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb index 348157e..7104a27 100644 --- a/spec/models/answer_spec.rb +++ b/spec/models/answer_spec.rb @@ -27,4 +27,8 @@ expect(answer1.question.answers).to eq [answer1, answer2] end end + + it 'has many attached files' do + expect(Answer.new.files).to be_an_instance_of(ActiveStorage::Attached::Many) + end end From dd621c93f90b04c751fccf976d5361970c0a473d Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Fri, 28 Jun 2019 16:24:36 +0300 Subject: [PATCH 06/12] Implement scenario 'user edits his question with attaching files'. --- spec/features/question/edit_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/features/question/edit_spec.rb b/spec/features/question/edit_spec.rb index 6537234..6ff5bac 100644 --- a/spec/features/question/edit_spec.rb +++ b/spec/features/question/edit_spec.rb @@ -40,6 +40,19 @@ end end + scenario 'edits his question with attaching files', js: true do + visit question_path(question1) + + within "#question_#{question1.id}" do + click_on 'Edit' + attach_file 'Files', ["#{Rails.root}/spec/rails_helper.rb", "#{Rails.root}/spec/spec_helper.rb"] + click_on 'Update' + + expect(page).to have_link "rails_helper.rb" + expect(page).to have_link "spec_helper.rb" + end + end + scenario 'tries to edit his question with errors', js: true do visit question_path(question1) From d9e02b60cabbf27f8ebb105b55099e3be92f3103 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Fri, 28 Jun 2019 16:39:14 +0300 Subject: [PATCH 07/12] Implement scenario 'edits his answer with adding attached files'. --- app/views/answers/_answer.html.slim | 7 +++++-- app/views/questions/_question_for_show.html.slim | 5 +++-- spec/features/answer/edit_spec.rb | 11 +++++++++++ spec/features/question/edit_spec.rb | 2 +- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/views/answers/_answer.html.slim b/app/views/answers/_answer.html.slim index a0a7763..e9a2986 100644 --- a/app/views/answers/_answer.html.slim +++ b/app/views/answers/_answer.html.slim @@ -8,12 +8,15 @@ div id="answer_#{answer.id}" class="mb-3" p class="mb-1"= answer.body - if answer.files.attached? - - answer.files.each do |file| - p= link_to file.filename.to_s, url_for(file) + div class="answer-files mb-1" + - answer.files.each do |file| + p.mb-0= link_to file.filename.to_s, url_for(file) p class="mb-1"= render(partial: 'shared/answer_links', locals: { answer: answer }) = form_with model: answer, class: 'hidden', html: { id: "edit-answer-#{answer.id}" } do |f| div= f.label :body, 'Your answer', class: "mb-0" div= f.text_area :body, cols: 32, rows: 3 + div= f.label :files, 'Files', class: "mb-0" + div= f.file_field :files, multiple: true, class: "mb-3" div= f.submit 'Update' diff --git a/app/views/questions/_question_for_show.html.slim b/app/views/questions/_question_for_show.html.slim index 601cb4b..89618d2 100644 --- a/app/views/questions/_question_for_show.html.slim +++ b/app/views/questions/_question_for_show.html.slim @@ -3,8 +3,9 @@ div id="question_#{@question.id}" p class="mb-1"= @question.body - if @question.files.attached? - - @question.files.each do |file| - p= link_to file.filename.to_s, url_for(file) + div class="question-files mb-1" + - @question.files.each do |file| + p.mb-0= link_to file.filename.to_s, url_for(file) p class="mb-1"=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) diff --git a/spec/features/answer/edit_spec.rb b/spec/features/answer/edit_spec.rb index 0e5c5a8..f071a44 100644 --- a/spec/features/answer/edit_spec.rb +++ b/spec/features/answer/edit_spec.rb @@ -38,6 +38,17 @@ end end + scenario 'edits his answer with adding attached files', js: true do + within "#answer_#{answer.id}" do + click_on 'Edit' + attach_file 'Files', ["#{Rails.root}/spec/rails_helper.rb", "#{Rails.root}/spec/spec_helper.rb"] + click_on 'Update' + + expect(page).to have_link "rails_helper.rb" + expect(page).to have_link "spec_helper.rb" + end + end + scenario 'tries to edit his answer with errors', js: true do within "#answer_#{answer.id}" do click_on 'Edit' diff --git a/spec/features/question/edit_spec.rb b/spec/features/question/edit_spec.rb index 6ff5bac..270561c 100644 --- a/spec/features/question/edit_spec.rb +++ b/spec/features/question/edit_spec.rb @@ -40,7 +40,7 @@ end end - scenario 'edits his question with attaching files', js: true do + scenario 'edits his question with adding attached files', js: true do visit question_path(question1) within "#question_#{question1.id}" do From 612f11e91a270213f44aac933da8df29b09ad1ef Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Sat, 29 Jun 2019 01:17:54 +0300 Subject: [PATCH 08/12] Implement feature 'user can delete attachments of his question'. --- app/assets/javascripts/files.coffee | 3 ++ app/assets/stylesheets/files.scss | 3 ++ app/controllers/files_controller.rb | 7 ++++ app/helpers/files_helper.rb | 2 + app/views/files/destroy.js.erb | 4 ++ .../questions/_question_for_show.html.slim | 6 ++- config/routes.rb | 3 ++ spec/controllers/files_controller_spec.rb | 40 ++++++++++++++++++ spec/factories/questions.rb | 4 ++ .../question/delete_attacments_spec.rb | 41 +++++++++++++++++++ 10 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/files.coffee create mode 100644 app/assets/stylesheets/files.scss create mode 100644 app/controllers/files_controller.rb create mode 100644 app/helpers/files_helper.rb create mode 100644 app/views/files/destroy.js.erb create mode 100644 spec/controllers/files_controller_spec.rb create mode 100644 spec/features/question/delete_attacments_spec.rb diff --git a/app/assets/javascripts/files.coffee b/app/assets/javascripts/files.coffee new file mode 100644 index 0000000..24f83d1 --- /dev/null +++ b/app/assets/javascripts/files.coffee @@ -0,0 +1,3 @@ +# Place all the behaviors and hooks related to the matching controller here. +# All this logic will automatically be available in application.js. +# You can use CoffeeScript in this file: http://coffeescript.org/ diff --git a/app/assets/stylesheets/files.scss b/app/assets/stylesheets/files.scss new file mode 100644 index 0000000..ee0acc7 --- /dev/null +++ b/app/assets/stylesheets/files.scss @@ -0,0 +1,3 @@ +// Place all the styles related to the Files controller here. +// They will automatically be included in application.css. +// You can use Sass (SCSS) here: http://sass-lang.com/ diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb new file mode 100644 index 0000000..a37941d --- /dev/null +++ b/app/controllers/files_controller.rb @@ -0,0 +1,7 @@ +class FilesController < ApplicationController + def destroy + @file = ActiveStorage::Attachment.find(params[:id]) + return head :forbidden unless @file.record_type.in?(%w[Question Answer]) + @file.purge if current_user&.owner?(@file.record) + end +end diff --git a/app/helpers/files_helper.rb b/app/helpers/files_helper.rb new file mode 100644 index 0000000..e9da4f6 --- /dev/null +++ b/app/helpers/files_helper.rb @@ -0,0 +1,2 @@ +module FilesHelper +end diff --git a/app/views/files/destroy.js.erb b/app/views/files/destroy.js.erb new file mode 100644 index 0000000..7d74cae --- /dev/null +++ b/app/views/files/destroy.js.erb @@ -0,0 +1,4 @@ + <% if @file.record_type == "Question" %> + <% @question = @file.record %> + $("#question_" + <%= @question.id %>).replaceWith('<%= j render 'questions/question_for_show', resource: @question %>') + <% end %> diff --git a/app/views/questions/_question_for_show.html.slim b/app/views/questions/_question_for_show.html.slim index 89618d2..2e68ce9 100644 --- a/app/views/questions/_question_for_show.html.slim +++ b/app/views/questions/_question_for_show.html.slim @@ -5,9 +5,11 @@ div id="question_#{@question.id}" - if @question.files.attached? div class="question-files mb-1" - @question.files.each do |file| - p.mb-0= link_to file.filename.to_s, url_for(file) + p.mb-0 + = link_to file.filename.to_s, url_for(file) + =< link_to('Delete file', file_path(file), method: :delete, remote: true) if current_user&.owner?(@question) - p class="mb-1"=render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) + p class="mb-1"= render(partial: 'shared/question_links', locals: { question: @question }) if current_user&.owner?(@question) .question-errors =render 'shared/errors', resource: @question diff --git a/config/routes.rb b/config/routes.rb index 4175820..6979ce2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ Rails.application.routes.draw do + get 'files/destroy' devise_for :users root to: "questions#index" @@ -8,5 +9,7 @@ post :mark end end + + resources :files, shallow: true, only: %i[destroy] end end diff --git a/spec/controllers/files_controller_spec.rb b/spec/controllers/files_controller_spec.rb new file mode 100644 index 0000000..ea925e4 --- /dev/null +++ b/spec/controllers/files_controller_spec.rb @@ -0,0 +1,40 @@ +require 'rails_helper' + +RSpec.describe FilesController, type: :controller do + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:question1) { create(:question, :with_attachments, user: user1) } + let(:question2) { create(:question, :with_attachments, user: user2) } + let!(:attachment1) { question1.files.first } + let!(:attachment2) { question2.files.first } + + describe "DELETE #destroy" do + context "authenticated user" do + before { login(user1) } + + it "deletes users's question attachment" do + expect { + delete :destroy, params: { id: attachment1 }, format: :js + }.to change(question1.files.attachments, :count).by(-1) + end + + it "tries to delete another user's question attachment" do + expect { + delete :destroy, params: { id: attachment2 }, format: :js + }.to_not change(question2.files.attachments, :count) + end + + it "render destroy template" do + delete :destroy, params: { id: attachment1 }, format: :js + + expect(response).to render_template :destroy + end + end + + it "unauthenticated user tries to delete question attachment" do + expect { + delete :destroy, params: { id: attachment1 }, format: :js + }.to_not change(question1.files.attachments, :count) + end + end +end diff --git a/spec/factories/questions.rb b/spec/factories/questions.rb index 525f50e..ba2910e 100644 --- a/spec/factories/questions.rb +++ b/spec/factories/questions.rb @@ -10,5 +10,9 @@ trait :invalid do title { "Title" } end + + trait :with_attachments do + files { [Rack::Test::UploadedFile.new(Rails.root.join('spec/rails_helper.rb'), 'text/plain')] } + end end end diff --git a/spec/features/question/delete_attacments_spec.rb b/spec/features/question/delete_attacments_spec.rb new file mode 100644 index 0000000..07a322b --- /dev/null +++ b/spec/features/question/delete_attacments_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +feature "user can delete attachments of his question", %q{ + as an authenticated user + i'd like to be able to delete only my own questions attachments +} do + given(:user1) { create(:user) } + given(:user2) { create(:user) } + given!(:question1) { create(:question, :with_attachments, user: user1) } + given!(:question2) { create(:question, :with_attachments, user: user2) } + + scenario "unauthenticated user tries to delete question attachments" do + visit question_path(question1) + + expect(page).to_not have_link 'Delete file' + end + + context "authenticated user" do + background do + login(user1) + end + + scenario "deletes attached files of his question", js: true do + visit question_path(question1) + + within "#question_#{question1.id}" do + click_on 'Delete file' + + expect(page).to_not have_link "rails_helper.rb" + end + end + + scenario "deletes attached files of his question" do + visit question_path(question2) + + within "#question_#{question2.id}" do + expect(page).to_not have_link "Delete file" + end + end + end +end From d51f0b5798d46d07dd7f056421e0c81986daf8eb Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Sat, 29 Jun 2019 02:09:22 +0300 Subject: [PATCH 09/12] Implement feature 'user can delete attachments of his answer'. --- app/views/answers/_answer.html.slim | 4 +- app/views/files/destroy.js.erb | 11 +++-- spec/controllers/files_controller_spec.rb | 42 +++++++++++++--- spec/factories/answers.rb | 4 ++ .../answer/delete_attachments_spec.rb | 49 +++++++++++++++++++ .../question/delete_attacments_spec.rb | 2 +- 6 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 spec/features/answer/delete_attachments_spec.rb diff --git a/app/views/answers/_answer.html.slim b/app/views/answers/_answer.html.slim index e9a2986..01f9bec 100644 --- a/app/views/answers/_answer.html.slim +++ b/app/views/answers/_answer.html.slim @@ -10,7 +10,9 @@ div id="answer_#{answer.id}" class="mb-3" - if answer.files.attached? div class="answer-files mb-1" - answer.files.each do |file| - p.mb-0= link_to file.filename.to_s, url_for(file) + p.mb-0 + = link_to file.filename.to_s, url_for(file) + =< link_to('Delete file', file_path(file), method: :delete, remote: true) if current_user&.owner?(answer) p class="mb-1"= render(partial: 'shared/answer_links', locals: { answer: answer }) diff --git a/app/views/files/destroy.js.erb b/app/views/files/destroy.js.erb index 7d74cae..bf9edbb 100644 --- a/app/views/files/destroy.js.erb +++ b/app/views/files/destroy.js.erb @@ -1,4 +1,7 @@ - <% if @file.record_type == "Question" %> - <% @question = @file.record %> - $("#question_" + <%= @question.id %>).replaceWith('<%= j render 'questions/question_for_show', resource: @question %>') - <% end %> +<% if @file.record_type == "Question" %> + <% @question = @file.record %> + $("#question_" + <%= @question.id %>).replaceWith('<%= j render 'questions/question_for_show', resource: @question %>') +<% elsif @file.record_type == "Answer" %> + <% @answer = @file.record %> + $("#answer_" + <%= @answer.id %>).replaceWith('<%= j render 'answers/answer', answer: @answer %>') +<% end %> diff --git a/spec/controllers/files_controller_spec.rb b/spec/controllers/files_controller_spec.rb index ea925e4..75fcd6f 100644 --- a/spec/controllers/files_controller_spec.rb +++ b/spec/controllers/files_controller_spec.rb @@ -5,8 +5,12 @@ let(:user2) { create(:user) } let(:question1) { create(:question, :with_attachments, user: user1) } let(:question2) { create(:question, :with_attachments, user: user2) } - let!(:attachment1) { question1.files.first } - let!(:attachment2) { question2.files.first } + let(:answer1) { create(:answer, :with_attachments, question: question1, user: user2) } + let(:answer2) { create(:answer, :with_attachments, question: question2, user: user1) } + let!(:question_attachment1) { question1.files.first } + let!(:question_attachment2) { question2.files.first } + let!(:answer_attachment1) { answer1.files.first } + let!(:answer_attachment2) { answer2.files.first } describe "DELETE #destroy" do context "authenticated user" do @@ -14,18 +18,36 @@ it "deletes users's question attachment" do expect { - delete :destroy, params: { id: attachment1 }, format: :js + delete :destroy, params: { id: question_attachment1 }, format: :js }.to change(question1.files.attachments, :count).by(-1) end + it "deletes users's answer attachment" do + expect { + delete :destroy, params: { id: answer_attachment2 }, format: :js + }.to change(answer2.files.attachments, :count).by(-1) + end + it "tries to delete another user's question attachment" do expect { - delete :destroy, params: { id: attachment2 }, format: :js + delete :destroy, params: { id: question_attachment2 }, format: :js }.to_not change(question2.files.attachments, :count) end - it "render destroy template" do - delete :destroy, params: { id: attachment1 }, format: :js + it "tries to delete another user's answer attachment" do + expect { + delete :destroy, params: { id: answer_attachment1 }, format: :js + }.to_not change(answer1.files.attachments, :count) + end + + it "render destroy template after deleting of question attachment" do + delete :destroy, params: { id: question_attachment1 }, format: :js + + expect(response).to render_template :destroy + end + + it "render destroy template after deleting of answer attachment" do + delete :destroy, params: { id: answer_attachment2 }, format: :js expect(response).to render_template :destroy end @@ -33,8 +55,14 @@ it "unauthenticated user tries to delete question attachment" do expect { - delete :destroy, params: { id: attachment1 }, format: :js + delete :destroy, params: { id: question_attachment1 }, format: :js }.to_not change(question1.files.attachments, :count) end + + it "unauthenticated user tries to delete answer attachment" do + expect { + delete :destroy, params: { id: answer_attachment1 }, format: :js + }.to_not change(answer1.files.attachments, :count) + end end end diff --git a/spec/factories/answers.rb b/spec/factories/answers.rb index 6ea985c..0a80840 100644 --- a/spec/factories/answers.rb +++ b/spec/factories/answers.rb @@ -7,5 +7,9 @@ trait :invalid do body { "#{"b" * 49}" } end + + trait :with_attachments do + files { [Rack::Test::UploadedFile.new(Rails.root.join('spec/rails_helper.rb'), 'text/plain')] } + end end end diff --git a/spec/features/answer/delete_attachments_spec.rb b/spec/features/answer/delete_attachments_spec.rb new file mode 100644 index 0000000..029ab3f --- /dev/null +++ b/spec/features/answer/delete_attachments_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' + +feature "user can delete attachments of his answer", %q{ + as an authenticated user + i'd like to be able to delete only my own answers attachments +} do + given(:user1) { create(:user) } + given(:user2) { create(:user) } + given!(:question1) { create(:question, user: user1) } + given!(:question2) { create(:question, user: user2) } + given!(:answer1) { create(:answer, + :with_attachments, + question: question1, + user: user2) } + given!(:answer2) { create(:answer, + :with_attachments, + question: question2, + user: user1) } + + scenario "unauthenticated user tries to delete answer attachments" do + visit question_path(question1) + + expect(page).to_not have_link 'Delete file' + end + + context "authenticated user" do + background do + login(user1) + end + + scenario "deletes attached files of his answer", js: true do + visit question_path(question2) + + within "#answer_#{answer2.id}" do + click_on 'Delete file' + + expect(page).to_not have_link "rails_helper.rb" + end + end + + scenario "tries to delete attached files of another user's answer" do + visit question_path(question1) + + within "#answer_#{answer1.id}" do + expect(page).to_not have_link "Delete file" + end + end + end +end diff --git a/spec/features/question/delete_attacments_spec.rb b/spec/features/question/delete_attacments_spec.rb index 07a322b..a405442 100644 --- a/spec/features/question/delete_attacments_spec.rb +++ b/spec/features/question/delete_attacments_spec.rb @@ -30,7 +30,7 @@ end end - scenario "deletes attached files of his question" do + scenario "tries to delete attached files of another user's question" do visit question_path(question2) within "#question_#{question2.id}" do From 05620c082ee4005c0cd13fd1419b1cfda35f0a9f Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Sat, 29 Jun 2019 14:53:31 +0300 Subject: [PATCH 10/12] Set Google Cloud Storage as a default storage system in development environment. --- .gitignore | 3 ++ Gemfile | 2 ++ Gemfile.lock | 56 ++++++++++++++++++++++++++++-- config/environments/development.rb | 2 +- config/storage.yml | 10 +++--- 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 4f30042..f01e63d 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,6 @@ # Ignore database settings /config/database.yml + +# Ignore secret configs +/config/secrets/* diff --git a/Gemfile b/Gemfile index 31cb56e..5e4ae75 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,8 @@ gem 'devise' gem 'jquery-rails' # UI gem 'bootstrap', '~> 4.3.1' +# Google Cloud Storage +gem "google-cloud-storage", "~> 1.11", require: false # Use CoffeeScript for .coffee assets and views gem 'coffee-rails', '~> 4.2' diff --git a/Gemfile.lock b/Gemfile.lock index 7e25859..b90fbe4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -57,7 +57,7 @@ GEM sassc-rails (>= 2.0.0) builder (3.2.3) byebug (11.0.1) - capybara (3.24.0) + capybara (3.25.0) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) @@ -76,6 +76,8 @@ GEM coffee-script-source (1.12.2) concurrent-ruby (1.1.5) crass (1.0.4) + declarative (0.0.10) + declarative-option (0.1.0) devise (4.6.2) bcrypt (~> 3.0) orm_adapter (~> 0.1) @@ -83,6 +85,7 @@ GEM responders warden (~> 1.2.3) diff-lcs (1.3) + digest-crc (0.4.1) erubi (1.8.0) execjs (2.7.0) factory_bot (5.0.2) @@ -90,9 +93,38 @@ GEM factory_bot_rails (5.0.2) factory_bot (~> 5.0.2) railties (>= 4.2.0) + faraday (0.15.4) + multipart-post (>= 1.2, < 3) ffi (1.11.1) globalid (0.4.2) activesupport (>= 4.2.0) + google-api-client (0.30.3) + addressable (~> 2.5, >= 2.5.1) + googleauth (>= 0.5, < 0.10.0) + httpclient (>= 2.8.1, < 3.0) + mini_mime (~> 1.0) + representable (~> 3.0) + retriable (>= 2.0, < 4.0) + signet (~> 0.10) + google-cloud-core (1.3.0) + google-cloud-env (~> 1.0) + google-cloud-env (1.2.0) + faraday (~> 0.11) + google-cloud-storage (1.18.2) + addressable (~> 2.5) + digest-crc (~> 0.4) + google-api-client (~> 0.26) + google-cloud-core (~> 1.2) + googleauth (>= 0.6.2, < 0.10.0) + mime-types (~> 3.0) + googleauth (0.8.1) + faraday (~> 0.12) + jwt (>= 1.4, < 3.0) + memoist (~> 0.16) + multi_json (~> 1.11) + os (>= 0.9, < 2.0) + signet (~> 0.7) + httpclient (2.8.3) i18n (1.6.0) concurrent-ruby (~> 1.0) jbuilder (2.9.1) @@ -101,6 +133,7 @@ GEM rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) + jwt (2.2.1) launchy (2.4.3) addressable (~> 2.3) listen (3.1.5) @@ -114,16 +147,23 @@ GEM mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) + memoist (0.16.0) method_source (0.9.2) + mime-types (3.2.2) + mime-types-data (~> 3.2015) + mime-types-data (3.2019.0331) mimemagic (0.3.3) mini_mime (1.0.1) mini_portile2 (2.4.0) minitest (5.11.3) msgpack (1.3.0) + multi_json (1.13.1) + multipart-post (2.1.1) nio4r (2.3.1) nokogiri (1.10.3) mini_portile2 (~> 2.4.0) orm_adapter (0.5.0) + os (1.0.1) pg (1.1.4) popper_js (1.14.5) public_suffix (3.1.1) @@ -164,10 +204,15 @@ GEM rb-inotify (0.10.0) ffi (~> 1.0) regexp_parser (1.5.1) + representable (3.0.4) + declarative (< 0.1.0) + declarative-option (< 0.2.0) + uber (< 0.2.0) responders (3.0.0) actionpack (>= 5.0) railties (>= 5.0) - rspec-core (3.8.1) + retriable (3.1.2) + rspec-core (3.8.2) rspec-support (~> 3.8.0) rspec-expectations (3.8.4) diff-lcs (>= 1.2.0, < 2.0) @@ -211,6 +256,11 @@ GEM rubyzip (~> 1.2, >= 1.2.2) shoulda-matchers (4.1.0) activesupport (>= 4.2.0) + signet (0.11.0) + addressable (~> 2.3) + faraday (~> 0.9) + jwt (>= 1.5, < 3.0) + multi_json (~> 1.10) slim (4.0.1) temple (>= 0.7.6, < 0.9) tilt (>= 2.0.6, < 2.1) @@ -238,6 +288,7 @@ GEM turbolinks-source (5.2.0) tzinfo (1.2.5) thread_safe (~> 0.1) + uber (0.1.0) uglifier (4.1.20) execjs (>= 0.3.0, < 3) warden (1.2.8) @@ -268,6 +319,7 @@ DEPENDENCIES coffee-rails (~> 4.2) devise factory_bot_rails + google-cloud-storage (~> 1.11) jbuilder (~> 2.5) jquery-rails launchy diff --git a/config/environments/development.rb b/config/environments/development.rb index 7e6af8d..c56a585 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -28,7 +28,7 @@ end # Store uploaded files on the local file system (see config/storage.yml for options) - config.active_storage.service = :local + config.active_storage.service = :google # Don't care if the mailer can't send. config.action_mailer.raise_delivery_errors = false diff --git a/config/storage.yml b/config/storage.yml index d32f76e..dc031fc 100644 --- a/config/storage.yml +++ b/config/storage.yml @@ -15,11 +15,11 @@ local: # bucket: your_own_bucket # Remember not to checkin your GCS keyfile to a repository -# google: -# service: GCS -# project: your_project -# credentials: <%= Rails.root.join("path/to/gcs.keyfile") %> -# bucket: your_own_bucket +google: + service: GCS + project: questions-and-answers-190629 + credentials: <%= Rails.root.join("config/secrets/questions-and-answers-190629.json") %> + bucket: questions_and_answers_files # Use rails credentials:edit to set the Azure Storage secret (as azure_storage:storage_access_key) # microsoft: From 65e96b47db33ac53ba05fcf4871a62748d2ab975 Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Sat, 29 Jun 2019 15:43:03 +0300 Subject: [PATCH 11/12] Add direct uploading to GCS for attachments. --- app/assets/javascripts/application.js | 1 + app/views/answers/_answer.html.slim | 2 +- app/views/questions/_question_for_show.html.slim | 2 +- app/views/questions/show.html.slim | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 69941fe..48d768d 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -16,4 +16,5 @@ //= require jquery3 //= require popper //= require bootstrap-sprockets +//= require activestorage //= require_tree . diff --git a/app/views/answers/_answer.html.slim b/app/views/answers/_answer.html.slim index 01f9bec..dac75ae 100644 --- a/app/views/answers/_answer.html.slim +++ b/app/views/answers/_answer.html.slim @@ -20,5 +20,5 @@ div id="answer_#{answer.id}" class="mb-3" div= f.label :body, 'Your answer', class: "mb-0" div= f.text_area :body, cols: 32, rows: 3 div= f.label :files, 'Files', class: "mb-0" - div= f.file_field :files, multiple: true, class: "mb-3" + div= f.file_field :files, multiple: true, direct_upload: true, class: "mb-3" div= f.submit 'Update' diff --git a/app/views/questions/_question_for_show.html.slim b/app/views/questions/_question_for_show.html.slim index 2e68ce9..3f90be7 100644 --- a/app/views/questions/_question_for_show.html.slim +++ b/app/views/questions/_question_for_show.html.slim @@ -20,5 +20,5 @@ div id="question_#{@question.id}" div= f.label :body, 'Body', class: "mb-0" div= f.text_area :body, cols: 32, rows: 3 div= f.label :files, 'Files', class: "mb-0" - div= f.file_field :files, multiple: true, class: "mb-3" + div= f.file_field :files, multiple: true, direct_upload: true, class: "mb-3" div= f.submit 'Update' diff --git a/app/views/questions/show.html.slim b/app/views/questions/show.html.slim index 62d8a07..eec6eb8 100644 --- a/app/views/questions/show.html.slim +++ b/app/views/questions/show.html.slim @@ -13,5 +13,5 @@ div.answers div= f.label :body, 'Your answer', class: "mb-0" div= f.text_area :body, cols: 32, rows: 3 div= f.label :files, 'Files', class: "mb-0" - div= f.file_field :files, multiple: true, class: "mb-3" + div= f.file_field :files, multiple: true, direct_upload: true, class: "mb-3" div= f.submit 'Leave' From 5f516dd9073210bef59234886639e1cadfed721b Mon Sep 17 00:00:00 2001 From: Oleg Sachev Date: Sat, 29 Jun 2019 23:05:17 +0300 Subject: [PATCH 12/12] Fix routes.rb. Fix tests for Files controller, for Question and Answer models. --- config/routes.rb | 1 - spec/controllers/files_controller_spec.rb | 2 ++ spec/models/answer_spec.rb | 7 +++---- spec/models/question_spec.rb | 4 +--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 6979ce2..6edefeb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,4 @@ Rails.application.routes.draw do - get 'files/destroy' devise_for :users root to: "questions#index" diff --git a/spec/controllers/files_controller_spec.rb b/spec/controllers/files_controller_spec.rb index 75fcd6f..3d4b420 100644 --- a/spec/controllers/files_controller_spec.rb +++ b/spec/controllers/files_controller_spec.rb @@ -20,12 +20,14 @@ expect { delete :destroy, params: { id: question_attachment1 }, format: :js }.to change(question1.files.attachments, :count).by(-1) + expect { question_attachment1.reload }.to raise_error ActiveRecord::RecordNotFound end it "deletes users's answer attachment" do expect { delete :destroy, params: { id: answer_attachment2 }, format: :js }.to change(answer2.files.attachments, :count).by(-1) + expect { answer_attachment2.reload }.to raise_error ActiveRecord::RecordNotFound end it "tries to delete another user's question attachment" do diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb index 7104a27..2ce8639 100644 --- a/spec/models/answer_spec.rb +++ b/spec/models/answer_spec.rb @@ -3,9 +3,12 @@ RSpec.describe Answer, type: :model do it { should belong_to :question } it { should belong_to :user } + it { should validate_presence_of :body } it { should validate_length_of(:body).is_at_least(50) } + it { should have_many(:files_attachments) } + let(:user) { create(:user) } let(:question) { create(:question, user: user) } let(:answer1) { create(:answer, question: question, user: user) } @@ -27,8 +30,4 @@ expect(answer1.question.answers).to eq [answer1, answer2] end end - - it 'has many attached files' do - expect(Answer.new.files).to be_an_instance_of(ActiveStorage::Attached::Many) - end end diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb index 171724b..bb00cd2 100644 --- a/spec/models/question_spec.rb +++ b/spec/models/question_spec.rb @@ -13,7 +13,5 @@ it { should validate_uniqueness_of(:title).case_insensitive } - it 'has many attached files' do - expect(Question.new.files).to be_an_instance_of(ActiveStorage::Attached::Many) - end + it { should have_many(:files_attachments) } end