From 26fa5d5d5ef225b4c3c88815e4396d3cad8de3a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Sun, 30 Aug 2015 23:17:47 +0200 Subject: [PATCH 01/16] adds setting value for max anonymous votes ratio --- db/seeds.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/db/seeds.rb b/db/seeds.rb index d39b15dba..6158a2a8d 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -5,4 +5,7 @@ Setting.create(key: 'official_level_1_name', value: 'Empleados públicos') Setting.create(key: 'official_level_2_name', value: 'Organización Municipal') Setting.create(key: 'official_level_3_name', value: 'Directores generales') Setting.create(key: 'official_level_4_name', value: 'Concejales') -Setting.create(key: 'official_level_5_name', value: 'Alcaldesa') \ No newline at end of file +Setting.create(key: 'official_level_5_name', value: 'Alcaldesa') + +# Max percentage of allowed anonymous votes on a debate +Setting.create(key: 'max_ratio_anon_votes_on_debates', value: '50') \ No newline at end of file From de71f551033849b8b43ecfe9960b68cf80c6d9d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Sun, 30 Aug 2015 23:27:22 +0200 Subject: [PATCH 02/16] adds cached_anonymous_votes_total to Debate --- ...50830212600_add_cached_anonymous_votes_total_to_debate.rb | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 db/migrate/20150830212600_add_cached_anonymous_votes_total_to_debate.rb diff --git a/db/migrate/20150830212600_add_cached_anonymous_votes_total_to_debate.rb b/db/migrate/20150830212600_add_cached_anonymous_votes_total_to_debate.rb new file mode 100644 index 000000000..196d5b517 --- /dev/null +++ b/db/migrate/20150830212600_add_cached_anonymous_votes_total_to_debate.rb @@ -0,0 +1,5 @@ +class AddCachedAnonymousVotesTotalToDebate < ActiveRecord::Migration + def change + add_column :debates, :cached_anonymous_votes_total, :integer, default: 0 + end +end From 916f0bf901514e71e375fde1195f5516c5ba8654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Mon, 31 Aug 2015 11:52:33 +0200 Subject: [PATCH 03/16] adds spec for Setting --- db/schema.rb | 29 +++++++++++++++-------------- spec/models/setting_spec.rb | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 spec/models/setting_spec.rb diff --git a/db/schema.rb b/db/schema.rb index fd22d3092..883d2a7e1 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: 20150828085718) do +ActiveRecord::Schema.define(version: 20150830212600) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -67,8 +67,8 @@ ActiveRecord::Schema.define(version: 20150828085718) do t.integer "rgt" t.datetime "created_at" t.datetime "updated_at" - t.datetime "hidden_at" t.integer "children_count", default: 0 + t.datetime "hidden_at" t.integer "flags_count", default: 0 t.datetime "ignored_flag_at" t.integer "moderator_id" @@ -87,20 +87,21 @@ ActiveRecord::Schema.define(version: 20150828085718) do add_index "comments", ["user_id"], name: "index_comments_on_user_id", using: :btree create_table "debates", force: :cascade do |t| - t.string "title", limit: 80 + t.string "title", limit: 80 t.text "description" t.integer "author_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "visit_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.datetime "hidden_at" - t.integer "flags_count", default: 0 - t.integer "cached_votes_total", default: 0 - t.integer "cached_votes_up", default: 0 - t.integer "cached_votes_down", default: 0 + t.string "visit_id" + t.integer "flags_count", default: 0 t.datetime "ignored_flag_at" - t.integer "comments_count", default: 0 + t.integer "cached_votes_total", default: 0 + t.integer "cached_votes_up", default: 0 + t.integer "cached_votes_down", default: 0 + t.integer "comments_count", default: 0 t.datetime "confirmed_hide_at" + t.integer "cached_anonymous_votes_total", default: 0 end add_index "debates", ["cached_votes_down"], name: "index_debates_on_cached_votes_down", using: :btree @@ -199,12 +200,13 @@ ActiveRecord::Schema.define(version: 20150828085718) do t.string "unconfirmed_email" t.boolean "email_on_debate_comment", default: false t.boolean "email_on_comment_reply", default: false - t.string "phone_number", limit: 30 t.string "official_position" t.integer "official_level", default: 0 t.datetime "hidden_at" - t.string "sms_confirmation_code" + t.string "phone_number", limit: 30 t.string "username" + t.datetime "confirmed_hide_at" + t.string "sms_confirmation_code" t.string "document_number" t.string "document_type" t.datetime "residence_verified_at" @@ -216,7 +218,6 @@ ActiveRecord::Schema.define(version: 20150828085718) do t.string "unconfirmed_phone" t.string "confirmed_phone" t.datetime "letter_requested_at" - t.datetime "confirmed_hide_at" end add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb new file mode 100644 index 000000000..82ecab088 --- /dev/null +++ b/spec/models/setting_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +describe Setting do + + it "should be accesible by key" do + create(:setting, key: "Important Setting", value: "42") + + expect(Setting.value_for("Important Setting")).to eq("42") + end + + it "should be nil if key does not exist" do + expect(Setting.value_for("Undefined key")).to be_nil + end + +end \ No newline at end of file From 2e5b5bccb557c557987d40754d675452d171702d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 31 Aug 2015 12:58:06 +0200 Subject: [PATCH 04/16] adds methods to decide if a user can vote a debate --- app/models/debate.rb | 13 ++++++++- app/views/admin/moderators/search.js.erb | 2 +- spec/models/debate_spec.rb | 37 ++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/app/models/debate.rb b/app/models/debate.rb index fdf939b2f..60f672d4e 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -31,7 +31,6 @@ class Debate < ActiveRecord::Base scope :sort_by_total_votes, -> { reorder(cached_votes_total: :desc) } scope :sort_by_likes , -> { reorder(cached_votes_up: :desc) } scope :sort_by_created_at, -> { reorder(created_at: :desc) } - # Ahoy setup visitable # Ahoy will automatically assign visit_id on create @@ -56,6 +55,10 @@ class Debate < ActiveRecord::Base cached_votes_total end + def total_anonymous_votes + cached_anonymous_votes_total + end + def editable? total_votes == 0 end @@ -64,6 +67,14 @@ class Debate < ActiveRecord::Base editable? && author == user end + def votable_by?(user) + user.level_three_verified? || user.level_two_verified? || anonymous_votes_ratio < Setting.value_for('max_ratio_anon_votes_on_debates').to_i + end + + def anonymous_votes_ratio + (cached_anonymous_votes_total.to_f / cached_votes_total) * 100 + end + def description super.try :html_safe end diff --git a/app/views/admin/moderators/search.js.erb b/app/views/admin/moderators/search.js.erb index 5b8a61207..a6ac48206 100644 --- a/app/views/admin/moderators/search.js.erb +++ b/app/views/admin/moderators/search.js.erb @@ -1 +1 @@ -$("#search-result").html("
<%= j render 'moderator', moderator: @moderator %>
"); +$("#search-result").html("
<%= j render 'moderator', moderator: @moderator %>
"); diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index 5e1ed76c8..22bd0bf91 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -78,6 +78,36 @@ describe Debate do end end + describe "#votable_by?" do + let(:debate) { create(:debate) } + + before(:all) do + create(:setting, key: "max_ratio_anon_votes_on_debates", value: "50") + end + + it "should be true for level two verified users" do + user = create(:user, residence_verified_at: Time.now, confirmed_phone: "666333111") + expect(debate.votable_by?(user)).to be true + end + + it "should be true for level three verified users" do + user = create(:user, verified_at: Time.now) + expect(debate.votable_by?(user)).to be true + end + + it "should be true for anonymous users if allowed anonymous votes" do + debate.update(cached_anonymous_votes_total: 42, cached_votes_total: 100) + user = create(:user) + expect(debate.votable_by?(user)).to be true + end + + it "should be false for anonymous users if too many anonymous votes" do + debate.update(cached_anonymous_votes_total: 52, cached_votes_total: 100) + user = create(:user) + expect(debate.votable_by?(user)).to be false + end + end + describe "#search" do let!(:economy) { create(:debate, tag_list: "Economy") } let!(:health) { create(:debate, tag_list: "Health") } @@ -102,4 +132,11 @@ describe Debate do end end + describe '#anonymous_votes_ratio' do + it "returns the percentage of anonymous votes of the total votes" do + debate = create(:debate, cached_anonymous_votes_total: 25, cached_votes_total: 100) + expect(debate.anonymous_votes_ratio).to eq(25.0) + end + end + end From 0dca4c277b6438788d9cbd428210699ddd145fab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 31 Aug 2015 13:35:56 +0200 Subject: [PATCH 05/16] adds Setting validation --- app/models/setting.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/setting.rb b/app/models/setting.rb index 1a52ebe60..8e1504bf3 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -1,4 +1,7 @@ class Setting < ActiveRecord::Base + + validates :key, presence: true, uniqueness: true + default_scope { order(key: :desc) } def self.value_for(key) From 3a1589561783563a4ff2634e071bc4ef129419cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Mon, 31 Aug 2015 13:37:07 +0200 Subject: [PATCH 06/16] loads seeds for testing --- spec/models/debate_spec.rb | 2 +- spec/spec_helper.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index 22bd0bf91..ac0c62570 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -82,7 +82,7 @@ describe Debate do let(:debate) { create(:debate) } before(:all) do - create(:setting, key: "max_ratio_anon_votes_on_debates", value: "50") + Setting.find_by(key: "max_ratio_anon_votes_on_debates").update(value: 50) end it "should be true for level two verified users" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bf1a65776..ef39ce84e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,6 +21,8 @@ RSpec.configure do |config| config.before(:each) do |example| DatabaseCleaner.strategy = example.metadata[:js] ? :truncation : :transaction DatabaseCleaner.start + + load "#{Rails.root}/db/seeds.rb" end config.after(:each) do From bbf96259fc764b3ee18c0940006da80fc57c676b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 31 Aug 2015 14:51:46 +0200 Subject: [PATCH 07/16] adds controller check for anonymous votes --- app/controllers/debates_controller.rb | 2 +- app/models/debate.rb | 1 + spec/controllers/debates_controller_spec.rb | 23 ++++++++++++++++++++- spec/features/votes_spec.rb | 2 +- spec/models/debate_spec.rb | 2 +- spec/spec_helper.rb | 1 - 6 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index 87df13d37..63b1bb34f 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -53,7 +53,7 @@ class DebatesController < ApplicationController end def vote - @debate.vote_by(voter: current_user, vote: params[:value]) + @debate.vote_by(voter: current_user, vote: params[:value]) if @debate.votable_by?(current_user) set_debate_votes(@debate) end diff --git a/app/models/debate.rb b/app/models/debate.rb index 60f672d4e..3f88eaff1 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -72,6 +72,7 @@ class Debate < ActiveRecord::Base end def anonymous_votes_ratio + return 0 if cached_votes_total == 0 (cached_anonymous_votes_total.to_f / cached_votes_total) * 100 end diff --git a/spec/controllers/debates_controller_spec.rb b/spec/controllers/debates_controller_spec.rb index 9ab4f7544..1628c9bcf 100644 --- a/spec/controllers/debates_controller_spec.rb +++ b/spec/controllers/debates_controller_spec.rb @@ -12,7 +12,6 @@ describe DebatesController do end describe 'POST create' do - it 'should create an ahoy event' do sign_in create(:user) @@ -22,4 +21,26 @@ describe DebatesController do expect(Ahoy::Event.last.properties['debate_id']).to eq Debate.last.id end end + + describe "Vote with too many anonymous votes" do + it 'should allow vote if user is allowed' do + Setting.find_by(key: "max_ratio_anon_votes_on_debates").update(value: 100) + debate = create(:debate) + sign_in create(:user) + + expect do + xhr :post, :vote, id: debate.id, value: 'yes' + end.to change { debate.reload.votes_for.size }.by(1) + end + + it 'should not allow vote if user is not allowed' do + Setting.find_by(key: "max_ratio_anon_votes_on_debates").update(value: 0) + debate = create(:debate) + sign_in create(:user) + + expect do + xhr :post, :vote, id: debate.id, value: 'yes' + end.to_not change { debate.reload.votes_for.size } + end + end end diff --git a/spec/features/votes_spec.rb b/spec/features/votes_spec.rb index 548c7e308..ea9761bde 100644 --- a/spec/features/votes_spec.rb +++ b/spec/features/votes_spec.rb @@ -5,7 +5,7 @@ feature 'Votes' do feature 'Debates' do background do - @manuela = create(:user) + @manuela = create(:user, verified_at: Time.now) @pablo = create(:user) @debate = create(:debate) diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index ac0c62570..f94bb02c6 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -81,7 +81,7 @@ describe Debate do describe "#votable_by?" do let(:debate) { create(:debate) } - before(:all) do + before(:each) do Setting.find_by(key: "max_ratio_anon_votes_on_debates").update(value: 50) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ef39ce84e..cb6b56d42 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,7 +21,6 @@ RSpec.configure do |config| config.before(:each) do |example| DatabaseCleaner.strategy = example.metadata[:js] ? :truncation : :transaction DatabaseCleaner.start - load "#{Rails.root}/db/seeds.rb" end From 9e7722e7ef1d5249b7dbbb520206502d3cf778dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Mon, 31 Aug 2015 16:52:08 +0200 Subject: [PATCH 08/16] adds verification specs for user includes #unverified? method --- lib/verification.rb | 4 ++++ spec/models/user_spec.rb | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/lib/verification.rb b/lib/verification.rb index 2e784d65c..1e2f7be81 100644 --- a/lib/verification.rb +++ b/lib/verification.rb @@ -16,5 +16,9 @@ module Verification verified_at.present? end + def unverified? + !level_two_verified? && !level_three_verified? + end + end \ No newline at end of file diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d00ed71ca..87d0994bb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -254,4 +254,49 @@ describe User do end end + describe "verification levels" do + it "residence_verified? is true only if residence_verified_at" do + user = create(:user, residence_verified_at: Time.now) + expect(user.residence_verified?).to eq(true) + + user = create(:user, residence_verified_at: nil) + expect(user.residence_verified?).to eq(false) + end + + it "sms_verified? is true only if confirmed_phone" do + user = create(:user, confirmed_phone: "123456789") + expect(user.sms_verified?).to eq(true) + + user = create(:user, confirmed_phone: nil) + expect(user.sms_verified?).to eq(false) + end + + it "level_two_verified? is true only if residence_verified_at and confirmed_phone" do + user = create(:user, confirmed_phone: "123456789", residence_verified_at: Time.now) + expect(user.level_two_verified?).to eq(true) + + user = create(:user, confirmed_phone: nil, residence_verified_at: Time.now) + expect(user.level_two_verified?).to eq(false) + + user = create(:user, confirmed_phone: "123456789", residence_verified_at: nil) + expect(user.level_two_verified?).to eq(false) + end + + it "level_three_verified? is true only if verified_at" do + user = create(:user, verified_at: Time.now) + expect(user.level_three_verified?).to eq(true) + + user = create(:user, verified_at: nil) + expect(user.level_three_verified?).to eq(false) + end + + it "unverified? is true only if not level_three_verified and not level_two_verified" do + user = create(:user, verified_at: nil, confirmed_phone: nil) + expect(user.unverified?).to eq(true) + + user = create(:user, verified_at: Time.now, confirmed_phone: "123456789", residence_verified_at: Time.now) + expect(user.unverified?).to eq(false) + end + end + end From 872a23443b058ec567e45ee7ce1cd51c05a5f9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Mon, 31 Aug 2015 17:03:52 +0200 Subject: [PATCH 09/16] updates anonymous counter in debate when voted refactors to move all related code to Debate model --- app/controllers/debates_controller.rb | 2 +- app/models/debate.rb | 11 ++++- spec/models/debate_spec.rb | 60 +++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index 63b1bb34f..451d8e893 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -53,7 +53,7 @@ class DebatesController < ApplicationController end def vote - @debate.vote_by(voter: current_user, vote: params[:value]) if @debate.votable_by?(current_user) + @debate.register_vote(current_user, params[:value]) set_debate_votes(@debate) end diff --git a/app/models/debate.rb b/app/models/debate.rb index 3f88eaff1..e088d8141 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -67,8 +67,17 @@ class Debate < ActiveRecord::Base editable? && author == user end + def register_vote(user, vote_value) + if votable_by?(user) + Debate.increment_counter(:cached_anonymous_votes_total, id) if (user.unverified? && !user.voted_for?(self)) + vote_by(voter: user, vote: vote_value) + end + end + def votable_by?(user) - user.level_three_verified? || user.level_two_verified? || anonymous_votes_ratio < Setting.value_for('max_ratio_anon_votes_on_debates').to_i + !user.unverified? || + anonymous_votes_ratio < Setting.value_for('max_ratio_anon_votes_on_debates').to_i || + user.voted_for?(self) end def anonymous_votes_ratio diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index f94bb02c6..96e3b3c96 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -78,6 +78,66 @@ describe Debate do end end + describe "#register_vote" do + let(:debate) { create(:debate) } + + before(:each) do + Setting.find_by(key: "max_ratio_anon_votes_on_debates").update(value: 50) + end + + describe "from level two verified users" do + it "should register vote" do + user = create(:user, residence_verified_at: Time.now, confirmed_phone: "666333111") + expect {debate.register_vote(user, 'yes')}.to change{debate.reload.votes_for.size}.by(1) + end + + it "should not increase anonymous votes counter " do + user = create(:user, residence_verified_at: Time.now, confirmed_phone: "666333111") + expect {debate.register_vote(user, 'yes')}.to_not change{debate.reload.cached_anonymous_votes_total} + end + end + + describe "from level three verified users" do + it "should register vote" do + user = create(:user, verified_at: Time.now) + expect {debate.register_vote(user, 'yes')}.to change{debate.reload.votes_for.size}.by(1) + end + + it "should not increase anonymous votes counter " do + user = create(:user, verified_at: Time.now) + expect {debate.register_vote(user, 'yes')}.to_not change{debate.reload.cached_anonymous_votes_total} + end + end + + describe "from anonymous users when anonymous votes are allowed" do + before(:each) {debate.update(cached_anonymous_votes_total: 42, cached_votes_total: 100)} + + it "should register vote " do + user = create(:user) + expect {debate.register_vote(user, 'yes')}.to change {debate.reload.votes_for.size}.by(1) + end + + it "should increase anonymous votes counter " do + user = create(:user) + expect {debate.register_vote(user, 'yes')}.to change {debate.reload.cached_anonymous_votes_total}.by(1) + end + end + + describe "from anonymous users when there are too many anonymous votes" do + before(:each) {debate.update(cached_anonymous_votes_total: 52, cached_votes_total: 100)} + + it "should not register vote " do + user = create(:user) + expect {debate.register_vote(user, 'yes')}.to_not change {debate.reload.votes_for.size} + end + + it "should not increase anonymous votes counter " do + user = create(:user) + expect {debate.register_vote(user, 'yes')}.to_not change {debate.reload.cached_anonymous_votes_total} + end + end + end + describe "#votable_by?" do let(:debate) { create(:debate) } From 57e373a7cf3dd3b0ff7b579bc7e8ea6863085021 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Cabeza Date: Sat, 22 Aug 2015 11:33:29 +0200 Subject: [PATCH 10/16] Adds styles for too much anonymous comments --- app/assets/stylesheets/participacion.scss | 6 ++++++ app/views/debates/show.html.erb | 6 ++++++ config/locales/en.yml | 2 ++ config/locales/es.yml | 2 ++ 4 files changed, 16 insertions(+) diff --git a/app/assets/stylesheets/participacion.scss b/app/assets/stylesheets/participacion.scss index 6c498adcc..0b027001b 100644 --- a/app/assets/stylesheets/participacion.scss +++ b/app/assets/stylesheets/participacion.scss @@ -773,6 +773,12 @@ form { background-color: $warning-bg; border-color: $warning-border; color: $warning-color; + + a { + color: $warning-color; + font-weight: bold; + text-decoration: underline; + } } &.alert { diff --git a/app/views/debates/show.html.erb b/app/views/debates/show.html.erb index 8fab71128..64fb4d9d8 100644 --- a/app/views/debates/show.html.erb +++ b/app/views/debates/show.html.erb @@ -61,6 +61,12 @@