From 63cbe2f7c115e5602cd8f73bbc2f9029b8e38759 Mon Sep 17 00:00:00 2001 From: Manuel Lucena Date: Tue, 3 Oct 2017 18:25:22 +0200 Subject: [PATCH 01/16] 20171003 - [WIP] Functionality and tests for polls comments --- app/controllers/polls_controller.rb | 5 + app/helpers/flags_helper.rb | 2 +- app/models/comment.rb | 2 +- app/models/comment_notifier.rb | 1 + app/models/poll.rb | 5 +- app/views/polls/questions/show.html.erb | 92 +++++++++++++++++++ app/views/polls/show.html.erb | 6 +- app/views/shared/_comments.html.erb | 31 +++++++ ...71003143034_add_comments_count_to_polls.rb | 7 ++ db/schema.rb | 3 + spec/features/polls/polls_spec.rb | 33 +++++++ 11 files changed, 183 insertions(+), 4 deletions(-) create mode 100644 app/views/polls/questions/show.html.erb create mode 100644 app/views/shared/_comments.html.erb create mode 100644 db/migrate/20171003143034_add_comments_count_to_polls.rb diff --git a/app/controllers/polls_controller.rb b/app/controllers/polls_controller.rb index 41a038b46..289a33be3 100644 --- a/app/controllers/polls_controller.rb +++ b/app/controllers/polls_controller.rb @@ -1,8 +1,10 @@ class PollsController < ApplicationController + include CommentableActions load_and_authorize_resource has_filters %w{current expired incoming} + has_orders %w{most_voted newest oldest}, only: :show ::Poll::Answer # trigger autoload @@ -13,6 +15,9 @@ class PollsController < ApplicationController def show @questions = @poll.questions.for_render.sort_for_list + @commentable = @poll + @comment_tree = CommentTree.new(@commentable, params[:page], @current_order) + @answers_by_question_id = {} poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user.try(:id)) poll_answers.each do |answer| diff --git a/app/helpers/flags_helper.rb b/app/helpers/flags_helper.rb index b5ba67f41..9983b34ae 100644 --- a/app/helpers/flags_helper.rb +++ b/app/helpers/flags_helper.rb @@ -12,7 +12,7 @@ module FlagsHelper def flagged?(flaggable) if flaggable.is_a? Comment - @comment_flags[flaggable.id] + @comment_flags[flaggable.id] unless flaggable.commentable_type == "Poll" else Flag.flagged?(current_user, flaggable) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 8b68e11ad..37fa9b630 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -3,7 +3,7 @@ class Comment < ActiveRecord::Base include HasPublicAuthor include Graphqlable - COMMENTABLE_TYPES = %w(Debate Proposal Budget::Investment Poll::Question Legislation::Question Legislation::Annotation Topic).freeze + COMMENTABLE_TYPES = %w(Debate Proposal Budget::Investment Poll::Question Legislation::Question Legislation::Annotation Topic Poll).freeze acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases diff --git a/app/models/comment_notifier.rb b/app/models/comment_notifier.rb index 68b350e6b..600009a23 100644 --- a/app/models/comment_notifier.rb +++ b/app/models/comment_notifier.rb @@ -22,6 +22,7 @@ class CommentNotifier end def email_on_comment? + return false if @comment.commentable.is_a?(Poll) commentable_author = @comment.commentable.author commentable_author != @author && commentable_author.email_on_comment? end diff --git a/app/models/poll.rb b/app/models/poll.rb index 84349bf0b..60b6d1406 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -1,6 +1,7 @@ class Poll < ActiveRecord::Base include Imageable - + acts_as_paranoid column: :hidden_at + include ActsAsParanoidAliases has_many :booth_assignments, class_name: "Poll::BoothAssignment" has_many :booths, through: :booth_assignments has_many :partial_results, through: :booth_assignments @@ -9,8 +10,10 @@ class Poll < ActiveRecord::Base has_many :officer_assignments, through: :booth_assignments has_many :officers, through: :officer_assignments has_many :questions + has_many :comments, as: :commentable has_and_belongs_to_many :geozones + belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id' validates :name, presence: true diff --git a/app/views/polls/questions/show.html.erb b/app/views/polls/questions/show.html.erb new file mode 100644 index 000000000..f8d12c59d --- /dev/null +++ b/app/views/polls/questions/show.html.erb @@ -0,0 +1,92 @@ +<% provide :title do %><%= @question.title %><% end %> + +
+
+
+ <%= back_link_to %> + +

<%= @question.title %>

+ + <% if @question.proposal.present? %> +
+ <%= link_to t('poll_questions.show.original_proposal'), @question.proposal %> +
+ <% end %> + + <% if can? :answer, @question %> + <%= link_to t('poll_questions.show.answer_this_question'), + @question.poll, + class: 'large button' %> + <% else %> + <%= render 'polls/reasons_for_not_answering', poll: @question.poll %> + <% end %> +
+ +
+

+ + <%= t('poll_questions.show.author') %> + +
+ <% if @question.author_visible_name.present? %> + <%= @question.author_visible_name %> + <% else %> + <%= link_to @question.author.name, @question.author %> + <% end %> + +

+ +

+ + <%= t('poll_questions.show.poll') %> + +
+ <%= link_to @question.poll.name, @question.poll %> +

+ +

+ + <%= t('poll_questions.show.dates_title') %> + +
+ <%= poll_dates(@question.poll) %> +

+
+
+
+ +<% if @question.video_url.present? %> +
+
+ +
+
+ +<% end %> + +
+
+

<%= t('poll_questions.show.more_info') %>

+ <%= @question.description %> +
+
+ +
+ <%= render "polls/questions/filter_subnav" %> + +
+ <%= render "shared/comments" %> +
+ +
+ <%= render 'documents/documents', + documents: @question.documents, + max_documents_allowed: Poll::Question.max_documents_allowed %> +
+
diff --git a/app/views/polls/show.html.erb b/app/views/polls/show.html.erb index b632fac55..ae334568c 100644 --- a/app/views/polls/show.html.erb +++ b/app/views/polls/show.html.erb @@ -79,6 +79,10 @@ <% end %> - + + +
+ <%= render "shared/comments" %> +
diff --git a/app/views/shared/_comments.html.erb b/app/views/shared/_comments.html.erb new file mode 100644 index 000000000..db097a7c4 --- /dev/null +++ b/app/views/shared/_comments.html.erb @@ -0,0 +1,31 @@ +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@commentable), @comment_tree.comments, @comment_tree.comment_authors, @commentable.comments_count, @comment_flags] do %> +
+
+
+

+ <%= t("shared.comments.title") %> + (<%= @commentable.comments_count %>) +

+ + <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %> + + <% if user_signed_in? %> + <%= render 'comments/form', {commentable: @commentable, parent_id: nil, toggeable: false} %> + <% else %> +
+ +
+ <%= t("shared.comments.login_to_comment", + signin: link_to(t("votes.signin"), new_user_session_path), + signup: link_to(t("votes.signup"), new_user_registration_path)).html_safe %> +
+ <% end %> + + <% @comment_tree.root_comments.each do |comment| %> + <%= render 'comments/comment', comment: comment %> + <% end %> + <%= paginate @comment_tree.root_comments %> +
+
+
+<% end %> diff --git a/db/migrate/20171003143034_add_comments_count_to_polls.rb b/db/migrate/20171003143034_add_comments_count_to_polls.rb new file mode 100644 index 000000000..675acae3f --- /dev/null +++ b/db/migrate/20171003143034_add_comments_count_to_polls.rb @@ -0,0 +1,7 @@ +class AddCommentsCountToPolls < ActiveRecord::Migration + def change + add_column :polls, :comments_count, :integer, default: 0 + add_column :polls, :author_id, :integer + add_column :polls, :hidden_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index dbbd3ec46..d393716e6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -792,6 +792,9 @@ ActiveRecord::Schema.define(version: 20171004151553) do t.boolean "geozone_restricted", default: false t.text "summary" t.text "description" + t.integer "comments_count", default: 0 + t.integer "author_id" + t.datetime "hidden_at" end add_index "polls", ["starts_at", "ends_at"], name: "index_polls_on_starts_at_and_ends_at", using: :btree diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index 22b90740a..1c18e752d 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -256,5 +256,38 @@ feature 'Polls' do expect(page).to have_link('Han Solo') end + scenario 'User can write comments' do + create(:comment, commentable: poll) + + visit poll_path(poll) + + expect(page).to have_css('.comment', count: 1) + + comment = Comment.last + within first('.comment') do + expect(page).to have_content comment.user.name + expect(page).to have_content I18n.l(comment.created_at, format: :datetime) + expect(page).to have_content comment.body + end + end + + scenario 'user can reply to comment' do + oliver = create(:user, username: 'Oliver Atom') + benji = create(:user, username: 'Benji Prince') + create(:comment, commentable: poll, user: oliver) + + login_as(oliver) + visit poll_path(poll) + + expect(page).to have_content oliver.username + end + + scenario 'user can upvote a comment' do + + end + + scenario 'user can downvote a comment' do + + end end end From c3d7d47c3f24de2ffa90edbb66f3b42d6704946d Mon Sep 17 00:00:00 2001 From: Manuel Lucena Date: Wed, 4 Oct 2017 15:23:24 +0200 Subject: [PATCH 02/16] 20171004 - Refactored specs for polls comments On branch mlucena-poll-comments Changes to be committed: modified: app/views/comments/show.html.erb modified: spec/features/polls/polls_spec.rb --- app/views/comments/show.html.erb | 2 +- spec/features/polls/polls_spec.rb | 52 ++++++++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/app/views/comments/show.html.erb b/app/views/comments/show.html.erb index a779d1dfa..ce76633a0 100644 --- a/app/views/comments/show.html.erb +++ b/app/views/comments/show.html.erb @@ -1,7 +1,7 @@
<%= back_link_to commentable_path(@comment), - t("comments.show.return_to_commentable") + @comment.commentable.title %> + t("comments.show.return_to_commentable") + @comment.commentable.title unless @comment != "Poll" %>
diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index 1c18e752d..76932558b 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -271,23 +271,67 @@ feature 'Polls' do end end - scenario 'user can reply to comment' do + scenario 'user b can access to comments from user a' do oliver = create(:user, username: 'Oliver Atom') benji = create(:user, username: 'Benji Prince') create(:comment, commentable: poll, user: oliver) - login_as(oliver) + login_as(benji) visit poll_path(poll) expect(page).to have_content oliver.username end - scenario 'user can upvote a comment' do + scenario 'user b can reply to comments from user a', :js do + koji = create(:user, username: 'Koji Kabuto') + sayaka = create(:user, username: 'Sayaka') + comment = create(:comment, commentable: poll, user: koji) + login_as(sayaka) + visit poll_path(poll) + + click_link "Reply" + + within "#js-comment-form-comment_#{comment.id}" do + fill_in "comment-body-comment_#{comment.id}", with: 'MAZINGER!!.' + click_button 'Publish reply' + end + + within "#comment_#{comment.id}" do + expect(page).to have_content 'MAZINGER!!.' + end + + expect(page).to_not have_selector("#js-comment-form-comment_#{comment.id}", visible: true) end - scenario 'user can downvote a comment' do + scenario 'user can upvote a comment', :js do + goku = create(:user, username: 'Goku') + vegeta = create(:user, username: 'Vegeta') + comment = create(:comment, commentable: poll, user: goku) + login_as(vegeta) + visit poll_path(poll) + + within("#comment_#{comment.id}_votes") do + find('.in_favor a').click + + expect(page).to have_content "1 vote" + end + end + + scenario 'user can downvote a comment', :js do + doraemon = create(:user, username: 'Doraemon') + nobita = create(:user, username: 'Nobi Nobita') + comment = create(:comment, commentable: poll, user: doraemon) + + login_as(nobita) + visit poll_path(poll) + + within("#comment_#{comment.id}_votes") do + find('.against a').click + + expect(page).to have_content "1 vote" + end end end end From 8277e3cc2b7c332d2bd009adf8220759ded683bb Mon Sep 17 00:00:00 2001 From: Manuel Lucena Date: Thu, 5 Oct 2017 18:24:16 +0200 Subject: [PATCH 03/16] removed obsolete polls questions view --- app/views/polls/questions/show.html.erb | 92 ------------------------- app/views/polls/show.html.erb | 2 - 2 files changed, 94 deletions(-) delete mode 100644 app/views/polls/questions/show.html.erb diff --git a/app/views/polls/questions/show.html.erb b/app/views/polls/questions/show.html.erb deleted file mode 100644 index f8d12c59d..000000000 --- a/app/views/polls/questions/show.html.erb +++ /dev/null @@ -1,92 +0,0 @@ -<% provide :title do %><%= @question.title %><% end %> - -
-
-
- <%= back_link_to %> - -

<%= @question.title %>

- - <% if @question.proposal.present? %> -
- <%= link_to t('poll_questions.show.original_proposal'), @question.proposal %> -
- <% end %> - - <% if can? :answer, @question %> - <%= link_to t('poll_questions.show.answer_this_question'), - @question.poll, - class: 'large button' %> - <% else %> - <%= render 'polls/reasons_for_not_answering', poll: @question.poll %> - <% end %> -
- -
-

- - <%= t('poll_questions.show.author') %> - -
- <% if @question.author_visible_name.present? %> - <%= @question.author_visible_name %> - <% else %> - <%= link_to @question.author.name, @question.author %> - <% end %> - -

- -

- - <%= t('poll_questions.show.poll') %> - -
- <%= link_to @question.poll.name, @question.poll %> -

- -

- - <%= t('poll_questions.show.dates_title') %> - -
- <%= poll_dates(@question.poll) %> -

-
-
-
- -<% if @question.video_url.present? %> -
-
- -
-
- -<% end %> - -
-
-

<%= t('poll_questions.show.more_info') %>

- <%= @question.description %> -
-
- -
- <%= render "polls/questions/filter_subnav" %> - -
- <%= render "shared/comments" %> -
- -
- <%= render 'documents/documents', - documents: @question.documents, - max_documents_allowed: Poll::Question.max_documents_allowed %> -
-
diff --git a/app/views/polls/show.html.erb b/app/views/polls/show.html.erb index ae334568c..a67aca798 100644 --- a/app/views/polls/show.html.erb +++ b/app/views/polls/show.html.erb @@ -79,8 +79,6 @@ <% end %> - -
<%= render "shared/comments" %>
From faa2f31b3a4c698fa134d2d5df0db82ccbcbb5d8 Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 12:04:16 +0200 Subject: [PATCH 04/16] Adds comment's tab --- app/views/polls/_filter_subnav.html.erb | 14 ++++++++++++++ app/views/polls/show.html.erb | 12 ++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 app/views/polls/_filter_subnav.html.erb diff --git a/app/views/polls/_filter_subnav.html.erb b/app/views/polls/_filter_subnav.html.erb new file mode 100644 index 000000000..b096edaf2 --- /dev/null +++ b/app/views/polls/_filter_subnav.html.erb @@ -0,0 +1,14 @@ +
+
+
    +
  • + <%= link_to "#tab-comments" do %> +

    + <%= t("polls.show.comments_tab") %> + (<%= @poll.comments_count %>) +

    + <% end %> +
  • +
+
+
diff --git a/app/views/polls/show.html.erb b/app/views/polls/show.html.erb index 5d60ed284..d9d871249 100644 --- a/app/views/polls/show.html.erb +++ b/app/views/polls/show.html.erb @@ -128,10 +128,14 @@ <% end %> - -
- <%= render "shared/comments" %> -
+ +
+ <%= render "filter_subnav" %> + +
+ <%= render "comments" %> +
+
\ No newline at end of file From 23397eecb387fff16b1cb0c1b36517e63680d928 Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 12:04:55 +0200 Subject: [PATCH 05/16] increases consistency by moving comment's view to poll's folder --- app/views/polls/_comments.html.erb | 24 ++++++++++++++++++++++ app/views/shared/_comments.html.erb | 31 ----------------------------- 2 files changed, 24 insertions(+), 31 deletions(-) create mode 100644 app/views/polls/_comments.html.erb delete mode 100644 app/views/shared/_comments.html.erb diff --git a/app/views/polls/_comments.html.erb b/app/views/polls/_comments.html.erb new file mode 100644 index 000000000..09b59cee6 --- /dev/null +++ b/app/views/polls/_comments.html.erb @@ -0,0 +1,24 @@ +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@poll), @comment_tree.comments, @comment_tree.comment_authors, @poll.comments_count, @comment_flags] do %> +
+
+ <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %> + + <% if user_signed_in? %> + <%= render 'comments/form', {commentable: @poll, parent_id: nil, toggeable: false} %> + <% else %> +
+ +
+ <%= t("polls.show.login_to_comment", + signin: link_to(t("votes.signin"), new_user_session_path), + signup: link_to(t("votes.signup"), new_user_registration_path)).html_safe %> +
+ <% end %> + + <% @comment_tree.root_comments.each do |comment| %> + <%= render 'comments/comment', comment: comment %> + <% end %> + <%= paginate @comment_tree.root_comments %> +
+
+<% end %> diff --git a/app/views/shared/_comments.html.erb b/app/views/shared/_comments.html.erb deleted file mode 100644 index db097a7c4..000000000 --- a/app/views/shared/_comments.html.erb +++ /dev/null @@ -1,31 +0,0 @@ -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@commentable), @comment_tree.comments, @comment_tree.comment_authors, @commentable.comments_count, @comment_flags] do %> -
-
-
-

- <%= t("shared.comments.title") %> - (<%= @commentable.comments_count %>) -

- - <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %> - - <% if user_signed_in? %> - <%= render 'comments/form', {commentable: @commentable, parent_id: nil, toggeable: false} %> - <% else %> -
- -
- <%= t("shared.comments.login_to_comment", - signin: link_to(t("votes.signin"), new_user_session_path), - signup: link_to(t("votes.signup"), new_user_registration_path)).html_safe %> -
- <% end %> - - <% @comment_tree.root_comments.each do |comment| %> - <%= render 'comments/comment', comment: comment %> - <% end %> - <%= paginate @comment_tree.root_comments %> -
-
-
-<% end %> From d405d70f4c9e5627c37cb6c11650203081499b6b Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 12:05:02 +0200 Subject: [PATCH 06/16] add translations --- config/locales/en/general.yml | 2 ++ config/locales/es/general.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index 413a6abf8..77579a7ea 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -482,6 +482,8 @@ en: already_voted_in_web: "You have already participated in this poll. If you vote again it will be overwritten." back: Back to voting cant_answer_not_logged_in: "You must %{signin} or %{signup} to participate." + comments_tab: Comments + login_to_comment: You must %{signin} or %{signup} to leave a comment. signin: Sign in signup: Sign up cant_answer_verify_html: "You must %{verify_link} in order to answer." diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index b8747406b..11e5e252a 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -482,6 +482,8 @@ es: already_voted_in_web: "Ya has participado en esta votación. Si vuelves a votar se sobreescribirá tu resultado anterior." back: Volver a votaciones cant_answer_not_logged_in: "Necesitas %{signin} o %{signup} para participar." + comments_tab: Comentarios + login_to_comment: Necesitas %{signin} o %{signup} para comentar. signin: iniciar sesión signup: registrarte cant_answer_verify_html: "Por favor %{verify_link} para poder responder." From 37cae41ee91edcb386dc882f88ea893694fc56ae Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 12:05:18 +0200 Subject: [PATCH 07/16] adds specs --- spec/features/comments/polls_spec.rb | 522 +++++++++++++++++++++++++++ 1 file changed, 522 insertions(+) create mode 100644 spec/features/comments/polls_spec.rb diff --git a/spec/features/comments/polls_spec.rb b/spec/features/comments/polls_spec.rb new file mode 100644 index 000000000..8ef528cd3 --- /dev/null +++ b/spec/features/comments/polls_spec.rb @@ -0,0 +1,522 @@ +require 'rails_helper' +include ActionView::Helpers::DateHelper + +feature 'Commenting polls' do + let(:user) { create :user } + let(:poll) { create :poll } + + scenario 'Index' do + 3.times { create(:comment, commentable: poll) } + + visit poll_path(poll) + + expect(page).to have_css('.comment', count: 3) + + comment = Comment.last + within first('.comment') do + expect(page).to have_content comment.user.name + expect(page).to have_content I18n.l(comment.created_at, format: :datetime) + expect(page).to have_content comment.body + end + end + + scenario 'Show' do + skip "Feature not implemented yet, review soon" + + parent_comment = create(:comment, commentable: poll) + first_child = create(:comment, commentable: poll, parent: parent_comment) + second_child = create(:comment, commentable: poll, parent: parent_comment) + + visit comment_path(parent_comment) + + expect(page).to have_css(".comment", count: 3) + expect(page).to have_content parent_comment.body + expect(page).to have_content first_child.body + expect(page).to have_content second_child.body + expect(page).to have_link "Go back to #{poll.name}", href: poll_path(poll) + + expect(page).to have_selector("ul#comment_#{parent_comment.id}>li", count: 2) + expect(page).to have_selector("ul#comment_#{first_child.id}>li", count: 1) + expect(page).to have_selector("ul#comment_#{second_child.id}>li", count: 1) + end + + scenario 'Collapsable comments', :js do + parent_comment = create(:comment, body: "Main comment", commentable: poll) + child_comment = create(:comment, body: "First subcomment", commentable: poll, parent: parent_comment) + grandchild_comment = create(:comment, body: "Last subcomment", commentable: poll, parent: child_comment) + + visit poll_path(poll) + + expect(page).to have_css('.comment', count: 3) + + find("#comment_#{child_comment.id}_children_arrow").trigger('click') + + expect(page).to have_css('.comment', count: 2) + expect(page).to_not have_content grandchild_comment.body + + find("#comment_#{child_comment.id}_children_arrow").trigger('click') + + expect(page).to have_css('.comment', count: 3) + expect(page).to have_content grandchild_comment.body + + find("#comment_#{parent_comment.id}_children_arrow").trigger('click') + + expect(page).to have_css('.comment', count: 1) + expect(page).to_not have_content child_comment.body + expect(page).to_not have_content grandchild_comment.body + end + + scenario 'Comment order' do + c1 = create(:comment, :with_confidence_score, commentable: poll, cached_votes_up: 100, + cached_votes_total: 120, created_at: Time.current - 2) + c2 = create(:comment, :with_confidence_score, commentable: poll, cached_votes_up: 10, + cached_votes_total: 12, created_at: Time.current - 1) + c3 = create(:comment, :with_confidence_score, commentable: poll, cached_votes_up: 1, + cached_votes_total: 2, created_at: Time.current) + + visit poll_path(poll, order: :most_voted) + + expect(c1.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c3.body) + + visit poll_path(poll, order: :newest) + + expect(c3.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c1.body) + + visit poll_path(poll, order: :oldest) + + expect(c1.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c3.body) + end + + scenario 'Creation date works differently in roots and in child comments, when sorting by confidence_score' do + old_root = create(:comment, commentable: poll, created_at: Time.current - 10) + new_root = create(:comment, commentable: poll, created_at: Time.current) + old_child = create(:comment, commentable: poll, parent_id: new_root.id, created_at: Time.current - 10) + new_child = create(:comment, commentable: poll, parent_id: new_root.id, created_at: Time.current) + + visit poll_path(poll, order: :most_voted) + + expect(new_root.body).to appear_before(old_root.body) + expect(old_child.body).to appear_before(new_child.body) + + visit poll_path(poll, order: :newest) + + expect(new_root.body).to appear_before(old_root.body) + expect(new_child.body).to appear_before(old_child.body) + + visit poll_path(poll, order: :oldest) + + expect(old_root.body).to appear_before(new_root.body) + expect(old_child.body).to appear_before(new_child.body) + end + + scenario 'Turns links into html links' do + create :comment, commentable: poll, body: 'Built with http://rubyonrails.org/' + + visit poll_path(poll) + + within first('.comment') do + expect(page).to have_content 'Built with http://rubyonrails.org/' + expect(page).to have_link('http://rubyonrails.org/', href: 'http://rubyonrails.org/') + expect(find_link('http://rubyonrails.org/')[:rel]).to eq('nofollow') + expect(find_link('http://rubyonrails.org/')[:target]).to eq('_blank') + end + end + + scenario 'Sanitizes comment body for security' do + create :comment, commentable: poll, + body: " click me http://www.url.com" + + visit poll_path(poll) + + within first('.comment') do + expect(page).to have_content "click me http://www.url.com" + expect(page).to have_link('http://www.url.com', href: 'http://www.url.com') + expect(page).not_to have_link('click me') + end + end + + scenario 'Paginated comments' do + per_page = 10 + (per_page + 2).times { create(:comment, commentable: poll)} + + visit poll_path(poll) + + expect(page).to have_css('.comment', count: per_page) + within("ul.pagination") do + expect(page).to have_content("1") + expect(page).to have_content("2") + expect(page).to_not have_content("3") + click_link "Next", exact: false + end + + expect(page).to have_css('.comment', count: 2) + end + + feature 'Not logged user' do + scenario 'can not see comments forms' do + create(:comment, commentable: poll) + visit poll_path(poll) + + expect(page).to have_content 'You must Sign in or Sign up to leave a comment' + within('#comments') do + expect(page).to_not have_content 'Write a comment' + expect(page).to_not have_content 'Reply' + end + end + end + + scenario 'Create', :js do + login_as(user) + visit poll_path(poll) + + fill_in "comment-body-poll_#{poll.id}", with: 'Have you thought about...?' + click_button 'Publish comment' + + within "#comments" do + expect(page).to have_content 'Have you thought about...?' + end + + within "#tab-comments-label" do + expect(page).to have_content 'Comments (1)' + end + end + + scenario 'Errors on create', :js do + login_as(user) + visit poll_path(poll) + + click_button 'Publish comment' + + expect(page).to have_content "Can't be blank" + end + + scenario 'Reply', :js do + citizen = create(:user, username: 'Ana') + manuela = create(:user, username: 'Manuela') + comment = create(:comment, commentable: poll, user: citizen) + + login_as(manuela) + visit poll_path(poll) + + click_link "Reply" + + within "#js-comment-form-comment_#{comment.id}" do + fill_in "comment-body-comment_#{comment.id}", with: 'It will be done next week.' + click_button 'Publish reply' + end + + within "#comment_#{comment.id}" do + expect(page).to have_content 'It will be done next week.' + end + + expect(page).to_not have_selector("#js-comment-form-comment_#{comment.id}", visible: true) + end + + scenario 'Errors on reply', :js do + comment = create(:comment, commentable: poll, user: user) + + login_as(user) + visit poll_path(poll) + + click_link "Reply" + + within "#js-comment-form-comment_#{comment.id}" do + click_button 'Publish reply' + expect(page).to have_content "Can't be blank" + end + + end + + scenario "N replies", :js do + parent = create(:comment, commentable: poll) + + 7.times do + create(:comment, commentable: poll, parent: parent) + parent = parent.children.first + end + + visit poll_path(poll) + expect(page).to have_css(".comment.comment.comment.comment.comment.comment.comment.comment") + end + + scenario "Flagging as inappropriate", :js do + skip "Feature not implemented yet, review soon" + + comment = create(:comment, commentable: poll) + + login_as(user) + visit poll_path(poll) + + within "#comment_#{comment.id}" do + page.find("#flag-expand-comment-#{comment.id}").click + page.find("#flag-comment-#{comment.id}").click + + expect(page).to have_css("#unflag-expand-comment-#{comment.id}") + end + + expect(Flag.flagged?(user, comment)).to be + end + + scenario "Undoing flagging as inappropriate", :js do + skip "Feature not implemented yet, review soon" + + comment = create(:comment, commentable: poll) + Flag.flag(user, comment) + + login_as(user) + visit poll_path(poll) + + within "#comment_#{comment.id}" do + page.find("#unflag-expand-comment-#{comment.id}").click + page.find("#unflag-comment-#{comment.id}").click + + expect(page).to have_css("#flag-expand-comment-#{comment.id}") + end + + expect(Flag.flagged?(user, comment)).to_not be + end + + scenario "Flagging turbolinks sanity check", :js do + skip "Feature not implemented yet, review soon" + + poll = create(:poll, title: "Should we change the world?") + comment = create(:comment, commentable: poll) + + login_as(user) + visit polls_path + click_link "Should we change the world?" + + within "#comment_#{comment.id}" do + page.find("#flag-expand-comment-#{comment.id}").click + expect(page).to have_selector("#flag-comment-#{comment.id}") + end + end + + scenario "Erasing a comment's author" do + poll = create(:poll) + comment = create(:comment, commentable: poll, body: "this should be visible") + comment.user.erase + + visit poll_path(poll) + within "#comment_#{comment.id}" do + expect(page).to have_content('User deleted') + expect(page).to have_content('this should be visible') + end + end + + feature "Moderators" do + + scenario "can create comment as a moderator", :js do + skip "Feature not implemented yet, review soon" + + moderator = create(:moderator) + + login_as(moderator.user) + visit poll_path(poll) + + fill_in "comment-body-poll_#{poll.id}", with: "I am moderating!" + check "comment-as-moderator-poll_#{poll.id}" + click_button "Publish comment" + + within "#comments" do + expect(page).to have_content "I am moderating!" + expect(page).to have_content "Moderator ##{moderator.id}" + expect(page).to have_css "div.is-moderator" + expect(page).to have_css "img.moderator-avatar" + end + end + + scenario "can create reply as a moderator", :js do + skip "Feature not implemented yet, review soon" + + citizen = create(:user, username: "Ana") + manuela = create(:user, username: "Manuela") + moderator = create(:moderator, user: manuela) + comment = create(:comment, commentable: poll, user: citizen) + + login_as(manuela) + visit poll_path(poll) + + click_link "Reply" + + within "#js-comment-form-comment_#{comment.id}" do + fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + check "comment-as-moderator-comment_#{comment.id}" + click_button 'Publish reply' + end + + within "#comment_#{comment.id}" do + expect(page).to have_content "I am moderating!" + expect(page).to have_content "Moderator ##{moderator.id}" + expect(page).to have_css "div.is-moderator" + expect(page).to have_css "img.moderator-avatar" + end + + expect(page).to_not have_selector("#js-comment-form-comment_#{comment.id}", visible: true) + end + + scenario "can not comment as an administrator" do + skip "Feature not implemented yet, review soon" + + moderator = create(:moderator) + + login_as(moderator.user) + visit poll_path(poll) + + expect(page).to_not have_content "Comment as administrator" + end + end + + feature "Administrators" do + scenario "can create comment as an administrator", :js do + skip "Feature not implemented yet, review soon" + + admin = create(:administrator) + + login_as(admin.user) + visit poll_path(poll) + + fill_in "comment-body-poll_#{poll.id}", with: "I am your Admin!" + check "comment-as-administrator-poll_#{poll.id}" + click_button "Publish comment" + + within "#comments" do + expect(page).to have_content "I am your Admin!" + expect(page).to have_content "Administrator ##{admin.id}" + expect(page).to have_css "div.is-admin" + expect(page).to have_css "img.admin-avatar" + end + end + + scenario "can create reply as an administrator", :js do + skip "Feature not implemented yet, review soon" + + citizen = create(:user, username: "Ana") + manuela = create(:user, username: "Manuela") + admin = create(:administrator, user: manuela) + comment = create(:comment, commentable: poll, user: citizen) + + login_as(manuela) + visit poll_path(poll) + + click_link "Reply" + + within "#js-comment-form-comment_#{comment.id}" do + fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + check "comment-as-administrator-comment_#{comment.id}" + click_button 'Publish reply' + end + + within "#comment_#{comment.id}" do + expect(page).to have_content "Top of the world!" + expect(page).to have_content "Administrator ##{admin.id}" + expect(page).to have_css "div.is-admin" + expect(page).to have_css "img.admin-avatar" + end + + expect(page).to_not have_selector("#js-comment-form-comment_#{comment.id}", visible: true) + end + + scenario "can not comment as a moderator" do + skip "Feature not implemented yet, review soon" + + admin = create(:administrator) + + login_as(admin.user) + visit poll_path(poll) + + expect(page).to_not have_content "Comment as moderator" + end + end + + feature 'Voting comments' do + + background do + @manuela = create(:user, verified_at: Time.current) + @pablo = create(:user) + @poll = create(:poll) + @comment = create(:comment, commentable: @poll) + + login_as(@manuela) + end + + scenario 'Show' do + create(:vote, voter: @manuela, votable: @comment, vote_flag: true) + create(:vote, voter: @pablo, votable: @comment, vote_flag: false) + + visit poll_path(@poll) + + within("#comment_#{@comment.id}_votes") do + within(".in_favor") do + expect(page).to have_content "1" + end + + within(".against") do + expect(page).to have_content "1" + end + + expect(page).to have_content "2 votes" + end + end + + scenario 'Create', :js do + visit poll_path(@poll) + + within("#comment_#{@comment.id}_votes") do + find(".in_favor a").click + + within(".in_favor") do + expect(page).to have_content "1" + end + + within(".against") do + expect(page).to have_content "0" + end + + expect(page).to have_content "1 vote" + end + end + + scenario 'Update', :js do + visit poll_path(@poll) + + within("#comment_#{@comment.id}_votes") do + find('.in_favor a').click + find('.against a').click + + within('.in_favor') do + expect(page).to have_content "0" + end + + within('.against') do + expect(page).to have_content "1" + end + + expect(page).to have_content "1 vote" + end + end + + scenario 'Trying to vote multiple times', :js do + visit poll_path(@poll) + + within("#comment_#{@comment.id}_votes") do + find('.in_favor a').click + find('.in_favor a').click + + within('.in_favor') do + expect(page).to have_content "1" + end + + within('.against') do + expect(page).to have_content "0" + end + + expect(page).to have_content "1 vote" + end + end + end + +end From 843048a86176cbfe4b4f023a0b0debcd740add63 Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 12:14:04 +0200 Subject: [PATCH 08/16] cleans up --- app/controllers/polls_controller.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/controllers/polls_controller.rb b/app/controllers/polls_controller.rb index 6240d4809..7f8fc8e4d 100644 --- a/app/controllers/polls_controller.rb +++ b/app/controllers/polls_controller.rb @@ -1,6 +1,4 @@ class PollsController < ApplicationController - include CommentableActions - include PollsHelper load_and_authorize_resource @@ -17,13 +15,13 @@ class PollsController < ApplicationController def show @questions = @poll.questions.for_render.sort_for_list @token = poll_voter_token(@poll, current_user) - + @answers_by_question_id = {} poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user.try(:id)) poll_answers.each do |answer| @answers_by_question_id[answer.question_id] = answer.answer end - + @commentable = @poll @comment_tree = CommentTree.new(@commentable, params[:page], @current_order) end From ffdbdacc789c43bddf885c89d71e9cb4874b70ab Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 12:47:09 +0200 Subject: [PATCH 09/16] fixes specs --- app/views/comments/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/comments/show.html.erb b/app/views/comments/show.html.erb index ce76633a0..2b2e3e841 100644 --- a/app/views/comments/show.html.erb +++ b/app/views/comments/show.html.erb @@ -1,7 +1,7 @@
<%= back_link_to commentable_path(@comment), - t("comments.show.return_to_commentable") + @comment.commentable.title unless @comment != "Poll" %> + t("comments.show.return_to_commentable") + @comment.commentable.title unless @commentable.class == Poll %>
From b95dc572c51a67ea9a8f505d17c1f2569aa669da Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 10 Oct 2017 12:53:10 +0200 Subject: [PATCH 10/16] improves answer description height to avoid texts cuts --- app/assets/stylesheets/participation.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 7d6a0fff7..5fefd8c53 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -1596,7 +1596,7 @@ height: 100%; &.short { - height: $line-height * 12; + height: rem-calc(300); overflow: hidden; } } From 89f81bf7a4026b8439cf192e2fba3dcd7847e55c Mon Sep 17 00:00:00 2001 From: rgarcia Date: Tue, 10 Oct 2017 16:01:20 +0200 Subject: [PATCH 11/16] fixes exception when sending email about comment reply --- app/models/poll.rb | 4 +++ spec/features/emails_spec.rb | 49 ++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/app/models/poll.rb b/app/models/poll.rb index 60b6d1406..8675ce49a 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -27,6 +27,10 @@ class Poll < ActiveRecord::Base scope :sort_for_list, -> { order(:geozone_restricted, :starts_at, :name) } + def title + name + end + def current?(timestamp = Date.current.beginning_of_day) starts_at <= timestamp && timestamp <= ends_at end diff --git a/spec/features/emails_spec.rb b/spec/features/emails_spec.rb index 21fdf8a64..a26e8ce11 100644 --- a/spec/features/emails_spec.rb +++ b/spec/features/emails_spec.rb @@ -371,6 +371,55 @@ feature 'Emails' do end + context "Polls" do + + scenario "Do not send email on poll comment", :js do + user1 = create(:user, email_on_comment: true) + user2 = create(:user) + + poll = create(:poll, author: user1) + reset_mailer + + login_as(user2) + visit poll_path(poll) + + fill_in "comment-body-poll_#{poll.id}", with: 'Have you thought about...?' + click_button 'Publish comment' + + expect(page).to have_content 'Have you thought about...?' + + expect { open_last_email }.to raise_error "No email has been sent!" + end + + scenario "Send email on poll comment reply", :js do + user1 = create(:user, email_on_comment_reply: true) + user2 = create(:user) + + poll = create(:poll) + comment = create(:comment, commentable: poll, author: user1) + + login_as(user2) + visit poll_path(poll) + + click_link "Reply" + within "#js-comment-form-comment_#{comment.id}" do + fill_in "comment-body-comment_#{comment.id}", with: 'It will be done next week.' + click_button 'Publish reply' + end + expect(page).to have_content 'It will be done next week.' + + + email = open_last_email + expect(email).to have_subject('Someone has responded to your comment') + expect(email).to deliver_to(user1) + expect(email).to_not have_body_text(poll_path(poll)) + expect(email).to have_body_text(comment_path(Comment.last)) + expect(email).to have_body_text(I18n.t("mailers.config.manage_email_subscriptions")) + expect(email).to have_body_text(account_path) + end + + end + context "Users without email" do scenario "should not receive emails", :js do user = create(:user, :verified, email_on_comment: true) From 2b10b59e2aab6834fcfa67dd61d99946af40de51 Mon Sep 17 00:00:00 2001 From: iagirre Date: Mon, 9 Oct 2017 17:36:48 +0200 Subject: [PATCH 12/16] Order in the admin page using jquery-ui sortable widget. --- app/assets/javascripts/application.js | 3 ++ app/assets/javascripts/sortable.js.coffee | 9 ++++ app/assets/stylesheets/application.scss | 1 + .../poll/questions/answers_controller.rb | 6 +++ app/models/poll/question/answer.rb | 8 +++ app/views/admin/poll/questions/show.html.erb | 50 ++++++++++--------- config/routes.rb | 1 + ...dd_given_order_to_poll_question_answers.rb | 5 ++ db/schema.rb | 3 +- 9 files changed, 61 insertions(+), 25 deletions(-) create mode 100644 app/assets/javascripts/sortable.js.coffee create mode 100644 db/migrate/20171010143623_add_given_order_to_poll_question_answers.rb diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index ce9861547..51de9c678 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -15,6 +15,7 @@ //= require jquery-ui/widgets/datepicker //= require jquery-ui/i18n/datepicker-es //= require jquery-ui/widgets/autocomplete +//= require jquery-ui/widgets/sortable //= require jquery-fileupload/basic //= require foundation //= require turbolinks @@ -71,6 +72,7 @@ //= require leaflet //= require map //= require polls +//= require sortable var initialize_modules = function() { App.Comments.initialize(); @@ -110,6 +112,7 @@ var initialize_modules = function() { App.PollsAdmin.initialize(); App.Map.initialize(); App.Polls.initialize(); + App.Sortable.initialize(); }; $(function(){ diff --git a/app/assets/javascripts/sortable.js.coffee b/app/assets/javascripts/sortable.js.coffee new file mode 100644 index 000000000..1af543f6a --- /dev/null +++ b/app/assets/javascripts/sortable.js.coffee @@ -0,0 +1,9 @@ +App.Sortable = + initialize: -> + $(".sortable").sortable + update: (event, ui) -> + new_order = $(this).sortable('toArray', {attribute: 'data-answer-id'}); + $.ajax + url: $('.sortable').data('js-url'), + data: {ordered_list: new_order}, + type: 'POST' diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index aff55c88c..3f93ffa6b 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -18,4 +18,5 @@ @import 'datepicker_overrides'; @import 'jquery-ui/autocomplete'; @import 'autocomplete_overrides'; +@import 'jquery-ui/sortable'; @import 'leaflet'; diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 51c44dd94..a2490475b 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -37,6 +37,12 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController @documents = @answer.documents render 'admin/poll/questions/answers/documents' + end + + def order_answers + ::Poll::Question::Answer.order_answers(params[:ordered_list]) + #redirect_to admin_question_path(params[:question_id]) + render :nothing => true end private diff --git a/app/models/poll/question/answer.rb b/app/models/poll/question/answer.rb index b3324e57f..ba7fe366a 100644 --- a/app/models/poll/question/answer.rb +++ b/app/models/poll/question/answer.rb @@ -14,4 +14,12 @@ class Poll::Question::Answer < ActiveRecord::Base def description super.try :html_safe end + + def self.order_answers(ordered_array) + ordered_array.each_with_index do |answer_id, order| + answer = self.find(answer_id) + answer.update_attribute(:given_order, (order + 1)) + answer.save + end + end end diff --git a/app/views/admin/poll/questions/show.html.erb b/app/views/admin/poll/questions/show.html.erb index 5baabeafe..205b707fb 100644 --- a/app/views/admin/poll/questions/show.html.erb +++ b/app/views/admin/poll/questions/show.html.erb @@ -47,30 +47,32 @@ <%= t("admin.questions.show.answers.videos") %> - <% @question.question_answers.each do |answer| %> - - <%= link_to answer.title, admin_answer_path(answer) %> - <%= answer.description %> - - (<%= answer.images.count %>) -
- <%= link_to t("admin.questions.show.answers.images_list"), - admin_answer_images_path(answer) %> - - - (<%= answer.documents.count rescue 0 %>) -
- <%= link_to t("admin.questions.show.answers.documents_list"), - admin_answer_documents_path(answer) %> - - - (<%= answer.videos.count %>) -
- <%= link_to t("admin.questions.show.answers.video_list"), - admin_answer_videos_path(answer) %> - - - <% end %> + + <% @question.question_answers.each do |answer| %> + + <%= link_to answer.title, admin_answer_path(answer) %> + <%= answer.description %> + + (<%= answer.images.count %>) +
+ <%= link_to t("admin.questions.show.answers.images_list"), + admin_answer_images_path(answer) %> + + + (<%= answer.documents.count rescue 0 %>) +
+ <%= link_to t("admin.questions.show.answers.documents_list"), + admin_answer_documents_path(answer) %> + + + (<%= answer.videos.count %>) +
+ <%= link_to t("admin.questions.show.answers.video_list"), + admin_answer_videos_path(answer) %> + + + <% end %> + <% if @question.video_url.present? %> diff --git a/config/routes.rb b/config/routes.rb index 83b5754d7..1fb74aa53 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -306,6 +306,7 @@ Rails.application.routes.draw do resources :videos, controller: 'questions/answers/videos' get :documents, to: 'questions/answers#documents' end + post '/answers/order_answers', to: 'questions/answers#order_answers' end end diff --git a/db/migrate/20171010143623_add_given_order_to_poll_question_answers.rb b/db/migrate/20171010143623_add_given_order_to_poll_question_answers.rb new file mode 100644 index 000000000..6161e55e4 --- /dev/null +++ b/db/migrate/20171010143623_add_given_order_to_poll_question_answers.rb @@ -0,0 +1,5 @@ +class AddGivenOrderToPollQuestionAnswers < ActiveRecord::Migration + def change + add_column :poll_question_answers, :given_order, :integer, default: 1 + end +end diff --git a/db/schema.rb b/db/schema.rb index 9adba12b1..b9a1524ab 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171006145053) do +ActiveRecord::Schema.define(version: 20171010143623) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -680,6 +680,7 @@ ActiveRecord::Schema.define(version: 20171006145053) do t.string "title" t.text "description" t.integer "question_id" + t.integer "given_order", default: 1 end add_index "poll_question_answers", ["question_id"], name: "index_poll_question_answers_on_question_id", using: :btree From 943c1f23af529dff41e2826387120afe393904dd Mon Sep 17 00:00:00 2001 From: iagirre Date: Tue, 10 Oct 2017 13:28:49 +0200 Subject: [PATCH 13/16] Spects added to test the order of answers. Default order for question_answers set. --- .../poll/questions/answers_controller.rb | 9 +++---- app/models/poll/question.rb | 2 +- app/models/poll/question/answer.rb | 16 +++++++++---- .../poll/questions/answers/answers_spec.rb | 24 +++++++++++++++++-- spec/features/polls/answers_spec.rb | 6 +++-- spec/features/polls/polls_spec.rb | 24 +++++++++++++++++++ 6 files changed, 67 insertions(+), 14 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index a2490475b..7261adc9f 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -9,6 +9,7 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController def create @answer = ::Poll::Question::Answer.new(answer_params) + @answer.set_order if @answer.save redirect_to admin_question_path(@answer.question), @@ -37,12 +38,12 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController @documents = @answer.documents render 'admin/poll/questions/answers/documents' - end - + end + def order_answers ::Poll::Question::Answer.order_answers(params[:ordered_list]) - #redirect_to admin_question_path(params[:question_id]) - render :nothing => true + # redirect_to admin_question_path(params[:question_id]) + render nothing: true end private diff --git a/app/models/poll/question.rb b/app/models/poll/question.rb index e6ef482a9..efcaf4635 100644 --- a/app/models/poll/question.rb +++ b/app/models/poll/question.rb @@ -10,7 +10,7 @@ class Poll::Question < ActiveRecord::Base has_many :comments, as: :commentable has_many :answers, class_name: 'Poll::Answer' - has_many :question_answers, class_name: 'Poll::Question::Answer' + has_many :question_answers, -> { order 'given_order asc' }, class_name: 'Poll::Question::Answer' has_many :partial_results belongs_to :proposal diff --git a/app/models/poll/question/answer.rb b/app/models/poll/question/answer.rb index ba7fe366a..a7aa3339a 100644 --- a/app/models/poll/question/answer.rb +++ b/app/models/poll/question/answer.rb @@ -1,5 +1,5 @@ class Poll::Question::Answer < ActiveRecord::Base - include Galleryable + include Galleryable include Documentable documentable max_documents_allowed: 3, max_file_size: 3.megabytes, @@ -14,12 +14,18 @@ class Poll::Question::Answer < ActiveRecord::Base def description super.try :html_safe end - + def self.order_answers(ordered_array) ordered_array.each_with_index do |answer_id, order| - answer = self.find(answer_id) + answer = find(answer_id) answer.update_attribute(:given_order, (order + 1)) - answer.save - end + answer.save + end + end + + def set_order + last_position = Poll::Question::Answer.where(question_id: question_id).maximum("given_order") || 0 + next_position = last_position + 1 + update_attribute(:given_order, next_position) end end diff --git a/spec/features/admin/poll/questions/answers/answers_spec.rb b/spec/features/admin/poll/questions/answers/answers_spec.rb index 66918c4d8..48c2017f4 100644 --- a/spec/features/admin/poll/questions/answers/answers_spec.rb +++ b/spec/features/admin/poll/questions/answers/answers_spec.rb @@ -4,7 +4,7 @@ feature 'Answers' do background do admin = create(:administrator) - login_as (admin.user) + login_as admin.user end scenario 'Create' do @@ -24,9 +24,27 @@ feature 'Answers' do expect(page).to have_content(description) end + scenario 'Create second answer and place after the first one' do + question = create(:poll_question) + answer = create(:poll_question_answer, title: 'First', question: question, given_order: 1) + title = 'Second' + description = "Description" + + visit admin_question_path(question) + click_link 'Add answer' + + fill_in 'poll_question_answer_title', with: title + fill_in 'poll_question_answer_description', with: description + + click_button 'Save' + + expect(page.body.index('First')).to be < page.body.index('Second') + end + scenario 'Update' do question = create(:poll_question) - answer = create(:poll_question_answer, question: question, title: "Answer title") + answer = create(:poll_question_answer, question: question, title: "Answer title", given_order: 1) + answer2 = create(:poll_question_answer, question: question, title: "Another title", given_order: 2) visit admin_answer_path(answer) @@ -46,6 +64,8 @@ feature 'Answers' do expect(page).to have_content(new_title) expect(page).to_not have_content(old_title) + + expect(page.body.index(new_title)).to be < page.body.index(answer2.title) end end diff --git a/spec/features/polls/answers_spec.rb b/spec/features/polls/answers_spec.rb index f7dfd4d38..368597830 100644 --- a/spec/features/polls/answers_spec.rb +++ b/spec/features/polls/answers_spec.rb @@ -9,13 +9,15 @@ feature 'Answers' do scenario "Index" do question = create(:poll_question) - answer1 = create(:poll_question_answer, question: question) - answer2 = create(:poll_question_answer, question: question) + answer1 = create(:poll_question_answer, question: question, given_order: 1) + answer2 = create(:poll_question_answer, question: question, given_order: 2) visit admin_question_path(question) expect(page).to have_css(".poll_question_answer", count: 2) + expect(page.body.index(answer1.title)).to be < page.body.index(answer2.title) + within("#poll_question_answer_#{answer1.id}") do expect(page).to have_content answer1.title expect(page).to have_content answer1.description diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index 9fdb07255..fc21e1e5f 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -78,6 +78,30 @@ feature 'Polls' do expect(page).to have_content(proposal_question.title) end + scenario "Question answers appear in the given order" do + question = create(:poll_question, poll: poll) + answer1 = create(:poll_question_answer, title: 'First', question: question, given_order: 1) + answer2 = create(:poll_question_answer, title: 'Second', question: question, given_order: 2) + + visit poll_path(poll) + + within("div#poll_question_#{question.id}") do + expect(page.body.index(answer1.title)).to be < page.body.index(answer2.title) + end + end + + scenario "More info answers appear in the given order" do + question = create(:poll_question, poll: poll) + answer1 = create(:poll_question_answer, title: 'First', question: question, given_order: 1) + answer2 = create(:poll_question_answer, title: 'Second', question: question, given_order: 2) + + visit poll_path(poll) + + within('div.poll-more-info-answers') do + expect(page.body.index(answer1.title)).to be < page.body.index(answer2.title) + end + end + scenario 'Non-logged in users' do question = create(:poll_question, poll: poll) answer1 = create(:poll_question_answer, question: question, title: 'Han Solo') From 44474e76865dfd48f623784824e9ced0f9ebf7f2 Mon Sep 17 00:00:00 2001 From: iagirre Date: Tue, 10 Oct 2017 13:58:06 +0200 Subject: [PATCH 14/16] Deleted a forgotten comment --- app/controllers/admin/poll/questions/answers_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 7261adc9f..4e420575f 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -42,7 +42,6 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController def order_answers ::Poll::Question::Answer.order_answers(params[:ordered_list]) - # redirect_to admin_question_path(params[:question_id]) render nothing: true end From 644d09ebd2426073addf1631bfab5bee62f34931 Mon Sep 17 00:00:00 2001 From: iagirre Date: Wed, 11 Oct 2017 09:42:52 +0200 Subject: [PATCH 15/16] PR comments applied and poll_question_answer default name changed in factory. --- .../admin/poll/questions/answers_controller.rb | 1 - app/models/poll/question/answer.rb | 16 ++++++++++------ spec/factories.rb | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 4e420575f..bef176a22 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -9,7 +9,6 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController def create @answer = ::Poll::Question::Answer.new(answer_params) - @answer.set_order if @answer.save redirect_to admin_question_path(@answer.question), diff --git a/app/models/poll/question/answer.rb b/app/models/poll/question/answer.rb index a7aa3339a..7514b85a4 100644 --- a/app/models/poll/question/answer.rb +++ b/app/models/poll/question/answer.rb @@ -10,6 +10,9 @@ class Poll::Question::Answer < ActiveRecord::Base has_many :videos, class_name: 'Poll::Question::Answer::Video' validates :title, presence: true + validates :given_order, presence: true, uniqueness: { scope: :question_id } + + before_validation :set_order, on: :create def description super.try :html_safe @@ -17,15 +20,16 @@ class Poll::Question::Answer < ActiveRecord::Base def self.order_answers(ordered_array) ordered_array.each_with_index do |answer_id, order| - answer = find(answer_id) - answer.update_attribute(:given_order, (order + 1)) - answer.save + find(answer_id).update_attribute(:given_order, (order + 1)) end end def set_order - last_position = Poll::Question::Answer.where(question_id: question_id).maximum("given_order") || 0 - next_position = last_position + 1 - update_attribute(:given_order, next_position) + next_position = self.class.last_position(question_id) + 1 + self.given_order = next_position + end + + def self.last_position(question_id) + where(question_id: question_id).maximum("given_order") || 0 end end diff --git a/spec/factories.rb b/spec/factories.rb index a28471efc..69aa92c62 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -503,8 +503,8 @@ FactoryGirl.define do factory :poll_question_answer, class: 'Poll::Question::Answer' do association :question, factory: :poll_question - sequence(:title) { |n| "Question title #{n}" } - sequence(:description) { |n| "Question description #{n}" } + sequence(:title) { |n| "Answer title #{n}" } + sequence(:description) { |n| "Answer description #{n}" } end factory :poll_booth, class: 'Poll::Booth' do From 34c278db749de473386784a1731e7b803337f80f Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 11 Oct 2017 12:05:20 +0200 Subject: [PATCH 16/16] Small fixes for Poll Question Answer ordering --- app/controllers/polls_controller.rb | 1 + app/models/poll/question/answer.rb | 5 ++--- app/views/polls/show.html.erb | 6 +++--- .../features/admin/poll/questions/answers/answers_spec.rb | 4 ++-- spec/features/polls/answers_spec.rb | 6 +++--- spec/features/polls/polls_spec.rb | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/controllers/polls_controller.rb b/app/controllers/polls_controller.rb index 7f8fc8e4d..18b9add0f 100644 --- a/app/controllers/polls_controller.rb +++ b/app/controllers/polls_controller.rb @@ -15,6 +15,7 @@ class PollsController < ApplicationController def show @questions = @poll.questions.for_render.sort_for_list @token = poll_voter_token(@poll, current_user) + @poll_questions_answers = Poll::Question::Answer.where(question: @poll.questions) @answers_by_question_id = {} poll_answers = ::Poll::Answer.by_question(@poll.question_ids).by_author(current_user.try(:id)) diff --git a/app/models/poll/question/answer.rb b/app/models/poll/question/answer.rb index 7514b85a4..ccbcf1abd 100644 --- a/app/models/poll/question/answer.rb +++ b/app/models/poll/question/answer.rb @@ -25,11 +25,10 @@ class Poll::Question::Answer < ActiveRecord::Base end def set_order - next_position = self.class.last_position(question_id) + 1 - self.given_order = next_position + self.given_order = self.class.last_position(question_id) + 1 end def self.last_position(question_id) - where(question_id: question_id).maximum("given_order") || 0 + where(question_id: question_id).maximum('given_order') || 0 end end diff --git a/app/views/polls/show.html.erb b/app/views/polls/show.html.erb index d9d871249..177d5e66d 100644 --- a/app/views/polls/show.html.erb +++ b/app/views/polls/show.html.erb @@ -79,7 +79,7 @@
- <% @poll.questions.map(&:question_answers).flatten.each do |answer| %> + <% @poll_questions_answers.each do |answer| %>
@@ -128,7 +128,7 @@
<% end %>
- +
@@ -138,4 +138,4 @@
<%= render "comments" %>
- \ No newline at end of file + diff --git a/spec/features/admin/poll/questions/answers/answers_spec.rb b/spec/features/admin/poll/questions/answers/answers_spec.rb index 48c2017f4..8514f2f26 100644 --- a/spec/features/admin/poll/questions/answers/answers_spec.rb +++ b/spec/features/admin/poll/questions/answers/answers_spec.rb @@ -43,8 +43,8 @@ feature 'Answers' do scenario 'Update' do question = create(:poll_question) - answer = create(:poll_question_answer, question: question, title: "Answer title", given_order: 1) - answer2 = create(:poll_question_answer, question: question, title: "Another title", given_order: 2) + answer = create(:poll_question_answer, question: question, title: "Answer title", given_order: 2) + answer2 = create(:poll_question_answer, question: question, title: "Another title", given_order: 1) visit admin_answer_path(answer) diff --git a/spec/features/polls/answers_spec.rb b/spec/features/polls/answers_spec.rb index 368597830..32b5732a0 100644 --- a/spec/features/polls/answers_spec.rb +++ b/spec/features/polls/answers_spec.rb @@ -9,8 +9,8 @@ feature 'Answers' do scenario "Index" do question = create(:poll_question) - answer1 = create(:poll_question_answer, question: question, given_order: 1) - answer2 = create(:poll_question_answer, question: question, given_order: 2) + answer1 = create(:poll_question_answer, question: question, given_order: 2) + answer2 = create(:poll_question_answer, question: question, given_order: 1) visit admin_question_path(question) @@ -56,4 +56,4 @@ feature 'Answers' do true end -end \ No newline at end of file +end diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index fc21e1e5f..90deca711 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -80,8 +80,8 @@ feature 'Polls' do scenario "Question answers appear in the given order" do question = create(:poll_question, poll: poll) - answer1 = create(:poll_question_answer, title: 'First', question: question, given_order: 1) - answer2 = create(:poll_question_answer, title: 'Second', question: question, given_order: 2) + answer1 = create(:poll_question_answer, title: 'First', question: question, given_order: 2) + answer2 = create(:poll_question_answer, title: 'Second', question: question, given_order: 1) visit poll_path(poll) @@ -92,8 +92,8 @@ feature 'Polls' do scenario "More info answers appear in the given order" do question = create(:poll_question, poll: poll) - answer1 = create(:poll_question_answer, title: 'First', question: question, given_order: 1) - answer2 = create(:poll_question_answer, title: 'Second', question: question, given_order: 2) + answer1 = create(:poll_question_answer, title: 'First', question: question, given_order: 2) + answer2 = create(:poll_question_answer, title: 'Second', question: question, given_order: 1) visit poll_path(poll)