From 4d140d0e10fc71e1135912d7b8dabd802d3af467 Mon Sep 17 00:00:00 2001 From: tuuli Date: Tue, 20 Nov 2018 09:21:35 +0100 Subject: [PATCH 01/18] Preliminary commit --- .rubocop_todo.yml | 2 + .../actions_thermometers_controller.rb | 19 +++++ .../plugins/thermometers_controller.rb | 5 +- app/liquid/liquid_renderer.rb | 8 +- app/models/page.rb | 2 +- app/models/plugins.rb | 25 +++--- app/models/plugins/actions_thermometer.rb | 82 +++++++++++++++++++ app/models/plugins/donations_thermometer.rb | 82 +++++++++++++++++++ app/models/plugins/thermometer.rb | 5 ++ app/views/pages/_personalization.slim | 2 +- app/views/pages/edit.slim | 6 +- .../_actions_thermometer.liquid | 19 +++++ .../_form.slim | 0 .../donations_thermometers/_form.liquid | 0 app/views/plugins/show.html.slim | 2 +- .../plugins/thermometers/_thermometer.liquid | 24 +++--- .../20181116103619_add_type_to_thermometer.rb | 5 ++ db/schema.rb | 3 +- lib/tasks/sumofus.rake | 6 +- .../plugins/thermometers_controller_spec.rb | 2 +- .../factories/plugins_actions_thermometers.rb | 25 ++++++ spec/factories/plugins_thermometers.rb | 4 + spec/liquid/liquid_renderer_spec.rb | 26 +++--- spec/liquid/page_plugin_switcher_spec.rb | 17 ++-- spec/models/page_spec.rb | 4 +- spec/models/plugins/plugins_spec.rb | 68 +++++++-------- spec/models/plugins/thermometer_spec.rb | 9 +- spec/services/page_builder_spec.rb | 3 +- spec/services/page_cloner_spec.rb | 10 ++- spec/services/page_updater_spec.rb | 22 ++--- .../search_by_plugin_spec.rb | 17 ++-- 31 files changed, 383 insertions(+), 121 deletions(-) create mode 100644 app/controllers/plugins/actions_thermometers_controller.rb create mode 100644 app/models/plugins/actions_thermometer.rb create mode 100644 app/models/plugins/donations_thermometer.rb create mode 100644 app/views/plugins/actions_thermometers/_actions_thermometer.liquid rename app/views/plugins/{thermometers => actions_thermometers}/_form.slim (100%) create mode 100644 app/views/plugins/donations_thermometers/_form.liquid create mode 100644 db/migrate/20181116103619_add_type_to_thermometer.rb create mode 100644 spec/factories/plugins_actions_thermometers.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index be47d83cc..2338039eb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -126,6 +126,8 @@ Style/EachWithObject: Style/FormatString: Exclude: - 'app/models/plugins/thermometer.rb' + - 'app/models/plugins/actions_thermometer.rb' + - 'app/models/plugins/donations_thermometer.rb' # Offense count: 5 # Configuration parameters: MinBodyLength. diff --git a/app/controllers/plugins/actions_thermometers_controller.rb b/app/controllers/plugins/actions_thermometers_controller.rb new file mode 100644 index 000000000..2acbe80d1 --- /dev/null +++ b/app/controllers/plugins/actions_thermometers_controller.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Plugins::ActionsThermometersController < Plugins::BaseController + private + + def permitted_params + params + .require(:plugins_actions_thermometer) + .permit(:title, :offset, :goal, :active, :type) + end + + def plugin_class + Plugins::ActionsThermometer + end + + def plugin_symbol + :plugins_thermometer + end +end diff --git a/app/controllers/plugins/thermometers_controller.rb b/app/controllers/plugins/thermometers_controller.rb index aea89769e..8b119850f 100644 --- a/app/controllers/plugins/thermometers_controller.rb +++ b/app/controllers/plugins/thermometers_controller.rb @@ -1,15 +1,16 @@ # frozen_string_literal: true + class Plugins::ThermometersController < Plugins::BaseController private def permitted_params params .require(:plugins_thermometer) - .permit(:title, :offset, :goal, :active) + .permit(:title, :offset, :goal, :active, :type) end def plugin_class - Plugins::Thermometer + Plugins::ActionsThermometer end def plugin_symbol diff --git a/app/liquid/liquid_renderer.rb b/app/liquid/liquid_renderer.rb index b7f3d4d77..057de4e98 100644 --- a/app/liquid/liquid_renderer.rb +++ b/app/liquid/liquid_renderer.rb @@ -42,7 +42,7 @@ def personalization_data form_values: form_values, outstanding_fields: outstanding_fields, donation_bands: donation_bands, - thermometer: thermometer, + actions_thermometer: actions_thermometer, call_tool: call_tool_data, email_tool: email_tool_data, email_pension: email_pension_data, @@ -117,8 +117,10 @@ def donation_bands isolate_from_plugin_data(:donation_bands).first end - def thermometer - plugin_data.deep_symbolize_keys[:plugins][:thermometer].try(:values).try(:first) + def actions_thermometer + data = plugin_data.deep_symbolize_keys[:plugins][:actions_thermometer].try(:values).try(:first) + pp 'Actions thermometer data:', data + data end def call_tool_data diff --git a/app/models/page.rb b/app/models/page.rb index 6eed80f2a..7ac025396 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -92,7 +92,7 @@ def plugins end def plugin_names - plugins.map { |plugin| plugin.model_name.name.split('::')[1].downcase } + plugins.map { |plugin| plugin.name.demodulize.underscore } end def tag_names diff --git a/app/models/plugins.rb b/app/models/plugins.rb index 91de3d4f7..6df519e4b 100644 --- a/app/models/plugins.rb +++ b/app/models/plugins.rb @@ -2,6 +2,18 @@ module Plugins class << self + def registered + [Plugins::Petition, + Plugins::ActionsThermometer, + Plugins::DonationsThermometer, + Plugins::Fundraiser, + Plugins::Survey, + Plugins::Text, + Plugins::CallTool, + Plugins::EmailTool, + Plugins::EmailPension] + end + def table_name_prefix 'plugins_' end @@ -22,17 +34,6 @@ def create_for_page(plugin_name, page, ref) plugin.save! end - def registered - [Plugins::Petition, - Plugins::Thermometer, - Plugins::Fundraiser, - Plugins::Survey, - Plugins::Text, - Plugins::CallTool, - Plugins::EmailTool, - Plugins::EmailPension] - end - def translate_defaults(defaults, locale) defaults.inject({}) do |translated, (key, val)| translated[key] = val.is_a?(String) ? I18n.t(val, locale: locale) : val @@ -55,7 +56,7 @@ def data_for_view(page, supplemental_data = {}) end def names - registered.map { |plugin| plugin.to_s.underscore.split('/').last } + registered.map { |plugin| plugin.name.demodulize.underscore } end def find_for(plugin_class, plugin_id) diff --git a/app/models/plugins/actions_thermometer.rb b/app/models/plugins/actions_thermometer.rb new file mode 100644 index 000000000..d87cc2e6d --- /dev/null +++ b/app/models/plugins/actions_thermometer.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: plugins_thermometers +# +# id :integer not null, primary key +# type :string not null +# title :string +# offset :integer +# page_id :integer +# active :boolean default("false") +# created_at :datetime not null +# updated_at :datetime not null +# ref :string +# + +class Plugins::ActionsThermometer < Plugins::Thermometer + belongs_to :page, touch: true + + DEFAULTS = { offset: 0 }.freeze + GOALS = [100, 200, 300, 400, 500, + 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, + 10_000, 15_000, 20_000, 25_000, 50_000, 75_000, 100_000, + 150_000, 200_000, 250_000, 300_000, 500_000, 750_000, + 1_000_000, 1_500_000, 2_000_000].freeze + + validates :offset, presence: true, + numericality: { greater_than_or_equal_to: 0 } + after_initialize :set_defaults + + def current_total + offset + action_count + end + + def current_progress + current_total / goal.to_f * 100 + end + + def goal + GOALS.find { |goal| current_total < goal } || next_goal_as_multiple_of(1_000_000) + end + + def liquid_data(_supplemental_data = {}) + attributes.merge( + percentage: current_progress, + remaining: ActionController::Base.helpers.number_with_delimiter(goal - current_total), + signatures: ActionController::Base.helpers.number_with_delimiter(current_total), + goal_k: abbreviate_number(goal) + ) + end + + def name + self.class.name.demodulize + end + + private + + def action_count + @action_count ||= if page.campaign_id.present? + Page.where(campaign_id: page.campaign_id).sum(:action_count) + else + page.action_count + end + end + + def abbreviate_number(number) + return number.to_s if number < 1000 + return "#{(goal / 1000).to_i}k" if number < 1_000_000 + locale = page.try(:language).try(:code) + "%g #{I18n.t('thermometer.million', locale: locale)}" % (goal / 1_000_000.0).round(1) + end + + def next_goal_as_multiple_of(step) + remainder = current_total % step + current_total + step - remainder + end + + def set_defaults + self.offset ||= DEFAULTS[:offset] + end +end diff --git a/app/models/plugins/donations_thermometer.rb b/app/models/plugins/donations_thermometer.rb new file mode 100644 index 000000000..ea8e7c144 --- /dev/null +++ b/app/models/plugins/donations_thermometer.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: plugins_thermometers +# +# id :integer not null, primary key +# type :string not null +# title :string +# offset :integer +# page_id :integer +# active :boolean default("false") +# created_at :datetime not null +# updated_at :datetime not null +# ref :string +# + +class Plugins::DonationsThermometer < Plugins::Thermometer + belongs_to :page, touch: true + + DEFAULTS = { offset: 0 }.freeze + GOALS = [100, 200, 300, 400, 500, + 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, + 10_000, 15_000, 20_000, 25_000, 50_000, 75_000, 100_000, + 150_000, 200_000, 250_000, 300_000, 500_000, 750_000, + 1_000_000, 1_500_000, 2_000_000].freeze + + validates :offset, presence: true, + numericality: { greater_than_or_equal_to: 0 } + after_initialize :set_defaults + + def current_total + offset + action_count + end + + def current_progress + current_total / goal.to_f * 100 + end + + def goal + GOALS.find { |goal| current_total < goal } || next_goal_as_multiple_of(1_000_000) + end + + def liquid_data(_supplemental_data = {}) + attributes.merge( + percentage: current_progress, + remaining: ActionController::Base.helpers.number_with_delimiter(goal - current_total), + signatures: ActionController::Base.helpers.number_with_delimiter(current_total), + goal_k: abbreviate_number(goal) + ) + end + + def name + self.class.name.demodulize + end + + private + + def action_count + @action_count ||= if page.campaign_id.present? + Page.where(campaign_id: page.campaign_id).sum(:action_count) + else + page.action_count + end + end + + def abbreviate_number(number) + return number.to_s if number < 1000 + return "#{(goal / 1000).to_i}k" if number < 1_000_000 + locale = page.try(:language).try(:code) + "%g #{I18n.t('thermometer.million', locale: locale)}" % (goal / 1_000_000.0).round(1) + end + + def next_goal_as_multiple_of(step) + remainder = current_total % step + current_total + step - remainder + end + + def set_defaults + self.offset ||= DEFAULTS[:offset] + end +end diff --git a/app/models/plugins/thermometer.rb b/app/models/plugins/thermometer.rb index 0a93fbaa1..1c99a4edc 100644 --- a/app/models/plugins/thermometer.rb +++ b/app/models/plugins/thermometer.rb @@ -5,6 +5,7 @@ # Table name: plugins_thermometers # # id :integer not null, primary key +# type :string not null # title :string # offset :integer # page_id :integer @@ -15,6 +16,10 @@ # class Plugins::Thermometer < ApplicationRecord + def self.store_full_sti_class + false + end + belongs_to :page, touch: true DEFAULTS = { offset: 0 }.freeze diff --git a/app/views/pages/_personalization.slim b/app/views/pages/_personalization.slim index fa3a93a66..682c3cfaa 100644 --- a/app/views/pages/_personalization.slim +++ b/app/views/pages/_personalization.slim @@ -5,7 +5,7 @@ javascript: paymentMethods: #{serialize(data, 'payment_methods')}, donationBands: #{serialize(data, 'donation_bands')}, actionCount: #{ serialize(data, 'action_count') }, - thermometer: #{ serialize(data, 'thermometer') }, + actionsThermometer: #{ serialize(data, 'actions_thermometer') }, formValues: #{serialize(data, 'form_values') }, callTool: #{ serialize(data, 'call_tool') }, emailTool: #{ serialize(data, 'email_tool') }, diff --git a/app/views/pages/edit.slim b/app/views/pages/edit.slim index c03eb6072..ed51b268c 100644 --- a/app/views/pages/edit.slim +++ b/app/views/pages/edit.slim @@ -1,7 +1,7 @@ - content_for(:title, @page.title) = render 'edit_sidebar', page: @page -- low_priority_plugins = [Plugins::Thermometer] +- low_priority_plugins = [Plugins::ActionsThermometer, Plugins::DonationsThermometer] section.page-edit-step#basics data-icon='paragraph' h1.page-edit-step__title = t('.content') @@ -15,7 +15,7 @@ section.page-edit-step#layout data-icon='cog' - next if low_priority_plugins.include? plugin.class section.page-edit-step id=plugin_section_id(plugin) data-icon=plugin_icon(plugin) h1.page-edit-step__title= plugin_title(plugin) - = render "#{plugin.class.to_s.underscore.pluralize}/form", plugin: plugin, page: @page + = render "#{plugin.class.name.underscore.pluralize}/form", plugin: plugin, page: @page section.page-edit-step#pictures data-icon='camera-retro' h1.page-edit-step__title = t('.pictures') @@ -33,7 +33,7 @@ section.page-edit-step#sources data-icon='link' - next unless low_priority_plugins.include? plugin.class section.page-edit-step id=plugin_section_id(plugin) data-icon=plugin_icon(plugin) h1.page-edit-step__title= plugin_title(plugin) - = render "#{plugin.class.to_s.underscore.pluralize}/form", plugin: plugin, page: @page + = render "#{plugin.class.name.underscore.pluralize}/form", plugin: plugin, page: @page section.page-edit-step.page-edit-step--just-title#review data-icon='eye' data-link-to=member_facing_page_url(@page) h1.page-edit-step__title diff --git a/app/views/plugins/actions_thermometers/_actions_thermometer.liquid b/app/views/plugins/actions_thermometers/_actions_thermometer.liquid new file mode 100644 index 000000000..e2cfd4152 --- /dev/null +++ b/app/views/plugins/actions_thermometers/_actions_thermometer.liquid @@ -0,0 +1,19 @@ +{% if plugins.actions_thermometer[ref].active %} + +
+
+
+ {{ plugins.actions_thermometer[ref].signatures }} {{ 'actions_thermometer.signatures' | t }} +
+
+ {{ 'actions_thermometer.signatures_until_goal' | + val: 'goal', plugins.actions_thermometer[ref].goal_k | + val: 'remaining', plugins.actions_thermometer[ref].remaining | t }} +
+
+
+
+
+
+{% endif %} diff --git a/app/views/plugins/thermometers/_form.slim b/app/views/plugins/actions_thermometers/_form.slim similarity index 100% rename from app/views/plugins/thermometers/_form.slim rename to app/views/plugins/actions_thermometers/_form.slim diff --git a/app/views/plugins/donations_thermometers/_form.liquid b/app/views/plugins/donations_thermometers/_form.liquid new file mode 100644 index 000000000..e69de29bb diff --git a/app/views/plugins/show.html.slim b/app/views/plugins/show.html.slim index 8db294c3f..b00c2e49c 100644 --- a/app/views/plugins/show.html.slim +++ b/app/views/plugins/show.html.slim @@ -8,6 +8,6 @@ = page_nav_item(plugin_title(plugin), path) .col-md-10 - = render partial: "#{@plugin.class.to_s.underscore.pluralize}/form", locals: { plugin: @plugin, page: @page } + = render partial: "#{@plugin.class.name.underscore.pluralize}/form", locals: { plugin: @plugin, page: @page } diff --git a/app/views/plugins/thermometers/_thermometer.liquid b/app/views/plugins/thermometers/_thermometer.liquid index 7e02930cb..6533ace32 100644 --- a/app/views/plugins/thermometers/_thermometer.liquid +++ b/app/views/plugins/thermometers/_thermometer.liquid @@ -1,19 +1,19 @@ {% if plugins.thermometer[ref].active %} - -
+ +
-
- {{ plugins.thermometer[ref].signatures }} {{ 'thermometer.signatures' | t }} -
-
- {{ 'thermometer.signatures_until_goal' | +
+ {{ plugins.thermometer[ref].signatures }} {{ 'thermometer.signatures' | t }} +
+
+ {{ 'thermometer.signatures_until_goal' | val: 'goal', plugins.thermometer[ref].goal_k | val: 'remaining', plugins.thermometer[ref].remaining | t }} -
+
-
+
-
-{% endif %} \ No newline at end of file +
+{% endif %} diff --git a/db/migrate/20181116103619_add_type_to_thermometer.rb b/db/migrate/20181116103619_add_type_to_thermometer.rb new file mode 100644 index 000000000..86717cc1a --- /dev/null +++ b/db/migrate/20181116103619_add_type_to_thermometer.rb @@ -0,0 +1,5 @@ +class AddTypeToThermometer < ActiveRecord::Migration[5.2] + def change + add_column :plugins_thermometers, :type, :string, default: 'ActionsThermometer', null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index ae628cdb2..c49a77a99 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_11_12_103031) do +ActiveRecord::Schema.define(version: 2018_11_16_103619) do # These are extensions that must be enabled in order to support this database enable_extension "intarray" @@ -551,6 +551,7 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "ref" + t.string "type", default: "ActionsThermometer", null: false t.index ["page_id"], name: "index_plugins_thermometers_on_page_id" end diff --git a/lib/tasks/sumofus.rake b/lib/tasks/sumofus.rake index 49a76d9d9..f9db578ba 100644 --- a/lib/tasks/sumofus.rake +++ b/lib/tasks/sumofus.rake @@ -196,9 +196,9 @@ namespace :sumofus do Page.reset_counters(page.id, :actions) Page.update_counters(page.id, action_count: entry['signature_count']) - thermometer = Plugins::Thermometer.where(page_id: page.id).first - thermometer.goal = entry['thermometer_target'] - thermometer.save! + actions_thermometer = Plugins::ActionsThermometer.where(page_id: page.id).first + actions_thermometer.goal = entry['thermometer_target'] + actions_thermometer.save! petition = Plugins::Petition.where(page_id: page.id).first petition.description = entry['petition_ask'].delete('"').gsub('Petition Text:', '') petition.target = entry['petition_target'].gsub(/Sign our petition to /i, '').gsub(/Sign the petition to /, '').delete(':') diff --git a/spec/controllers/plugins/thermometers_controller_spec.rb b/spec/controllers/plugins/thermometers_controller_spec.rb index 00a8c6bb1..5c9b483a4 100644 --- a/spec/controllers/plugins/thermometers_controller_spec.rb +++ b/spec/controllers/plugins/thermometers_controller_spec.rb @@ -4,5 +4,5 @@ require_relative 'shared_examples' describe Plugins::ThermometersController do - include_examples 'plugins controller', Plugins::Thermometer, :plugins_thermometer + include_examples 'plugins controller', Plugins::ActionsThermometer, :plugins_thermometer end diff --git a/spec/factories/plugins_actions_thermometers.rb b/spec/factories/plugins_actions_thermometers.rb new file mode 100644 index 000000000..130473b65 --- /dev/null +++ b/spec/factories/plugins_actions_thermometers.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: plugins_thermometers +# +# id :integer not null, primary key +# title :string +# offset :integer +# page_id :integer +# active :boolean default("false") +# created_at :datetime not null +# updated_at :datetime not null +# ref :string +# + +FactoryGirl.define do + factory :plugins_actions_thermometer, class: 'Plugins::Thermometer' do + title 'MyString' + type 'ActionsThermometer' + offset 1 + page nil + active false + end +end diff --git a/spec/factories/plugins_thermometers.rb b/spec/factories/plugins_thermometers.rb index df339c038..c7cffb286 100644 --- a/spec/factories/plugins_thermometers.rb +++ b/spec/factories/plugins_thermometers.rb @@ -20,5 +20,9 @@ offset { 1 } page { nil } active { false } + + trait :donations_thermometer do + type { 'DonationsThermometer' } + end end end diff --git a/spec/liquid/liquid_renderer_spec.rb b/spec/liquid/liquid_renderer_spec.rb index 0c53373d5..a52274329 100644 --- a/spec/liquid/liquid_renderer_spec.rb +++ b/spec/liquid/liquid_renderer_spec.rb @@ -162,7 +162,7 @@ location member donation_bands - thermometer + actions_thermometer action_count payment_methods form_values @@ -181,7 +181,7 @@ end it "is [] if it's plugins don't have forms" do - create :plugins_thermometer, page: page + create :plugins_actions_thermometer, page: page expect(LiquidRenderer.new(page).personalization_data['outstanding_fields']).to eq [] end @@ -354,23 +354,23 @@ end end - describe 'thermometer' do + describe 'actions thermometer' do it 'is nil if no plugins' do expect(page.plugins.size).to eq 0 - expect(LiquidRenderer.new(page).personalization_data['thermometer']).to eq nil + expect(LiquidRenderer.new(page).personalization_data['actions_thermometer']).to eq nil end - it 'is nil if no thermometer plugin' do + it 'is nil if no actions thermometer plugin' do create :plugins_fundraiser, page: page expect(page.plugins.size).to eq 1 - expect(LiquidRenderer.new(page).personalization_data['thermometer']).to eq nil + expect(LiquidRenderer.new(page).personalization_data['actions_thermometer']).to eq nil end - it "is serializes the thermometer plugin's data" do - t1 = create :plugins_thermometer, page: page + it "is serializes the actions thermometer plugin's data" do + t1 = create :plugins_actions_thermometer, page: page t1.current_progress # allow goal to update expected = t1.liquid_data.stringify_keys - actual = LiquidRenderer.new(page).personalization_data['thermometer'] + actual = LiquidRenderer.new(page).personalization_data['actions_thermometer'] # disagreement over timestamps is not what this test is about [expected, actual].each do |h| h.delete('updated_at') @@ -379,13 +379,13 @@ expect(actual).to eq expected end - it 'is uses the first if multiple thermometer plugins' do - t1 = create :plugins_thermometer, page: page, ref: 'secondary' - create :plugins_thermometer, page: page + it 'is uses the first if multiple actions thermometer plugins' do + t1 = create :plugins_actions_thermometer, page: page, ref: 'secondary' + create :plugins_actions_thermometer, page: page expect(page.plugins.size).to eq 2 t1.current_progress # allow goal to update expected = t1.liquid_data.stringify_keys - actual = LiquidRenderer.new(page).personalization_data['thermometer'] + actual = LiquidRenderer.new(page).personalization_data['actions_thermometer'] # disagreement over timestamps is not what this test is about [expected, actual].each do |h| h.delete('updated_at') diff --git a/spec/liquid/page_plugin_switcher_spec.rb b/spec/liquid/page_plugin_switcher_spec.rb index 4346f6675..98dcf6fcb 100644 --- a/spec/liquid/page_plugin_switcher_spec.rb +++ b/spec/liquid/page_plugin_switcher_spec.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true + require 'rails_helper' describe PagePluginSwitcher do let!(:petition_partial) { create :liquid_partial, title: 'petition', content: '{{ plugins.petition[ref] }}' } - let!(:thermo_partial) { create :liquid_partial, title: 'thermo', content: '{{ plugins.thermometer[ref] }}' } + let!(:thermo_partial) { create :liquid_partial, title: 'thermo', content: '{{ plugins.actions_thermometer[ref] }}' } let!(:petition_nested_thermo_partial) do create :liquid_partial, title: 'petition_nested_thermo', content: '{{ plugins.petition[ref] }}{% include "thermo" %}' end @@ -24,12 +25,12 @@ page.plugins.each(&:destroy) expect(page.plugins).to be_empty expect(page.liquid_layout).to eq both_refless_layout - expect { switcher.switch(both_refless_layout) }.to change { Plugins::Petition.count }.by(1).and change { Plugins::Thermometer.count }.by 1 + expect { switcher.switch(both_refless_layout) }.to change { Plugins::Petition.count }.by(1).and change { Plugins::ActionsThermometer.count }.by 1 expect(page.plugins.size).to eq 2 end it 'creates new plugins when switching to a template with more plugins' do - expect { switcher.switch(many_petition_layout) }.to change { Plugins::Petition.count }.by(2).and change { Plugins::Thermometer.count }.by -1 + expect { switcher.switch(many_petition_layout) }.to change { Plugins::Petition.count }.by(2).and change { Plugins::ActionsThermometer.count }.by -1 expect(page.plugins.size).to eq 3 end @@ -60,7 +61,7 @@ .to change { Plugins::Petition.count } .from(1) .to(3) - expect(page.plugins.map(&:class)).to match_array([Plugins::Petition] * 3 + [Plugins::Thermometer]) + expect(page.plugins.map(&:class)).to match_array([Plugins::Petition] * 3 + [Plugins::ActionsThermometer]) end end @@ -87,10 +88,10 @@ end it 'does replace one instance but not the other' do - thermo = page.plugins.select { |p| p.name == 'Thermometer' }.first - expect { switcher.switch(thermo_petition_ref_layout) }.to change { Plugins::Thermometer.count }.by 0 + thermo = page.plugins.select { |p| p.name == 'ActionsThermometer' }.first + expect { switcher.switch(thermo_petition_ref_layout) }.to change { Plugins::ActionsThermometer.count }.by 0 expect { thermo.reload }.not_to raise_error - new_thermo = page.plugins.select { |p| p.name == 'Thermometer' }.first + new_thermo = page.plugins.select { |p| p.name == 'ActionsThermometer' }.first expect(new_thermo.id).to eq thermo.id end end @@ -98,7 +99,7 @@ describe 'destroying' do it 'destroys all plugins when switching to a template without plugins' do expect { switcher.switch(blank_layout) } - .to change { Plugins::Thermometer.count }.by(-1) + .to change { Plugins::ActionsThermometer.count }.by(-1) .and change { Plugins::Petition.count }.by(-1) plugins = page.plugins diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index b38607d69..11db57ad8 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -448,8 +448,8 @@ describe 'plugins' do it 'correctly lists the names of plugins' do page = create :page - [create(:plugins_petition, page: page), create(:plugins_fundraiser, page: page), create(:plugins_thermometer, page: page)] - plugin_names = %w[petition fundraiser thermometer] + [create(:plugins_petition, page: page), create(:plugins_fundraiser, page: page), create(:plugins_actions_thermometer, page: page)] + plugin_names = %w[petition fundraiser actions_thermometer] expect(page.plugin_names).to match_array(plugin_names) end end diff --git a/spec/models/plugins/plugins_spec.rb b/spec/models/plugins/plugins_spec.rb index 7e7d328c8..5bde7636c 100644 --- a/spec/models/plugins/plugins_spec.rb +++ b/spec/models/plugins/plugins_spec.rb @@ -10,22 +10,26 @@ describe 'create_for_page' do it 'creates no plugins with a nil plugin_name' do - expect { Plugins.create_for_page(nil, page, 'my-ref') }.to change { Plugins::Thermometer.count }.by 0 + expect { Plugins.create_for_page(nil, page, 'my-ref') }.to change { Plugins::ActionsThermometer.count }.by 0 expect { Plugins.create_for_page(nil, page, 'my-ref') }.to change { Plugins::Petition.count }.by 0 end it 'creates no plugins with a nil page' do - expect { Plugins.create_for_page('thermometer', nil, 'my-ref') }.to change { Plugins::Thermometer.count }.by 0 + expect { Plugins.create_for_page('actions_thermometer', nil, 'my-ref') } + .to change { Plugins::ActionsThermometer.count }.by 0 expect { Plugins.create_for_page('petition', nil, 'my-ref') }.to change { Plugins::Petition.count }.by 0 end it 'create no plugin if one already exists for that page and ref' do - expect { Plugins.create_for_page('thermometer', page, 'my-ref') }.to change { Plugins::Thermometer.count }.by 1 - expect { Plugins.create_for_page('thermometer', page, 'my-ref') }.to change { Plugins::Thermometer.count }.by 0 + expect { Plugins.create_for_page('actions_thermometer', page, 'my-ref') } + .to change { Plugins::ActionsThermometer.count }.by 1 + expect { Plugins.create_for_page('actions_thermometer', page, 'my-ref') } + .to change { Plugins::ActionsThermometer.count }.by 0 end - it 'creates a thermometer plugin' do - expect { Plugins.create_for_page('thermometer', page, nil) }.to change { Plugins::Thermometer.count }.by 1 + it 'creates an actions thermometer plugin' do + expect { Plugins.create_for_page('actions_thermometer', page, nil) } + .to change { Plugins::ActionsThermometer.count }.by 1 end it 'creates an petition plugin' do @@ -58,11 +62,11 @@ expect(Plugins::Fundraiser.last.title).to eq 'Spenden Sie jetzt!' end - it 'works for thermometers' do - Plugins.create_for_page('thermometer', page, nil) - thermometer = Plugins::Thermometer.last - expect(thermometer.offset).to eq 0 - expect(thermometer.goal).to eq 100 + it 'works for actions thermometers' do + Plugins.create_for_page('actions_thermometer', page, nil) + actions_thermometer = Plugins::ActionsThermometer.last + expect(actions_thermometer.offset).to eq 0 + expect(actions_thermometer.goal).to eq 100 end end @@ -81,11 +85,11 @@ expect(Plugins::Fundraiser.last.title).to eq 'Faites un don sécurisé' end - it 'works for thermometers' do - Plugins.create_for_page('thermometer', page, nil) - thermometer = Plugins::Thermometer.last - expect(thermometer.offset).to eq 0 - expect(thermometer.goal).to eq 100 + it 'works for actions thermometers' do + Plugins.create_for_page('actions_thermometer', page, nil) + actions_thermometer = Plugins::ActionsThermometer.last + expect(actions_thermometer.offset).to eq 0 + expect(actions_thermometer.goal).to eq 100 end end @@ -104,11 +108,11 @@ expect(Plugins::Fundraiser.last.title).to eq 'Donate now' end - it 'works for thermometers' do - Plugins.create_for_page('thermometer', page, nil) - thermometer = Plugins::Thermometer.last - expect(thermometer.offset).to eq 0 - expect(thermometer.goal).to eq 100 + it 'works for actions thermometers' do + Plugins.create_for_page('actions_thermometer', page, nil) + actions_thermometer = Plugins::ActionsThermometer.last + expect(actions_thermometer.offset).to eq 0 + expect(actions_thermometer.goal).to eq 100 end end @@ -129,12 +133,12 @@ expect(Plugins::Fundraiser.last.title).to eq 'Donate now' end - it 'works for thermometers' do + it 'works for actions thermometers' do expect(page.language).to be_nil - Plugins.create_for_page('thermometer', page, nil) - thermometer = Plugins::Thermometer.last - expect(thermometer.offset).to eq 0 - expect(thermometer.goal).to eq 100 + Plugins.create_for_page('actions_thermometer', page, nil) + actions_thermometer = Plugins::ActionsThermometer.last + expect(actions_thermometer.offset).to eq 0 + expect(actions_thermometer.goal).to eq 100 end end end @@ -143,19 +147,19 @@ describe 'data_for_view' do describe 'with plugins' do before :each do - Plugins.create_for_page('thermometer', page, 'stripey') - Plugins.create_for_page('petition', page, 'swiper') - @thermometer = Plugins::Thermometer.last + Plugins.create_for_page('actions_thermometer', page, 'stripey') + Plugins.create_for_page('petition', page, 'swiper') + @actions_thermometer = Plugins::ActionsThermometer.last @petition = Plugins::Petition.last - expect(page.plugins).to match_array [@thermometer, @petition] + expect(page.plugins).to match_array [@actions_thermometer, @petition] end it 'namespaces all the plugins according to their ref' do data_for_view = Plugins.data_for_view(page) expect(data_for_view[:plugins]['petition']).to have_key 'swiper' expect(data_for_view[:plugins]['petition']['swiper']).to eq @petition.liquid_data - expect(data_for_view[:plugins]['thermometer']).to have_key 'stripey' - expect(data_for_view[:plugins]['thermometer']['stripey']).to eq @thermometer.liquid_data + expect(data_for_view[:plugins]['actions_thermometer']).to have_key 'stripey' + expect(data_for_view[:plugins]['actions_thermometer']['stripey']).to eq @actions_thermometer.liquid_data end it 'namespaces refless plugins under the default ref returned in top level hash' do diff --git a/spec/models/plugins/thermometer_spec.rb b/spec/models/plugins/thermometer_spec.rb index 017a9a526..e9e9fa10a 100644 --- a/spec/models/plugins/thermometer_spec.rb +++ b/spec/models/plugins/thermometer_spec.rb @@ -5,6 +5,7 @@ # Table name: plugins_thermometers # # id :integer not null, primary key +# type :string not null # title :string # offset :integer # page_id :integer @@ -16,11 +17,11 @@ require 'rails_helper' -describe Plugins::Thermometer do +describe Plugins::ActionsThermometer do let(:starting_action_count) { 37 } let(:liquid_layout) { LiquidLayout.create! title: 'test', content: 'test' } let(:page) { Page.create! title: 'test page', action_count: starting_action_count, liquid_layout: liquid_layout } - let(:thermometer) { Plugins::Thermometer.create! page: page } + let(:thermometer) { Plugins::ActionsThermometer.create! page: page } it 'can accept random supplemental data to liquid_data method' do expect { thermometer.liquid_data(foo: 'bar') }.not_to raise_error @@ -99,7 +100,7 @@ describe '#current_total' do context "given the page doesn't belong to a campaign" do let!(:page) { create(:page, action_count: 12) } - let!(:thermometer) { Plugins::Thermometer.create! offset: 10, page: page } + let!(:thermometer) { Plugins::ActionsThermometer.create! offset: 10, page: page } it 'it returns the page action_count + offset' do expect(thermometer.current_total).to eq 22 @@ -109,7 +110,7 @@ context 'given the page belongs to a campaign' do let(:campaign) { create(:campaign) } let!(:page) { create(:page, campaign: campaign, action_count: 20) } - let!(:thermometer) { Plugins::Thermometer.create! offset: 10, page: page } + let!(:thermometer) { Plugins::ActionsThermometer.create! offset: 10, page: page } before { create(:page, campaign: campaign, action_count: 30) } it 'consolidates the action counter of all pages of the campaign' do diff --git a/spec/services/page_builder_spec.rb b/spec/services/page_builder_spec.rb index 43f6d7dfd..b75935f0d 100644 --- a/spec/services/page_builder_spec.rb +++ b/spec/services/page_builder_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require 'rails_helper' describe PageBuilder do @@ -34,7 +35,7 @@ expect(Page.last.liquid_layout_id).to eq template.id end - [Plugins::Thermometer, Plugins::Petition].each do |plugin| + [Plugins::ActionsThermometer, Plugins::Petition].each do |plugin| it "creates a #{plugin.name}" do expect { subject }.to change { plugin.count }.by 1 end diff --git a/spec/services/page_cloner_spec.rb b/spec/services/page_cloner_spec.rb index 7a642c699..b4f0396c9 100644 --- a/spec/services/page_cloner_spec.rb +++ b/spec/services/page_cloner_spec.rb @@ -6,7 +6,9 @@ let!(:tag) { create(:tag) } let(:campaign) { create(:campaign) } let!(:petition_partial) { create :liquid_partial, title: 'petition', content: '{{ plugins.petition[ref] }}' } - let!(:thermo_partial) { create :liquid_partial, title: 'thermometer', content: '{{ plugins.thermometer[ref] }}' } + let!(:actions_thermo_partial) do + create :liquid_partial, title: 'thermometer', content: '{{ plugins.actions_thermometer[ref] }}' + end let(:liquid_layout) { create(:liquid_layout, :default) } let(:page) do create( @@ -212,7 +214,7 @@ def get_plugin(type) it 'has the plugins indicated by the liquid layout before the clone' do expect(page.plugins.count).to eq 2 - expect(page.plugins.map(&:class)).to match_array([Plugins::Petition, Plugins::Thermometer]) + expect(page.plugins.map(&:class)).to match_array([Plugins::Petition, Plugins::ActionsThermometer]) end it 'clones plugins' do @@ -225,8 +227,8 @@ def get_plugin(type) expect(cloned).not_to eq(original) end - it 'clones thermometer' do - original, cloned = get_plugin(Plugins::Thermometer) + it 'clones actions thermometer' do + original, cloned = get_plugin(Plugins::ActionsThermometer) expect(cloned).not_to eq(original) end diff --git a/spec/services/page_updater_spec.rb b/spec/services/page_updater_spec.rb index 85f683ab8..74c508030 100644 --- a/spec/services/page_updater_spec.rb +++ b/spec/services/page_updater_spec.rb @@ -7,13 +7,15 @@ # that will actually make or break the campaigner experience let!(:petition_partial) { create :liquid_partial, title: 'petition', content: '{{ plugins.petition[ref].lol }}' } - let!(:thermo_partial) { create :liquid_partial, title: 'thermometer', content: '{{ plugins.thermometer[ref].lol }}' } + let!(:thermo_partial) do + create :liquid_partial, title: 'thermometer', content: '{{ plugins.actions_thermometer[ref].lol }}' + end let(:liquid_layout) { create :liquid_layout, :default } let(:page) { create :page, liquid_layout: liquid_layout } let(:url) { 'sumofus.org/my-path' } let(:simple_changes) { { page: { title: 'howdy folks!', content: 'Did they get you to trade' } } } let(:breaking_changes) { { page: { title: nil, content: 'your heros for ghosts' } } } - let(:thermo_plugin) { page.plugins.select { |p| p.name == 'Thermometer' }.first } + let(:thermo_plugin) { page.plugins.select { |p| p.name == 'ActionsThermometer' }.first } let(:petition_plugin) { page.plugins.select { |p| p.name == 'Petition' }.first } subject(:pupdater) { PageUpdater.new(page, url) } @@ -110,7 +112,7 @@ id: petition_plugin.id, name: petition_plugin.name }, - plugins_thermometer: { + plugins_actions_thermometer: { offset: 1612, id: thermo_plugin.id, name: thermo_plugin.name @@ -136,7 +138,7 @@ it "updates the plugins even if it can't update the page" do params = { - plugins_thermometer: { offset: 1492, id: thermo_plugin.id, name: thermo_plugin.name }, + plugins_actions_thermometer: { offset: 1492, id: thermo_plugin.id, name: thermo_plugin.name }, page: { title: nil, content: 'hot air for a cool breeze' } } expect(pupdater.update(params)).to eq false @@ -146,7 +148,7 @@ it "updates the page even if it can't update the plugins" do params = { - plugins_thermometer: { offset: -100, id: thermo_plugin.id, name: thermo_plugin.name }, + plugins_actions_thermometer: { offset: -100, id: thermo_plugin.id, name: thermo_plugin.name }, page: { content: 'cold comfort for change' } } expect(pupdater.update(params)).to eq false @@ -155,12 +157,12 @@ end it 'raises ActiveRecord::RecordNotFound if missing plugin name' do - params = { plugins_thermometer: { offset: 100, id: thermo_plugin.id } } + params = { plugins_actions_thermometer: { offset: 100, id: thermo_plugin.id } } expect { pupdater.update(params) }.to raise_error ActiveRecord::RecordNotFound end it 'raises ActiveRecord::RecordNotFound if missing plugin id' do - params = { plugins_thermometer: { offset: 100, name: thermo_plugin.name } } + params = { plugins_actions_thermometer: { offset: 100, name: thermo_plugin.name } } expect { pupdater.update(params) }.to raise_error ActiveRecord::RecordNotFound end @@ -253,7 +255,7 @@ describe 'errors' do it 'returns errors nested by page' do params = { - plugins_thermometer: { + plugins_actions_thermometer: { offset: 1492, id: thermo_plugin.id, name: thermo_plugin.name @@ -269,7 +271,7 @@ it 'returns errors nested by plugin' do params = { - plugins_thermometer: { + plugins_actions_thermometer: { offset: -149, id: thermo_plugin.id, name: thermo_plugin.name @@ -280,7 +282,7 @@ } } expect(pupdater.update(params)).to eq false - expect(pupdater.errors).to eq(plugins_thermometer: { offset: 'must be greater than or equal to 0' }) + expect(pupdater.errors).to eq(plugins_actions_thermometer: { offset: 'must be greater than or equal to 0' }) end end diff --git a/spec/services/search/page_searcher/single_criterion_search/search_by_plugin_spec.rb b/spec/services/search/page_searcher/single_criterion_search/search_by_plugin_spec.rb index f30160e60..b7f1b3237 100644 --- a/spec/services/search/page_searcher/single_criterion_search/search_by_plugin_spec.rb +++ b/spec/services/search/page_searcher/single_criterion_search/search_by_plugin_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require 'rails_helper' describe 'Search ::' do @@ -58,20 +59,22 @@ describe 'returns no pages when searching' do it 'with a plugin has been turned off on all of the pages' do - expect(Search::PageSearcher.new(plugin_type: ['Plugins::Thermometer']).search).to match_array([]) + expect(Search::PageSearcher.new(plugin_type: ['Plugins::ActionsThermometer']).search).to match_array([]) end it 'with a plugin that does not exist' do expect(Search::PageSearcher.new(plugin_type: ['Plugins::UnusedPlugin']).search).to match_array([]) end it 'with several plugins where a page matches one but not the rest of them' do - search = Search::PageSearcher.new(plugin_type: ['Plugins::Thermometer', 'Plugins::UnusedPlugin']).search + search = Search::PageSearcher.new( + plugin_type: ['Plugins::ActionsThermometer', 'Plugins::UnusedPlugin'] + ).search expect(search).to match_array([]) end it 'with several plugins where at least one page matches by criteria but at least one of the' \ 'requested plugins is deactivated' do default_page_thermometer.update(active: false) - search = Search::PageSearcher.new(plugin_type: ['Plugins::Petition', 'Plugins::Thermometer']).search + search = Search::PageSearcher.new(plugin_type: ['Plugins::Petition', 'Plugins::ActionsThermometer']).search expect(search).to match_array([]) end end @@ -86,7 +89,7 @@ expect(default_page_thermometer.active).to eq(true) expect(thermometer_page_thermometer.active).to eq(true) expect(thermometer_page_thermometer.page).to eq(thermometer_page) - expect(Search::PageSearcher.new(plugin_type: ['Plugins::Thermometer']).search).to( + expect(Search::PageSearcher.new(plugin_type: ['Plugins::ActionsThermometer']).search).to( match_array([default_page, thermometer_page]) ) end @@ -101,9 +104,9 @@ petition_page_thermometer.update(active: false) thermometer_page_thermometer.update(active: true) - expect(Search::PageSearcher.new(plugin_type: ['Plugins::Petition', 'Plugins::Thermometer']).search).to( - match_array([default_page]) - ) + expect( + Search::PageSearcher.new(plugin_type: ['Plugins::Petition', 'Plugins::ActionsThermometer']).search + ).to(match_array([default_page])) end end end From 3125265a8d0e5acd336da5c0deb89ddfeff15e12 Mon Sep 17 00:00:00 2001 From: tuuli Date: Tue, 20 Nov 2018 11:09:46 +0100 Subject: [PATCH 02/18] revert back to original thermometer form partial --- app/controllers/plugins_controller.rb | 2 ++ app/models/plugins.rb | 1 + app/views/pages/edit.slim | 4 ++-- .../_actions_thermometer.liquid | 19 ------------------- app/views/plugins/show.html.slim | 2 +- .../_form.slim | 2 +- .../plugins/thermometers/_thermometer.liquid | 11 +++++++---- 7 files changed, 14 insertions(+), 27 deletions(-) delete mode 100644 app/views/plugins/actions_thermometers/_actions_thermometer.liquid rename app/views/plugins/{actions_thermometers => thermometers}/_form.slim (92%) diff --git a/app/controllers/plugins_controller.rb b/app/controllers/plugins_controller.rb index 775d2ac8e..a699e6c15 100644 --- a/app/controllers/plugins_controller.rb +++ b/app/controllers/plugins_controller.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + class PluginsController < ApplicationController before_action :find_page @@ -12,6 +13,7 @@ def index end def show + pp 'PARAMS:', params @plugin = Plugins.find_for(params[:type], params[:id]) end diff --git a/app/models/plugins.rb b/app/models/plugins.rb index 6df519e4b..3e54a52e2 100644 --- a/app/models/plugins.rb +++ b/app/models/plugins.rb @@ -60,6 +60,7 @@ def names end def find_for(plugin_class, plugin_id) + pp 'PLUGIN CLASS:', plugin_class.camelize Plugins.const_get(plugin_class.camelize).find(plugin_id) end end diff --git a/app/views/pages/edit.slim b/app/views/pages/edit.slim index ed51b268c..0e7d8c61e 100644 --- a/app/views/pages/edit.slim +++ b/app/views/pages/edit.slim @@ -15,7 +15,7 @@ section.page-edit-step#layout data-icon='cog' - next if low_priority_plugins.include? plugin.class section.page-edit-step id=plugin_section_id(plugin) data-icon=plugin_icon(plugin) h1.page-edit-step__title= plugin_title(plugin) - = render "#{plugin.class.name.underscore.pluralize}/form", plugin: plugin, page: @page + = render "#{plugin.class.base_class.name.underscore.pluralize}/form", plugin: plugin, page: @page section.page-edit-step#pictures data-icon='camera-retro' h1.page-edit-step__title = t('.pictures') @@ -33,7 +33,7 @@ section.page-edit-step#sources data-icon='link' - next unless low_priority_plugins.include? plugin.class section.page-edit-step id=plugin_section_id(plugin) data-icon=plugin_icon(plugin) h1.page-edit-step__title= plugin_title(plugin) - = render "#{plugin.class.name.underscore.pluralize}/form", plugin: plugin, page: @page + = render "#{plugin.class.base_class.name.underscore.pluralize}/form", plugin: plugin, page: @page section.page-edit-step.page-edit-step--just-title#review data-icon='eye' data-link-to=member_facing_page_url(@page) h1.page-edit-step__title diff --git a/app/views/plugins/actions_thermometers/_actions_thermometer.liquid b/app/views/plugins/actions_thermometers/_actions_thermometer.liquid deleted file mode 100644 index e2cfd4152..000000000 --- a/app/views/plugins/actions_thermometers/_actions_thermometer.liquid +++ /dev/null @@ -1,19 +0,0 @@ -{% if plugins.actions_thermometer[ref].active %} - -
-
-
- {{ plugins.actions_thermometer[ref].signatures }} {{ 'actions_thermometer.signatures' | t }} -
-
- {{ 'actions_thermometer.signatures_until_goal' | - val: 'goal', plugins.actions_thermometer[ref].goal_k | - val: 'remaining', plugins.actions_thermometer[ref].remaining | t }} -
-
-
-
-
-
-{% endif %} diff --git a/app/views/plugins/show.html.slim b/app/views/plugins/show.html.slim index b00c2e49c..1c4e4c44f 100644 --- a/app/views/plugins/show.html.slim +++ b/app/views/plugins/show.html.slim @@ -8,6 +8,6 @@ = page_nav_item(plugin_title(plugin), path) .col-md-10 - = render partial: "#{@plugin.class.name.underscore.pluralize}/form", locals: { plugin: @plugin, page: @page } + = render partial: "#{@plugin.class.base_class.name.underscore.pluralize}/form", locals: { plugin: @plugin, page: @page } diff --git a/app/views/plugins/actions_thermometers/_form.slim b/app/views/plugins/thermometers/_form.slim similarity index 92% rename from app/views/plugins/actions_thermometers/_form.slim rename to app/views/plugins/thermometers/_form.slim index 9cdadb41e..8c7e090fb 100644 --- a/app/views/plugins/actions_thermometers/_form.slim +++ b/app/views/plugins/thermometers/_form.slim @@ -1,7 +1,7 @@ .col-md-7 = render partial: 'plugins/shared/toggle_form', locals: { plugin: plugin, path: plugins_thermometer_path(plugin) } - - name = "plugins_thermometer_#{plugin.id}" + - name = "plugins_actions_thermometer_#{plugin.id}" = form_for plugin, url: plugins_thermometer_path(plugin), remote: true, as: name, html: {class: 'plugin-settings one-form', data: {type: name }} do |f| = render "shared/error_messages", target: plugin = render "shared/success_message", success: (defined?(success) || false) diff --git a/app/views/plugins/thermometers/_thermometer.liquid b/app/views/plugins/thermometers/_thermometer.liquid index 6533ace32..66adcc77c 100644 --- a/app/views/plugins/thermometers/_thermometer.liquid +++ b/app/views/plugins/thermometers/_thermometer.liquid @@ -1,15 +1,18 @@ -{% if plugins.thermometer[ref].active %} + +{% if plugins.actions_thermometer[ref].active %}
- {{ plugins.thermometer[ref].signatures }} {{ 'thermometer.signatures' | t }} + {{ plugins.actions_thermometer[ref].signatures }} {{ 'thermometer.signatures' | t }}
{{ 'thermometer.signatures_until_goal' | - val: 'goal', plugins.thermometer[ref].goal_k | - val: 'remaining', plugins.thermometer[ref].remaining | t }} + val: 'goal', plugins.actions_thermometer[ref].goal_k | + val: 'remaining', plugins.actions_thermometer[ref].remaining | t }}
From 6942e27527e5af6b3afddcae73392bb93b5ac9ae Mon Sep 17 00:00:00 2001 From: tuuli Date: Wed, 21 Nov 2018 12:25:37 +0100 Subject: [PATCH 03/18] WIP commit for Omar --- .../plugins/thermometers_controller.rb | 1 + app/controllers/plugins_controller.rb | 1 - app/liquid/liquid_renderer.rb | 9 ++- app/models/plugins/actions_thermometer.rb | 22 +------ app/models/plugins/donations_thermometer.rb | 61 ++++++------------- app/models/plugins/thermometer.rb | 44 +------------ .../_form.slim} | 0 app/views/plugins/thermometers/_form.slim | 2 +- .../plugins/thermometers/_thermometer.liquid | 7 +-- spec/liquid/liquid_renderer_spec.rb | 36 +++++++++++ ...er_spec.rb => actions_thermometer_spec.rb} | 0 11 files changed, 68 insertions(+), 115 deletions(-) rename app/views/plugins/{donations_thermometers/_form.liquid => donations_thermometer/_form.slim} (100%) rename spec/models/plugins/{thermometer_spec.rb => actions_thermometer_spec.rb} (100%) diff --git a/app/controllers/plugins/thermometers_controller.rb b/app/controllers/plugins/thermometers_controller.rb index 8b119850f..b99ef85fc 100644 --- a/app/controllers/plugins/thermometers_controller.rb +++ b/app/controllers/plugins/thermometers_controller.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +# This controller is only used for ActionsThermometers. class Plugins::ThermometersController < Plugins::BaseController private diff --git a/app/controllers/plugins_controller.rb b/app/controllers/plugins_controller.rb index a699e6c15..9a0e3699c 100644 --- a/app/controllers/plugins_controller.rb +++ b/app/controllers/plugins_controller.rb @@ -13,7 +13,6 @@ def index end def show - pp 'PARAMS:', params @plugin = Plugins.find_for(params[:type], params[:id]) end diff --git a/app/liquid/liquid_renderer.rb b/app/liquid/liquid_renderer.rb index 057de4e98..ff590339f 100644 --- a/app/liquid/liquid_renderer.rb +++ b/app/liquid/liquid_renderer.rb @@ -43,6 +43,7 @@ def personalization_data outstanding_fields: outstanding_fields, donation_bands: donation_bands, actions_thermometer: actions_thermometer, + donations_thermometer: donations_thermometer, call_tool: call_tool_data, email_tool: email_tool_data, email_pension: email_pension_data, @@ -118,8 +119,12 @@ def donation_bands end def actions_thermometer - data = plugin_data.deep_symbolize_keys[:plugins][:actions_thermometer].try(:values).try(:first) - pp 'Actions thermometer data:', data + plugin_data.deep_symbolize_keys[:plugins][:actions_thermometer].try(:values).try(:first) + end + + def donations_thermometer + data = plugin_data.deep_symbolize_keys[:plugins][:donations_thermometer].try(:values).try(:first) + pp 'Donations thermometer data: ', data data end diff --git a/app/models/plugins/actions_thermometer.rb b/app/models/plugins/actions_thermometer.rb index d87cc2e6d..6983e5275 100644 --- a/app/models/plugins/actions_thermometer.rb +++ b/app/models/plugins/actions_thermometer.rb @@ -18,17 +18,12 @@ class Plugins::ActionsThermometer < Plugins::Thermometer belongs_to :page, touch: true - DEFAULTS = { offset: 0 }.freeze GOALS = [100, 200, 300, 400, 500, 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, 10_000, 15_000, 20_000, 25_000, 50_000, 75_000, 100_000, 150_000, 200_000, 250_000, 300_000, 500_000, 750_000, 1_000_000, 1_500_000, 2_000_000].freeze - validates :offset, presence: true, - numericality: { greater_than_or_equal_to: 0 } - after_initialize :set_defaults - def current_total offset + action_count end @@ -50,33 +45,18 @@ def liquid_data(_supplemental_data = {}) ) end - def name - self.class.name.demodulize - end - private def action_count @action_count ||= if page.campaign_id.present? - Page.where(campaign_id: page.campaign_id).sum(:action_count) + page.campaign.action_count else page.action_count end end - def abbreviate_number(number) - return number.to_s if number < 1000 - return "#{(goal / 1000).to_i}k" if number < 1_000_000 - locale = page.try(:language).try(:code) - "%g #{I18n.t('thermometer.million', locale: locale)}" % (goal / 1_000_000.0).round(1) - end - def next_goal_as_multiple_of(step) remainder = current_total % step current_total + step - remainder end - - def set_defaults - self.offset ||= DEFAULTS[:offset] - end end diff --git a/app/models/plugins/donations_thermometer.rb b/app/models/plugins/donations_thermometer.rb index ea8e7c144..8ce140cf7 100644 --- a/app/models/plugins/donations_thermometer.rb +++ b/app/models/plugins/donations_thermometer.rb @@ -18,65 +18,38 @@ class Plugins::DonationsThermometer < Plugins::Thermometer belongs_to :page, touch: true - DEFAULTS = { offset: 0 }.freeze - GOALS = [100, 200, 300, 400, 500, - 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, - 10_000, 15_000, 20_000, 25_000, 50_000, 75_000, 100_000, - 150_000, 200_000, 250_000, 300_000, 500_000, 750_000, - 1_000_000, 1_500_000, 2_000_000].freeze - - validates :offset, presence: true, - numericality: { greater_than_or_equal_to: 0 } - after_initialize :set_defaults - def current_total - offset + action_count + Money.from_amount(offset, Settings.default_currency) + total_donations end def current_progress - current_total / goal.to_f * 100 - end - - def goal - GOALS.find { |goal| current_total < goal } || next_goal_as_multiple_of(1_000_000) + total_donations / fundraising_goal * 100 end def liquid_data(_supplemental_data = {}) attributes.merge( percentage: current_progress, - remaining: ActionController::Base.helpers.number_with_delimiter(goal - current_total), - signatures: ActionController::Base.helpers.number_with_delimiter(current_total), - goal_k: abbreviate_number(goal) + remaining: ActionController::Base.helpers.number_with_delimiter(fundraising_goal - current_total), + total_donations: ActionController::Base.helpers.number_with_delimiter(current_total), + goal_k: abbreviate_number(fundraising_goal) ) end - def name - self.class.name.demodulize - end - private - def action_count - @action_count ||= if page.campaign_id.present? - Page.where(campaign_id: page.campaign_id).sum(:action_count) - else - page.action_count - end - end - - def abbreviate_number(number) - return number.to_s if number < 1000 - return "#{(goal / 1000).to_i}k" if number < 1_000_000 - locale = page.try(:language).try(:code) - "%g #{I18n.t('thermometer.million', locale: locale)}" % (goal / 1_000_000.0).round(1) - end - - def next_goal_as_multiple_of(step) - remainder = current_total % step - current_total + step - remainder + def fundraising_goal + if page.campaign.blank? + page.fundraising_goal + else + page.campaign.fundraising_goal + end end - def set_defaults - self.offset ||= DEFAULTS[:offset] + def total_donations + if page.campaign.blank? + page.total_donations + else + page.campaign.total_donations + end end end diff --git a/app/models/plugins/thermometer.rb b/app/models/plugins/thermometer.rb index 1c99a4edc..fa7570d7a 100644 --- a/app/models/plugins/thermometer.rb +++ b/app/models/plugins/thermometer.rb @@ -23,49 +23,18 @@ def self.store_full_sti_class belongs_to :page, touch: true DEFAULTS = { offset: 0 }.freeze - GOALS = [100, 200, 300, 400, 500, - 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, - 10_000, 15_000, 20_000, 25_000, 50_000, 75_000, 100_000, - 150_000, 200_000, 250_000, 300_000, 500_000, 750_000, - 1_000_000, 1_500_000, 2_000_000].freeze - validates :offset, presence: true, numericality: { greater_than_or_equal_to: 0 } after_initialize :set_defaults - def current_total - offset + action_count - end - - def current_progress - current_total / goal.to_f * 100 - end - - def goal - GOALS.find { |goal| current_total < goal } || next_goal_as_multiple_of(1_000_000) - end - - def liquid_data(_supplemental_data = {}) - attributes.merge( - percentage: current_progress, - remaining: ActionController::Base.helpers.number_with_delimiter(goal - current_total), - signatures: ActionController::Base.helpers.number_with_delimiter(current_total), - goal_k: abbreviate_number(goal) - ) - end - def name self.class.name.demodulize end private - def action_count - @action_count ||= if page.campaign_id.present? - Page.where(campaign_id: page.campaign_id).sum(:action_count) - else - page.action_count - end + def set_defaults + self.offset ||= DEFAULTS[:offset] end def abbreviate_number(number) @@ -74,13 +43,4 @@ def abbreviate_number(number) locale = page.try(:language).try(:code) "%g #{I18n.t('thermometer.million', locale: locale)}" % (goal / 1_000_000.0).round(1) end - - def next_goal_as_multiple_of(step) - remainder = current_total % step - current_total + step - remainder - end - - def set_defaults - self.offset ||= DEFAULTS[:offset] - end end diff --git a/app/views/plugins/donations_thermometers/_form.liquid b/app/views/plugins/donations_thermometer/_form.slim similarity index 100% rename from app/views/plugins/donations_thermometers/_form.liquid rename to app/views/plugins/donations_thermometer/_form.slim diff --git a/app/views/plugins/thermometers/_form.slim b/app/views/plugins/thermometers/_form.slim index 8c7e090fb..9cdadb41e 100644 --- a/app/views/plugins/thermometers/_form.slim +++ b/app/views/plugins/thermometers/_form.slim @@ -1,7 +1,7 @@ .col-md-7 = render partial: 'plugins/shared/toggle_form', locals: { plugin: plugin, path: plugins_thermometer_path(plugin) } - - name = "plugins_actions_thermometer_#{plugin.id}" + - name = "plugins_thermometer_#{plugin.id}" = form_for plugin, url: plugins_thermometer_path(plugin), remote: true, as: name, html: {class: 'plugin-settings one-form', data: {type: name }} do |f| = render "shared/error_messages", target: plugin = render "shared/success_message", success: (defined?(success) || false) diff --git a/app/views/plugins/thermometers/_thermometer.liquid b/app/views/plugins/thermometers/_thermometer.liquid index 66adcc77c..47b22b552 100644 --- a/app/views/plugins/thermometers/_thermometer.liquid +++ b/app/views/plugins/thermometers/_thermometer.liquid @@ -1,9 +1,8 @@ + continued to work. This template is not used by DonationsThermometer. This partial will be cached, so the + javascript replaces the values for signature, remaining, and thermometer width --> {% if plugins.actions_thermometer[ref].active %} -
@@ -16,7 +15,7 @@
-
+
{% endif %} diff --git a/spec/liquid/liquid_renderer_spec.rb b/spec/liquid/liquid_renderer_spec.rb index a52274329..6d9c434a9 100644 --- a/spec/liquid/liquid_renderer_spec.rb +++ b/spec/liquid/liquid_renderer_spec.rb @@ -395,6 +395,42 @@ end end + describe 'donations_thermometer' do + it 'is nil if no donations thermometer plugin' do + create :plugins_fundraiser, :donations_thermometer, page: page + expect(page.plugins.size).to eq 1 + expect(LiquidRenderer.new(page).personalization_data['donations_thermometer']).to eq nil + end + + it "is serializes the actions thermometer plugin's data" do + t1 = create :plugins_thermometer, :donations_thermometer, page: page + t1.current_progress # allow goal to update + expected = t1.liquid_data.stringify_keys + actual = LiquidRenderer.new(page).personalization_data['donations_thermometer'] + # disagreement over timestamps is not what this test is about + [expected, actual].each do |h| + h.delete('updated_at') + h.delete('created_at') + end + expect(actual).to eq expected + end + + it 'is uses the first if multiple actions thermometer plugins' do + t1 = create :plugins_thermometer, :donations_thermometer, page: page, ref: 'secondary' + create :plugins_thermometer, :donations_thermometer, page: page + expect(page.plugins.size).to eq 2 + t1.current_progress # allow goal to update + expected = t1.liquid_data.stringify_keys + actual = LiquidRenderer.new(page).personalization_data['donations_thermometer'] + # disagreement over timestamps is not what this test is about + [expected, actual].each do |h| + h.delete('updated_at') + h.delete('created_at') + end + expect(actual).to eq expected + end + end + describe 'action_count' do it 'serializes page.action_count' do page.action_count = 1337 diff --git a/spec/models/plugins/thermometer_spec.rb b/spec/models/plugins/actions_thermometer_spec.rb similarity index 100% rename from spec/models/plugins/thermometer_spec.rb rename to spec/models/plugins/actions_thermometer_spec.rb From 62880b24dd0a66543493958329a41213b4ea1088 Mon Sep 17 00:00:00 2001 From: tuuli Date: Wed, 21 Nov 2018 15:52:01 +0100 Subject: [PATCH 04/18] Populate champaign.personalization with donations thermometer stuff --- app/liquid/liquid_renderer.rb | 4 +-- app/models/plugins.rb | 1 - app/models/plugins/donations_thermometer.rb | 33 +++++++++---------- app/models/plugins/fundraiser.rb | 5 +++ app/views/pages/_personalization.slim | 1 + .../factories/plugins_actions_thermometers.rb | 25 -------------- spec/factories/plugins_thermometers.rb | 9 +++-- spec/liquid/liquid_renderer_spec.rb | 22 ++++++------- spec/models/plugins/fundraiser_spec.rb | 4 +++ 9 files changed, 43 insertions(+), 61 deletions(-) delete mode 100644 spec/factories/plugins_actions_thermometers.rb diff --git a/app/liquid/liquid_renderer.rb b/app/liquid/liquid_renderer.rb index ff590339f..5d19f2963 100644 --- a/app/liquid/liquid_renderer.rb +++ b/app/liquid/liquid_renderer.rb @@ -123,9 +123,7 @@ def actions_thermometer end def donations_thermometer - data = plugin_data.deep_symbolize_keys[:plugins][:donations_thermometer].try(:values).try(:first) - pp 'Donations thermometer data: ', data - data + plugin_data.deep_symbolize_keys[:plugins][:donations_thermometer].try(:values).try(:first) end def call_tool_data diff --git a/app/models/plugins.rb b/app/models/plugins.rb index 3e54a52e2..6df519e4b 100644 --- a/app/models/plugins.rb +++ b/app/models/plugins.rb @@ -60,7 +60,6 @@ def names end def find_for(plugin_class, plugin_id) - pp 'PLUGIN CLASS:', plugin_class.camelize Plugins.const_get(plugin_class.camelize).find(plugin_id) end end diff --git a/app/models/plugins/donations_thermometer.rb b/app/models/plugins/donations_thermometer.rb index 8ce140cf7..f97711669 100644 --- a/app/models/plugins/donations_thermometer.rb +++ b/app/models/plugins/donations_thermometer.rb @@ -18,38 +18,37 @@ class Plugins::DonationsThermometer < Plugins::Thermometer belongs_to :page, touch: true - def current_total - Money.from_amount(offset, Settings.default_currency) + total_donations - end - def current_progress + return 0 if fundraising_goal.zero? total_donations / fundraising_goal * 100 end def liquid_data(_supplemental_data = {}) attributes.merge( percentage: current_progress, - remaining: ActionController::Base.helpers.number_with_delimiter(fundraising_goal - current_total), - total_donations: ActionController::Base.helpers.number_with_delimiter(current_total), - goal_k: abbreviate_number(fundraising_goal) + remaining_amounts: currencies_hash(fundraising_goal - current_total), + total_donations: currencies_hash(current_total), + goals: currencies_hash(fundraising_goal) ) end private + def currencies_hash(amount) + # Get a hash with amount converted into all supported currencies. + # Transform values from arrays of amounts to single amounts (e.g. GBP: [10] to GBP: 10) + Donations::Currencies.for([amount]).to_hash.with_indifferent_access.transform_values(&:pop) + end + def fundraising_goal - if page.campaign.blank? - page.fundraising_goal - else - page.campaign.fundraising_goal - end + page.campaign.blank? ? page.fundraising_goal : page.campaign.fundraising_goal end def total_donations - if page.campaign.blank? - page.total_donations - else - page.campaign.total_donations - end + page.campaign.blank? ? page.total_donations : page.campaign.total_donations + end + + def current_total + offset + total_donations end end diff --git a/app/models/plugins/fundraiser.rb b/app/models/plugins/fundraiser.rb index e91585073..cc4a19439 100644 --- a/app/models/plugins/fundraiser.rb +++ b/app/models/plugins/fundraiser.rb @@ -25,6 +25,8 @@ class Plugins::Fundraiser < ApplicationRecord belongs_to :page, touch: true belongs_to :donation_band + after_create :create_donations_thermometer + DEFAULTS = { title: 'fundraiser.donate_now' }.freeze def liquid_data(supplemental_data = {}) @@ -46,4 +48,7 @@ def self.donation_default_for_page(page_id) plugin = Plugins::Fundraiser.find_by(page_id: page_id) plugin ? plugin.recurring? : false end + + def create_donations_thermometer + end end diff --git a/app/views/pages/_personalization.slim b/app/views/pages/_personalization.slim index 682c3cfaa..ee9387800 100644 --- a/app/views/pages/_personalization.slim +++ b/app/views/pages/_personalization.slim @@ -6,6 +6,7 @@ javascript: donationBands: #{serialize(data, 'donation_bands')}, actionCount: #{ serialize(data, 'action_count') }, actionsThermometer: #{ serialize(data, 'actions_thermometer') }, + donationsThermometer: #{ serialize(data, 'donations_thermometer') }, formValues: #{serialize(data, 'form_values') }, callTool: #{ serialize(data, 'call_tool') }, emailTool: #{ serialize(data, 'email_tool') }, diff --git a/spec/factories/plugins_actions_thermometers.rb b/spec/factories/plugins_actions_thermometers.rb deleted file mode 100644 index 130473b65..000000000 --- a/spec/factories/plugins_actions_thermometers.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -# == Schema Information -# -# Table name: plugins_thermometers -# -# id :integer not null, primary key -# title :string -# offset :integer -# page_id :integer -# active :boolean default("false") -# created_at :datetime not null -# updated_at :datetime not null -# ref :string -# - -FactoryGirl.define do - factory :plugins_actions_thermometer, class: 'Plugins::Thermometer' do - title 'MyString' - type 'ActionsThermometer' - offset 1 - page nil - active false - end -end diff --git a/spec/factories/plugins_thermometers.rb b/spec/factories/plugins_thermometers.rb index c7cffb286..55f34dbff 100644 --- a/spec/factories/plugins_thermometers.rb +++ b/spec/factories/plugins_thermometers.rb @@ -20,9 +20,12 @@ offset { 1 } page { nil } active { false } + end + + factory :plugins_donations_thermometer, parent: :plugins_thermometer, class: 'Plugins::DonationsThermometer' do + type { 'DonationsThermometer' } + end - trait :donations_thermometer do - type { 'DonationsThermometer' } - end + factory :plugins_actions_thermometer, parent: :plugins_thermometer, class: 'Plugins::ActionsThermometer' do end end diff --git a/spec/liquid/liquid_renderer_spec.rb b/spec/liquid/liquid_renderer_spec.rb index 6d9c434a9..445bf7f47 100644 --- a/spec/liquid/liquid_renderer_spec.rb +++ b/spec/liquid/liquid_renderer_spec.rb @@ -163,6 +163,7 @@ member donation_bands actions_thermometer + donations_thermometer action_count payment_methods form_values @@ -383,7 +384,6 @@ t1 = create :plugins_actions_thermometer, page: page, ref: 'secondary' create :plugins_actions_thermometer, page: page expect(page.plugins.size).to eq 2 - t1.current_progress # allow goal to update expected = t1.liquid_data.stringify_keys actual = LiquidRenderer.new(page).personalization_data['actions_thermometer'] # disagreement over timestamps is not what this test is about @@ -391,20 +391,19 @@ h.delete('updated_at') h.delete('created_at') end - expect(actual).to eq expected + expect(actual).to match expected end end describe 'donations_thermometer' do it 'is nil if no donations thermometer plugin' do - create :plugins_fundraiser, :donations_thermometer, page: page + create :plugins_fundraiser, page: page expect(page.plugins.size).to eq 1 expect(LiquidRenderer.new(page).personalization_data['donations_thermometer']).to eq nil end - it "is serializes the actions thermometer plugin's data" do - t1 = create :plugins_thermometer, :donations_thermometer, page: page - t1.current_progress # allow goal to update + it "is serializes the donations thermometer plugin's data" do + t1 = create :plugins_donations_thermometer, page: page expected = t1.liquid_data.stringify_keys actual = LiquidRenderer.new(page).personalization_data['donations_thermometer'] # disagreement over timestamps is not what this test is about @@ -412,14 +411,13 @@ h.delete('updated_at') h.delete('created_at') end - expect(actual).to eq expected + expect(actual).to match expected end - it 'is uses the first if multiple actions thermometer plugins' do - t1 = create :plugins_thermometer, :donations_thermometer, page: page, ref: 'secondary' - create :plugins_thermometer, :donations_thermometer, page: page + it 'is uses the first if multiple donations thermometer plugins' do + t1 = create :plugins_donations_thermometer, page: page, ref: 'secondary' + create :plugins_donations_thermometer, page: page expect(page.plugins.size).to eq 2 - t1.current_progress # allow goal to update expected = t1.liquid_data.stringify_keys actual = LiquidRenderer.new(page).personalization_data['donations_thermometer'] # disagreement over timestamps is not what this test is about @@ -427,7 +425,7 @@ h.delete('updated_at') h.delete('created_at') end - expect(actual).to eq expected + expect(actual).to match expected end end diff --git a/spec/models/plugins/fundraiser_spec.rb b/spec/models/plugins/fundraiser_spec.rb index 47683ac8a..2d4a42534 100644 --- a/spec/models/plugins/fundraiser_spec.rb +++ b/spec/models/plugins/fundraiser_spec.rb @@ -37,6 +37,10 @@ expect(Plugins.registered).to include(Plugins::Fundraiser) end + it 'creates a fundraising thermometer when created' do + expect { create :plugins_fundraiser }.to change { Plugins::DonationsThermometer.count }.from(0).to(1) + end + it 'serializes the currency band' do allow(PaymentProcessor::Currency).to receive(:convert) band = create :donation_band From def0056a28a73111612ceb95c1de7fa58632e31e Mon Sep 17 00:00:00 2001 From: tuuli Date: Wed, 21 Nov 2018 17:21:24 +0100 Subject: [PATCH 05/18] Create donations thermometer when a fundraiser plugin is created --- app/models/plugins/fundraiser.rb | 1 + spec/models/plugins/fundraiser_spec.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/plugins/fundraiser.rb b/app/models/plugins/fundraiser.rb index cc4a19439..2378f2e75 100644 --- a/app/models/plugins/fundraiser.rb +++ b/app/models/plugins/fundraiser.rb @@ -50,5 +50,6 @@ def self.donation_default_for_page(page_id) end def create_donations_thermometer + Plugins::DonationsThermometer.create!(page: page) end end diff --git a/spec/models/plugins/fundraiser_spec.rb b/spec/models/plugins/fundraiser_spec.rb index 2d4a42534..65c4b997f 100644 --- a/spec/models/plugins/fundraiser_spec.rb +++ b/spec/models/plugins/fundraiser_spec.rb @@ -22,6 +22,7 @@ describe Plugins::Fundraiser do let(:fundraiser) { create :plugins_fundraiser } + let(:page) { create :page } subject { fundraiser } @@ -38,7 +39,7 @@ end it 'creates a fundraising thermometer when created' do - expect { create :plugins_fundraiser }.to change { Plugins::DonationsThermometer.count }.from(0).to(1) + expect { Plugins::Fundraiser.create(page: page) }.to change { Plugins::DonationsThermometer.count }.from(0).to(1) end it 'serializes the currency band' do From da91149cf37bccd9b1c25dce285ba5f10d22a212 Mon Sep 17 00:00:00 2001 From: tuuli Date: Thu, 22 Nov 2018 12:14:41 +0100 Subject: [PATCH 06/18] Refactor thermometersController to work with both types --- app/controllers/plugins/base_controller.rb | 2 +- .../plugins/thermometers_controller.rb | 27 +++++++++++++------ app/views/pages/edit.slim | 4 +-- .../plugins/actions_thermometers/_form.slim | 12 +++++++++ .../plugins/donations_thermometer/_form.slim | 0 .../plugins/donations_thermometers/_form.slim | 12 +++++++++ config/locales/champaign.en.yml | 6 ++++- spec/controllers/plugins/shared_examples.rb | 6 ++--- .../plugins/thermometers_controller_spec.rb | 3 ++- spec/liquid/liquid_renderer_spec.rb | 4 +-- spec/models/page_spec.rb | 4 +-- 11 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 app/views/plugins/actions_thermometers/_form.slim delete mode 100644 app/views/plugins/donations_thermometer/_form.slim create mode 100644 app/views/plugins/donations_thermometers/_form.slim diff --git a/app/controllers/plugins/base_controller.rb b/app/controllers/plugins/base_controller.rb index 51ce46d8a..f74359768 100644 --- a/app/controllers/plugins/base_controller.rb +++ b/app/controllers/plugins/base_controller.rb @@ -1,4 +1,4 @@ -# frozen_string_literal: true +# # frozen_string_literal: true class Plugins::BaseController < ApplicationController before_action :authenticate_user! diff --git a/app/controllers/plugins/thermometers_controller.rb b/app/controllers/plugins/thermometers_controller.rb index b99ef85fc..dd649e5ed 100644 --- a/app/controllers/plugins/thermometers_controller.rb +++ b/app/controllers/plugins/thermometers_controller.rb @@ -1,20 +1,31 @@ # frozen_string_literal: true -# This controller is only used for ActionsThermometers. class Plugins::ThermometersController < Plugins::BaseController + def update + @plugin = Plugins::Thermometer.find(params[:id]) + + respond_to do |format| + if @plugin.update(permitted_params) + format.js { render json: {} } + else + format.js { render json: { errors: @plugin.errors, name: plugin_name.to_sym }, status: :unprocessable_entity } + end + end + end + private + def plugin_class + params[:plugins_actions_thermometer].blank? ? Plugins::DonationsThermometer : Plugins::ActionsThermometer + end + def permitted_params params - .require(:plugins_thermometer) + .require(plugin_name) .permit(:title, :offset, :goal, :active, :type) end - def plugin_class - Plugins::ActionsThermometer - end - - def plugin_symbol - :plugins_thermometer + def plugin_name + plugin_class.name.underscore.tr('/', '_') end end diff --git a/app/views/pages/edit.slim b/app/views/pages/edit.slim index 0e7d8c61e..ed51b268c 100644 --- a/app/views/pages/edit.slim +++ b/app/views/pages/edit.slim @@ -15,7 +15,7 @@ section.page-edit-step#layout data-icon='cog' - next if low_priority_plugins.include? plugin.class section.page-edit-step id=plugin_section_id(plugin) data-icon=plugin_icon(plugin) h1.page-edit-step__title= plugin_title(plugin) - = render "#{plugin.class.base_class.name.underscore.pluralize}/form", plugin: plugin, page: @page + = render "#{plugin.class.name.underscore.pluralize}/form", plugin: plugin, page: @page section.page-edit-step#pictures data-icon='camera-retro' h1.page-edit-step__title = t('.pictures') @@ -33,7 +33,7 @@ section.page-edit-step#sources data-icon='link' - next unless low_priority_plugins.include? plugin.class section.page-edit-step id=plugin_section_id(plugin) data-icon=plugin_icon(plugin) h1.page-edit-step__title= plugin_title(plugin) - = render "#{plugin.class.base_class.name.underscore.pluralize}/form", plugin: plugin, page: @page + = render "#{plugin.class.name.underscore.pluralize}/form", plugin: plugin, page: @page section.page-edit-step.page-edit-step--just-title#review data-icon='eye' data-link-to=member_facing_page_url(@page) h1.page-edit-step__title diff --git a/app/views/plugins/actions_thermometers/_form.slim b/app/views/plugins/actions_thermometers/_form.slim new file mode 100644 index 000000000..20cb4eb3a --- /dev/null +++ b/app/views/plugins/actions_thermometers/_form.slim @@ -0,0 +1,12 @@ +.col-md-7 + = render partial: 'plugins/shared/toggle_form', locals: { plugin: plugin, path: plugins_thermometer_path(plugin) } + + - name = "plugins_actions_thermometer_#{plugin.id}" + = form_for plugin, url: plugins_thermometer_path(plugin), remote: true, as: name, html: {class: 'plugin-settings one-form', data: {type: name }} do |f| + = render "shared/error_messages", target: plugin + = render "shared/success_message", success: (defined?(success) || false) + = render 'plugins/shared/plugin_metadata', f: f + + .form-group + = label_with_tooltip(f, :offset, t('plugins.thermometer.offset'), t('tooltips.thermometer.actions.offset')) + = f.text_field(:offset, class: 'form-control') diff --git a/app/views/plugins/donations_thermometer/_form.slim b/app/views/plugins/donations_thermometer/_form.slim deleted file mode 100644 index e69de29bb..000000000 diff --git a/app/views/plugins/donations_thermometers/_form.slim b/app/views/plugins/donations_thermometers/_form.slim new file mode 100644 index 000000000..e153d6762 --- /dev/null +++ b/app/views/plugins/donations_thermometers/_form.slim @@ -0,0 +1,12 @@ +.col-md-7 + = render partial: 'plugins/shared/toggle_form', locals: { plugin: plugin, path: plugins_thermometer_path(plugin) } + + - name = "plugins_donations_thermometer_#{plugin.id}" + = form_for plugin, url: plugins_thermometer_path(plugin), remote: true, as: name, html: {class: 'plugin-settings one-form', data: {type: name }} do |f| + = render "shared/error_messages", target: plugin + = render "shared/success_message", success: (defined?(success) || false) + = render 'plugins/shared/plugin_metadata', f: f + + .form-group + = label_with_tooltip(f, :offset, t('plugins.thermometer.offset'), t('tooltips.thermometer.donations.offset')) + = f.text_field(:offset, class: 'form-control') diff --git a/config/locales/champaign.en.yml b/config/locales/champaign.en.yml index 6a4332f33..b4e7d3dab 100644 --- a/config/locales/champaign.en.yml +++ b/config/locales/champaign.en.yml @@ -92,7 +92,11 @@ en: click_count: "How many times this share button has been clicked" conversion_count: "How many times a page has been viewed through a share made out of this share variant." local_conversion_rate: "Percentage of the clicks / shares of this variant that got at least one person to view the page." - thermometer_offset: "Number of fake signatures to add to true signature count. No commas or spaces." + thermometer: + actions: + offset: "Number of fake signatures to add to true signature count. No commas or spaces." + donations: + offset: "Unformatted amount in USD to add to the amount of donations collected. No commas or spaces." fundraiser: donation_band: "This determines the amounts displayed on the page to an unknown user without any information in their URL. Generally, you will want to stick with an option designed for non-donors." recurring_default: 'This sets whether the recurring donation checkbox is checked and/or hidden by default. If the url param recurring_default=recurring, recurring_default=only_recurring or recurring_default=one_off is passed, that will override the behavior specified here.' diff --git a/spec/controllers/plugins/shared_examples.rb b/spec/controllers/plugins/shared_examples.rb index 215414d3a..07185cf66 100644 --- a/spec/controllers/plugins/shared_examples.rb +++ b/spec/controllers/plugins/shared_examples.rb @@ -13,13 +13,13 @@ describe 'successful' do before do - allow(plugin_class).to receive(:find).with('1') { plugin } + allow(plugin_class.base_class).to receive(:find).with('1') { plugin } allow(plugin).to receive(:update) { true } put :update, params: { id: '1', plugin_name => { title: 'bar' }, format: :js } end it 'finds the plugin' do - expect(plugin_class).to have_received(:find).with('1') + expect(plugin_class.base_class).to have_received(:find).with('1') end it 'updates the plugin' do @@ -40,7 +40,7 @@ describe 'failure' do before do - allow(plugin_class).to receive(:find).with('1') { plugin } + allow(plugin_class.base_class).to receive(:find).with('1') { plugin } allow(plugin).to receive(:update) { false } allow(plugin).to receive(:errors) { {} } put :update, params: { id: '1', plugin_name => { title: 'bar' }, format: :js } diff --git a/spec/controllers/plugins/thermometers_controller_spec.rb b/spec/controllers/plugins/thermometers_controller_spec.rb index 5c9b483a4..cec4fddb1 100644 --- a/spec/controllers/plugins/thermometers_controller_spec.rb +++ b/spec/controllers/plugins/thermometers_controller_spec.rb @@ -4,5 +4,6 @@ require_relative 'shared_examples' describe Plugins::ThermometersController do - include_examples 'plugins controller', Plugins::ActionsThermometer, :plugins_thermometer + include_examples 'plugins controller', Plugins::ActionsThermometer, :plugins_actions_thermometer + include_examples 'plugins controller', Plugins::DonationsThermometer, :plugins_donations_thermometer end diff --git a/spec/liquid/liquid_renderer_spec.rb b/spec/liquid/liquid_renderer_spec.rb index 445bf7f47..2584fe959 100644 --- a/spec/liquid/liquid_renderer_spec.rb +++ b/spec/liquid/liquid_renderer_spec.rb @@ -362,7 +362,7 @@ end it 'is nil if no actions thermometer plugin' do - create :plugins_fundraiser, page: page + create :call_tool, page: page expect(page.plugins.size).to eq 1 expect(LiquidRenderer.new(page).personalization_data['actions_thermometer']).to eq nil end @@ -397,7 +397,7 @@ describe 'donations_thermometer' do it 'is nil if no donations thermometer plugin' do - create :plugins_fundraiser, page: page + create :call_tool, page: page expect(page.plugins.size).to eq 1 expect(LiquidRenderer.new(page).personalization_data['donations_thermometer']).to eq nil end diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 11db57ad8..7c592ca45 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -448,8 +448,8 @@ describe 'plugins' do it 'correctly lists the names of plugins' do page = create :page - [create(:plugins_petition, page: page), create(:plugins_fundraiser, page: page), create(:plugins_actions_thermometer, page: page)] - plugin_names = %w[petition fundraiser actions_thermometer] + [create(:plugins_petition, page: page), create(:call_tool, page: page), create(:plugins_actions_thermometer, page: page)] + plugin_names = %w[petition call_tool actions_thermometer] expect(page.plugin_names).to match_array(plugin_names) end end From 32d941398311c77e964c3a4a2406475fbf5ccf01 Mon Sep 17 00:00:00 2001 From: tuuli Date: Thu, 22 Nov 2018 17:39:11 +0100 Subject: [PATCH 07/18] Fix form for donations thermometer --- app/models/plugins/donations_thermometer.rb | 2 +- app/views/plugins/actions_thermometers/_form.slim | 2 ++ .../plugins/donations_thermometers/_form.slim | 15 +++++++++++++-- spec/liquid/liquid_renderer_spec.rb | 1 + 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/models/plugins/donations_thermometer.rb b/app/models/plugins/donations_thermometer.rb index f97711669..697510f64 100644 --- a/app/models/plugins/donations_thermometer.rb +++ b/app/models/plugins/donations_thermometer.rb @@ -37,7 +37,7 @@ def liquid_data(_supplemental_data = {}) def currencies_hash(amount) # Get a hash with amount converted into all supported currencies. # Transform values from arrays of amounts to single amounts (e.g. GBP: [10] to GBP: 10) - Donations::Currencies.for([amount]).to_hash.with_indifferent_access.transform_values(&:pop) + Donations::Currencies.for([amount]).to_hash.with_indifferent_access.transform_values(&:pop).transform_values(&:to_d) end def fundraising_goal diff --git a/app/views/plugins/actions_thermometers/_form.slim b/app/views/plugins/actions_thermometers/_form.slim index 20cb4eb3a..c2776a2f3 100644 --- a/app/views/plugins/actions_thermometers/_form.slim +++ b/app/views/plugins/actions_thermometers/_form.slim @@ -1,4 +1,6 @@ .col-md-7 + // There's a single ThermometersController that handles both actions and donations thermometers, so use the + // generic plugins_thermometer_path for the toggle. = render partial: 'plugins/shared/toggle_form', locals: { plugin: plugin, path: plugins_thermometer_path(plugin) } - name = "plugins_actions_thermometer_#{plugin.id}" diff --git a/app/views/plugins/donations_thermometers/_form.slim b/app/views/plugins/donations_thermometers/_form.slim index e153d6762..4ad4413d1 100644 --- a/app/views/plugins/donations_thermometers/_form.slim +++ b/app/views/plugins/donations_thermometers/_form.slim @@ -1,4 +1,6 @@ .col-md-7 + // There's a single ThermometersController that handles both actions and donations thermometers, so use the + // generic plugins_thermometer_path for the toggle. = render partial: 'plugins/shared/toggle_form', locals: { plugin: plugin, path: plugins_thermometer_path(plugin) } - name = "plugins_donations_thermometer_#{plugin.id}" @@ -8,5 +10,14 @@ = render 'plugins/shared/plugin_metadata', f: f .form-group - = label_with_tooltip(f, :offset, t('plugins.thermometer.offset'), t('tooltips.thermometer.donations.offset')) - = f.text_field(:offset, class: 'form-control') + = label_with_tooltip(f, :offset_amount, t('plugins.thermometer.offset'), t('tooltips.thermometer.donations.offset')) + = number_field_tag :offset_amount, plugin.offset/100, class: 'form-control' + = f.hidden_field :offset, class: 'offset-in-cents' + +javascript: + $(function () { + $('#offset_amount').on('change', function(){ + // change the value of the hidden offset-in-cents field to the manually entered offset amount * 100 + $('.offset-in-cents').val($(this).val() * 100); + }) + }); diff --git a/spec/liquid/liquid_renderer_spec.rb b/spec/liquid/liquid_renderer_spec.rb index 2584fe959..c2325e6cb 100644 --- a/spec/liquid/liquid_renderer_spec.rb +++ b/spec/liquid/liquid_renderer_spec.rb @@ -220,6 +220,7 @@ end before :each do + allow_any_instance_of(Plugins::DonationsThermometer).to receive(:liquid_data).and_return({}) allow(PaymentProcessor::Currency).to receive(:convert) end From f7e2b275f53b5b902457fdd6f0061f2106ed05a3 Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Mon, 12 Nov 2018 23:47:53 +0000 Subject: [PATCH 08/18] Fundraising Thermometer state and service --- .../AmountSelection/AmountSelection.js | 2 + app/javascript/components/Payment/Payment.js | 2 +- app/javascript/components/Thermometer.css | 36 ++++++++++++++++ app/javascript/components/Thermometer.js | 38 +++++++++++++++++ app/javascript/components/Thermometer.test.js | 14 +++++++ .../__snapshots__/Thermometer.test.js.snap | 39 +++++++++++++++++ app/javascript/fundraiser/FundraiserView.js | 2 + .../fundraising_thermometer/index.js | 41 ++++++++++++++++++ app/javascript/state/features/index.js | 2 + app/javascript/state/reducers.js | 42 ++++++++++--------- app/javascript/state/thermometer/index.js | 38 +++++++++++++++++ 11 files changed, 235 insertions(+), 21 deletions(-) create mode 100644 app/javascript/components/Thermometer.css create mode 100644 app/javascript/components/Thermometer.js create mode 100644 app/javascript/components/Thermometer.test.js create mode 100644 app/javascript/components/__snapshots__/Thermometer.test.js.snap create mode 100644 app/javascript/fundraising_thermometer/index.js create mode 100644 app/javascript/state/thermometer/index.js diff --git a/app/javascript/components/AmountSelection/AmountSelection.js b/app/javascript/components/AmountSelection/AmountSelection.js index 374b7790f..78aaa2561 100644 --- a/app/javascript/components/AmountSelection/AmountSelection.js +++ b/app/javascript/components/AmountSelection/AmountSelection.js @@ -4,6 +4,7 @@ import { FormattedMessage } from 'react-intl'; import DonationBands from '../DonationBands/DonationBands'; import Button from '../Button/Button'; import ee from '../../shared/pub_sub'; +import Thermometer from '../Thermometer'; import CurrencyAmount from '../../components/CurrencyAmount'; @@ -74,6 +75,7 @@ export default class AmountSelection extends React.Component { render() { return (
+ ({ ? 'gocardless' : state.fundraiser.currentPaymentType, fundraiser: state.fundraiser, - page: state.page, paymentMethods: state.paymentMethods, + paymentMethodss: state.fundraisingThermometer, member: state.member, hideRecurring: state.fundraiser.recurringDefault === 'only_recurring', formData: { diff --git a/app/javascript/components/Thermometer.css b/app/javascript/components/Thermometer.css new file mode 100644 index 000000000..a7c45c639 --- /dev/null +++ b/app/javascript/components/Thermometer.css @@ -0,0 +1,36 @@ +.Thermometer-stats { + width: 100%; + line-height: 14px; + font-size: 0.9em; + font-weight: bold; + text-transform: uppercase; +} +.Thermometer-temperature { + width: auto; + float: left; + text-align: left; +} +.Thermometer-goal { + width: auto; + text-align: right; + color: rgba(48, 57, 79, 0.5); +} + +.Thermometer-bg { + clear: both; + position: relative; + background-color: white; + width: 100%; + height: 12px; + border-radius: 4px; + margin: 5px 0 25px; + padding: 0; +} + +.Thermometer-mercury { + border-radius: 4px; + background-color: #00c0cf; + margin: 0; + padding: 0; + height: 100%; +} diff --git a/app/javascript/components/Thermometer.js b/app/javascript/components/Thermometer.js new file mode 100644 index 000000000..93e664bb3 --- /dev/null +++ b/app/javascript/components/Thermometer.js @@ -0,0 +1,38 @@ +// @flow +import React from 'react'; +import CurrencyAmount from './CurrencyAmount'; +import './Thermometer.css'; +import type { AppState } from '../state/reducers'; + +type Props = { + donations: number, + goal: number, + currencyCode: string, + color?: string, +}; + +export default function Thermometer(props: Props) { + const mercuryStyle = { + backgroundColor: props.color || '#00c0cf', + width: `${Math.round((props.donations / props.goal) * 100)}%`, + }; + + return ( +
+
+
+ +
+
+ +
+
+
+
+
+
+ ); +} diff --git a/app/javascript/components/Thermometer.test.js b/app/javascript/components/Thermometer.test.js new file mode 100644 index 000000000..037e3109a --- /dev/null +++ b/app/javascript/components/Thermometer.test.js @@ -0,0 +1,14 @@ +import React from 'react'; +import renderer from 'react-test-renderer'; +import { IntlProvider } from 'react-intl'; +import Thermometer from './Thermometer'; + +test('Renders as expected', () => { + const component = renderer.create( + + + + ); + + expect(component.toJSON()).toMatchSnapshot(); +}); diff --git a/app/javascript/components/__snapshots__/Thermometer.test.js.snap b/app/javascript/components/__snapshots__/Thermometer.test.js.snap new file mode 100644 index 000000000..c02e01589 --- /dev/null +++ b/app/javascript/components/__snapshots__/Thermometer.test.js.snap @@ -0,0 +1,39 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Renders as expected 1`] = ` +
+
+
+ + £1,000 + +
+
+ + £600,000 + +
+
+
+
+
+
+`; diff --git a/app/javascript/fundraiser/FundraiserView.js b/app/javascript/fundraiser/FundraiserView.js index 9c86827b1..6e455fcbd 100644 --- a/app/javascript/fundraiser/FundraiserView.js +++ b/app/javascript/fundraiser/FundraiserView.js @@ -134,6 +134,7 @@ export class FundraiserView extends Component { }> this.props.setSubmitting(s)} /> @@ -145,6 +146,7 @@ export class FundraiserView extends Component { } export const mapStateToProps = (state: AppState) => ({ + features: state.features, fundraiser: state.fundraiser, member: state.member, page: state.page, diff --git a/app/javascript/fundraising_thermometer/index.js b/app/javascript/fundraising_thermometer/index.js new file mode 100644 index 000000000..c9b7dd305 --- /dev/null +++ b/app/javascript/fundraising_thermometer/index.js @@ -0,0 +1,41 @@ +// @flow +import { update, increment } from '../state/thermometer'; +import type { Store } from 'redux'; +import type { AppState } from '../state/reducers'; +import type { Action, State } from '../state/thermometer'; + +type Config = { + store: Store, + attrs: State, +}; + +// Creates a thermometer with a unique id (uuid/v4) and syncs it to the store. +export class Thermometer { + store: Store; + + constructor(options: Config) { + this.store = options.store; + this.store.dispatch(update(options.attrs)); + } + + attrs() { + return this.store.getState().fundraisingThermometer; + } + + update(attrs: $Shape) { + this.store.dispatch(update(attrs)); + } + + increment(donation: number) { + this.store.dispatch(increment(donation)); + } +} + +const myThermometer = new Thermometer({ + store: window.champaign.store, + attrs: { + donations: 0, + goal: 100, + currencyCode: 'GBP', + }, +}); diff --git a/app/javascript/state/features/index.js b/app/javascript/state/features/index.js index 00c7a937d..724c60ddb 100644 --- a/app/javascript/state/features/index.js +++ b/app/javascript/state/features/index.js @@ -10,10 +10,12 @@ import type { Store } from 'redux'; export type State = { googlepay: boolean, + thermometer: boolean, }; const defaults: State = { googlepay: false, + thermometer: false, }; type Action = diff --git a/app/javascript/state/reducers.js b/app/javascript/state/reducers.js index 71ebfd1e3..7f8434b68 100644 --- a/app/javascript/state/reducers.js +++ b/app/javascript/state/reducers.js @@ -6,9 +6,19 @@ import paymentMethods from './paymentMethods/reducer'; import page from './page/reducer'; import consent from './consent'; import features from './features'; +import fundraisingThermometer from './thermometer'; import extraActionFields from './extraActionFields'; import { reducer as emailTarget } from './email_pension/actions'; +import type { ConsentState } from './consent'; +import type { Fundraiser, EnumRecurringDefault } from './fundraiser/types'; +import type { Member } from './member/reducer'; +import type { PaymentMethod } from './paymentMethods/reducer'; +import type { State as ExtraActionFieldsState } from './extraActionFields'; +import type { State as FeaturesState } from './features'; +import type { ChampaignPage, ChampaignGlobalObject } from '../types'; +import type { State as FundraisingThermometerState } from './thermometer'; + const reducers = { consent, emailTarget, @@ -18,31 +28,23 @@ const reducers = { member, page, paymentMethods, + fundraisingThermometer, }; -export default combineReducers(reducers); - -// import types -import type { ConsentState } from './consent'; -import type { Fundraiser, EnumRecurringDefault } from './fundraiser/types'; -import type { Member } from './member/reducer'; -import type { PaymentMethod } from './paymentMethods/reducer'; -import type { State as ExtraActionFieldsState } from './extraActionFields'; -import type { State as FeaturesState } from './features'; -import type { ChampaignPage, ChampaignGlobalObject } from '../types'; - +// type ReturnTypes = ((...args: any[]) => V) => V; +// export type AppState = $ObjMap; export type AppState = { - consent: ConsentState, - extraActionFields: ExtraActionFieldsState, - emailTarget: { [string]: any }, - features: FeaturesState, - fundraiser: Fundraiser, - member: Member, - page: ChampaignPage, - paymentMethods: PaymentMethod[], + +consent: ConsentState, + +extraActionFields: ExtraActionFieldsState, + +features: FeaturesState, + +fundraiser: Fundraiser, + +member: Member, + +page: ChampaignPage, + +paymentMethods: PaymentMethod[], + +fundraisingThermometer: FundraisingThermometerState, }; -type ChampaignPaymentMethod = any; +export default combineReducers(reducers); export type FormField = { id: number, diff --git a/app/javascript/state/thermometer/index.js b/app/javascript/state/thermometer/index.js new file mode 100644 index 000000000..b66e706dd --- /dev/null +++ b/app/javascript/state/thermometer/index.js @@ -0,0 +1,38 @@ +// @flow + +export type Action = + | { type: '@@chmp:thermometer:update', attrs: $Shape } + | { type: '@@chmp:thermometer:increment', temperature: number }; + +export type State = { + donations: number, + goal: number, + currencyCode: string, + // showMarkers indicates that we want the percentage markers + // for 0, 25, 50, 75, 100 percentiles underneath the bar + showMarkers?: boolean, + // showPercentage indicates if the percentage should be shown + // in the thermometer bar. + showPercentage?: boolean, +}; + +const defaults: State = { + donations: 0, + goal: 100, + currencyCode: 'USD', +}; + +export default function reducer( + state?: State = defaults, + action: Action +): State { + return state; +} + +export function update(attrs: $Shape): Action { + return { type: '@@chmp:thermometer:update', attrs }; +} + +export function increment(temperature: number): Action { + return { type: '@@chmp:thermometer:increment', temperature }; +} From 986598e1694d8d55299abbeab59a04c788a8938d Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Thu, 22 Nov 2018 11:53:17 +0000 Subject: [PATCH 09/18] Rename `fundraisingThermometer` to `donationsThermometer` --- app/javascript/components/Payment/Payment.js | 2 +- .../index.js | 11 +---------- app/javascript/state/reducers.js | 8 ++++---- 3 files changed, 6 insertions(+), 15 deletions(-) rename app/javascript/{fundraising_thermometer => donations_thermometer}/index.js (77%) diff --git a/app/javascript/components/Payment/Payment.js b/app/javascript/components/Payment/Payment.js index 8ef84989f..051507401 100644 --- a/app/javascript/components/Payment/Payment.js +++ b/app/javascript/components/Payment/Payment.js @@ -503,7 +503,7 @@ const mapStateToProps = (state: AppState) => ({ : state.fundraiser.currentPaymentType, fundraiser: state.fundraiser, paymentMethods: state.paymentMethods, - paymentMethodss: state.fundraisingThermometer, + paymentMethodss: state.donationsThermometer, member: state.member, hideRecurring: state.fundraiser.recurringDefault === 'only_recurring', formData: { diff --git a/app/javascript/fundraising_thermometer/index.js b/app/javascript/donations_thermometer/index.js similarity index 77% rename from app/javascript/fundraising_thermometer/index.js rename to app/javascript/donations_thermometer/index.js index c9b7dd305..565eeff9a 100644 --- a/app/javascript/fundraising_thermometer/index.js +++ b/app/javascript/donations_thermometer/index.js @@ -19,7 +19,7 @@ export class Thermometer { } attrs() { - return this.store.getState().fundraisingThermometer; + return this.store.getState().donationsThermometer; } update(attrs: $Shape) { @@ -30,12 +30,3 @@ export class Thermometer { this.store.dispatch(increment(donation)); } } - -const myThermometer = new Thermometer({ - store: window.champaign.store, - attrs: { - donations: 0, - goal: 100, - currencyCode: 'GBP', - }, -}); diff --git a/app/javascript/state/reducers.js b/app/javascript/state/reducers.js index 7f8434b68..a999e9e0e 100644 --- a/app/javascript/state/reducers.js +++ b/app/javascript/state/reducers.js @@ -6,7 +6,7 @@ import paymentMethods from './paymentMethods/reducer'; import page from './page/reducer'; import consent from './consent'; import features from './features'; -import fundraisingThermometer from './thermometer'; +import donationsThermometer from './thermometer'; import extraActionFields from './extraActionFields'; import { reducer as emailTarget } from './email_pension/actions'; @@ -17,7 +17,7 @@ import type { PaymentMethod } from './paymentMethods/reducer'; import type { State as ExtraActionFieldsState } from './extraActionFields'; import type { State as FeaturesState } from './features'; import type { ChampaignPage, ChampaignGlobalObject } from '../types'; -import type { State as FundraisingThermometerState } from './thermometer'; +import type { State as donationsThermometerState } from './thermometer'; const reducers = { consent, @@ -28,7 +28,7 @@ const reducers = { member, page, paymentMethods, - fundraisingThermometer, + donationsThermometer, }; // type ReturnTypes = ((...args: any[]) => V) => V; @@ -41,7 +41,7 @@ export type AppState = { +member: Member, +page: ChampaignPage, +paymentMethods: PaymentMethod[], - +fundraisingThermometer: FundraisingThermometerState, + +donationsThermometer: donationsThermometerState, }; export default combineReducers(reducers); From d2a01e0884b91bc8ecb123d6575f7d1bed4a1bfc Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Thu, 22 Nov 2018 17:34:14 +0000 Subject: [PATCH 10/18] Bind Thermometer to redux state --- .../AmountSelection/AmountSelection.js | 2 +- .../AmountSelection/AmountSelection.test.js | 20 +++- app/javascript/components/Thermometer.css | 12 +- app/javascript/components/Thermometer.js | 61 +++++++--- app/javascript/components/Thermometer.test.js | 9 +- .../__snapshots__/Thermometer.test.js.snap | 38 +------ app/javascript/donations_thermometer/index.js | 32 ------ .../fundraiser/FundraiserView.test.js | 39 +++++++ app/javascript/state/thermometer/index.js | 57 +++++++--- app/javascript/types.js | 11 ++ app/models/plugins/donations_thermometer.rb | 2 +- spec/jest/mockReduxStore.js | 104 ++++++++++++++++++ 12 files changed, 272 insertions(+), 115 deletions(-) delete mode 100644 app/javascript/donations_thermometer/index.js create mode 100644 spec/jest/mockReduxStore.js diff --git a/app/javascript/components/AmountSelection/AmountSelection.js b/app/javascript/components/AmountSelection/AmountSelection.js index 78aaa2561..18c5f0997 100644 --- a/app/javascript/components/AmountSelection/AmountSelection.js +++ b/app/javascript/components/AmountSelection/AmountSelection.js @@ -75,7 +75,7 @@ export default class AmountSelection extends React.Component { render() { return (
- + { - const component = mountWithIntl(); + const component = mountWithIntl( + + + + ); expect(component.html()).toBeTruthy(); }); describe('Donation bands', () => { - const component = mountWithIntl(); + const component = mountWithIntl( + + + + ); it('shows the donation band passed as an argument', () => { const firstButton = component @@ -60,7 +70,11 @@ describe('Donation bands', () => { }); describe('Changing currency', () => { - const component = mountWithIntl(); + const component = mountWithIntl( + + + + ); it('does not show the currency menu by default', () => { expect(component.find('.AmountSelection__currency-selector').length).toBe( diff --git a/app/javascript/components/Thermometer.css b/app/javascript/components/Thermometer.css index a7c45c639..c8336618d 100644 --- a/app/javascript/components/Thermometer.css +++ b/app/javascript/components/Thermometer.css @@ -17,14 +17,15 @@ } .Thermometer-bg { - clear: both; - position: relative; background-color: white; - width: 100%; - height: 12px; + border-color: #aaa; border-radius: 4px; - margin: 5px 0 25px; + clear: both; + height: 16px; + margin: 5px 0 20px; padding: 0; + position: relative; + width: 100%; } .Thermometer-mercury { @@ -33,4 +34,5 @@ margin: 0; padding: 0; height: 100%; + min-width: 8px; } diff --git a/app/javascript/components/Thermometer.js b/app/javascript/components/Thermometer.js index 93e664bb3..13e268ac2 100644 --- a/app/javascript/components/Thermometer.js +++ b/app/javascript/components/Thermometer.js @@ -1,38 +1,65 @@ // @flow import React from 'react'; +import { connect } from 'react-redux'; +import { isEmpty, min } from 'lodash'; import CurrencyAmount from './CurrencyAmount'; import './Thermometer.css'; import type { AppState } from '../state/reducers'; -type Props = { - donations: number, - goal: number, - currencyCode: string, - color?: string, -}; +type Props = + | {} + | { + active: boolean, + currency: string, + donations: number, + goal: number, + offset: number, + remaining: number, + title: ?string, + }; -export default function Thermometer(props: Props) { - const mercuryStyle = { - backgroundColor: props.color || '#00c0cf', - width: `${Math.round((props.donations / props.goal) * 100)}%`, - }; +export function Thermometer(props: Props) { + // Only render if active + if (isEmpty(props) || !props.active) return null; + + // Prevent overflow when donations > goal. + const donations = min([props.donations, props.goal]); return (
+

{props.title}

- +
- +
-
+
); } + +const mapStateToProps = (state: AppState): Props => { + const data = state.donationsThermometer; + const currency = state.fundraiser.currency; + if (isEmpty(data)) return {}; + return { + active: data.active, + currency, + donations: data.totalDonations[currency], + goal: data.goals[currency], + offset: data.offset, + title: data.title, + }; +}; + +export default connect(mapStateToProps)(Thermometer); diff --git a/app/javascript/components/Thermometer.test.js b/app/javascript/components/Thermometer.test.js index 037e3109a..da74f8c3a 100644 --- a/app/javascript/components/Thermometer.test.js +++ b/app/javascript/components/Thermometer.test.js @@ -1,12 +1,17 @@ import React from 'react'; import renderer from 'react-test-renderer'; import { IntlProvider } from 'react-intl'; -import Thermometer from './Thermometer'; +import { Thermometer } from './Thermometer'; test('Renders as expected', () => { const component = renderer.create( - + ); diff --git a/app/javascript/components/__snapshots__/Thermometer.test.js.snap b/app/javascript/components/__snapshots__/Thermometer.test.js.snap index c02e01589..47c87b558 100644 --- a/app/javascript/components/__snapshots__/Thermometer.test.js.snap +++ b/app/javascript/components/__snapshots__/Thermometer.test.js.snap @@ -1,39 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Renders as expected 1`] = ` -
-
-
- - £1,000 - -
-
- - £600,000 - -
-
-
-
-
-
-`; +exports[`Renders as expected 1`] = `null`; diff --git a/app/javascript/donations_thermometer/index.js b/app/javascript/donations_thermometer/index.js deleted file mode 100644 index 565eeff9a..000000000 --- a/app/javascript/donations_thermometer/index.js +++ /dev/null @@ -1,32 +0,0 @@ -// @flow -import { update, increment } from '../state/thermometer'; -import type { Store } from 'redux'; -import type { AppState } from '../state/reducers'; -import type { Action, State } from '../state/thermometer'; - -type Config = { - store: Store, - attrs: State, -}; - -// Creates a thermometer with a unique id (uuid/v4) and syncs it to the store. -export class Thermometer { - store: Store; - - constructor(options: Config) { - this.store = options.store; - this.store.dispatch(update(options.attrs)); - } - - attrs() { - return this.store.getState().donationsThermometer; - } - - update(attrs: $Shape) { - this.store.dispatch(update(attrs)); - } - - increment(donation: number) { - this.store.dispatch(increment(donation)); - } -} diff --git a/app/javascript/fundraiser/FundraiserView.test.js b/app/javascript/fundraiser/FundraiserView.test.js index 98be2f6ac..d16fe712a 100644 --- a/app/javascript/fundraiser/FundraiserView.test.js +++ b/app/javascript/fundraiser/FundraiserView.test.js @@ -136,6 +136,45 @@ const fetchInitialState = vals => { ...defaults.fundraiser, ...vals, }, + donationsThermometer: { + id: 10, + title: null, + offset: 0, + page_id: 2, + active: true, + created_at: '2018-11-22T17:58:09.748Z', + updated_at: '2018-11-22T17:58:32.582Z', + ref: '', + type: 'DonationsThermometer', + percentage: 0, + remaining_amounts: { + USD: -90, + GBP: -90, + EUR: -90, + CHF: -90, + AUD: -90, + NZD: -90, + CAD: -90, + }, + total_donations: { + USD: 10, + GBP: 10, + EUR: 10, + CHF: 10, + AUD: 10, + NZD: 10, + CAD: 10, + }, + goals: { + USD: 100, + GBP: 100, + EUR: 100, + CHF: 100, + AUD: 100, + NZD: 100, + CAD: 100, + }, + }, }, }; }; diff --git a/app/javascript/state/thermometer/index.js b/app/javascript/state/thermometer/index.js index b66e706dd..2d4d04029 100644 --- a/app/javascript/state/thermometer/index.js +++ b/app/javascript/state/thermometer/index.js @@ -1,38 +1,61 @@ // @flow +import type { ChampaignGlobalObject } from '../../types'; +import { isEmpty, mapValues } from 'lodash'; export type Action = - | { type: '@@chmp:thermometer:update', attrs: $Shape } + | { type: '@@chmp:initialize', payload: ChampaignGlobalObject } + | { type: '@@chmp:thermometer:update', attrs: State } | { type: '@@chmp:thermometer:increment', temperature: number }; -export type State = { - donations: number, - goal: number, - currencyCode: string, - // showMarkers indicates that we want the percentage markers - // for 0, 25, 50, 75, 100 percentiles underneath the bar - showMarkers?: boolean, - // showPercentage indicates if the percentage should be shown - // in the thermometer bar. - showPercentage?: boolean, -}; +export type State = + | {} + | { + active: boolean, + goals: { [currency: string]: number }, + offset: number, + remainingAmounts: { [currency: string]: number }, + title: ?string, + totalDonations?: { [currency: string]: number }, + }; const defaults: State = { - donations: 0, - goal: 100, - currencyCode: 'USD', + active: false, + goals: {}, + offset: 0, + remainingAmounts: {}, + title: '', + totalDonations: {}, }; export default function reducer( state?: State = defaults, action: Action ): State { - return state; + switch (action.type) { + case '@@chmp:initialize': + return getStateFromChampaign(action.payload); + default: + return state; + } } -export function update(attrs: $Shape): Action { +export function update(attrs: State): Action { return { type: '@@chmp:thermometer:update', attrs }; } export function increment(temperature: number): Action { return { type: '@@chmp:thermometer:increment', temperature }; } + +function getStateFromChampaign(chmp: ChampaignGlobalObject): State { + const data = chmp.personalization.donationsThermometer; + if (isEmpty(data)) return { active: false }; + return { + active: data.active, + offset: data.offset, + title: data.title, + goals: data.goals, + remainingAmounts: data.remaining_amounts, + totalDonations: data.total_donations, + }; +} diff --git a/app/javascript/types.js b/app/javascript/types.js index e28df6482..e84023ebb 100644 --- a/app/javascript/types.js +++ b/app/javascript/types.js @@ -57,12 +57,23 @@ export type ChampaignPage = { follow_up_liquid_layout_id?: number, }; +type DonationsThermometer = + | {} + | { + active: boolean, + goals: { [currency: string]: number }, + offset: number, + remaining_amounts: { [currency: string]: number }, + title: string, + total_donations: { [currency: string]: number }, + }; export type ChampaignPersonalizationData = { locale: string, location: ChampaignLocation, member: ChampaignMember, paymentMethods: any[], urlParams: { [key: string]: string }, + donationsThermometer: DonationsThermometer, }; export type ChampaignGlobalObject = { diff --git a/app/models/plugins/donations_thermometer.rb b/app/models/plugins/donations_thermometer.rb index 697510f64..34ed54a7e 100644 --- a/app/models/plugins/donations_thermometer.rb +++ b/app/models/plugins/donations_thermometer.rb @@ -37,7 +37,7 @@ def liquid_data(_supplemental_data = {}) def currencies_hash(amount) # Get a hash with amount converted into all supported currencies. # Transform values from arrays of amounts to single amounts (e.g. GBP: [10] to GBP: 10) - Donations::Currencies.for([amount]).to_hash.with_indifferent_access.transform_values(&:pop).transform_values(&:to_d) + ::Donations::Currencies.for([amount]).to_hash.map { |k, v| [k, ::Donations::Utils.round(v).first] }.to_h end def fundraising_goal diff --git a/spec/jest/mockReduxStore.js b/spec/jest/mockReduxStore.js new file mode 100644 index 000000000..697284a67 --- /dev/null +++ b/spec/jest/mockReduxStore.js @@ -0,0 +1,104 @@ +import configureStore from '../../app/javascript/state'; + +const champaign = { + page: { + id: 2, + language_id: 1, + campaign_id: null, + title: 'Petition', + slug: 'petition', + status: 'pending', + action_count: 1, + follow_up_page_id: 1, + publish_status: 'published', + allow_duplicate_actions: false, + enforce_styles: false, + publish_actions: 'secure', + total_donations: '0.0', + fundraising_goal: '100000.0', + language_code: 'en', + layout: 'Example Layout', + follow_up_layout: null, + plugins: ['Plugins::Fundraiser', 'Plugins::DonationsThermometer'], + }, + personalization: { + outstandingFields: [], + paymentMethods: [], + donationBands: { + USD: [1, 5, 10, 15, 20], + GBP: [1, 4, 7, 11, 14], + EUR: [1, 4, 8, 12, 16], + CHF: [1, 5, 10, 14, 19], + AUD: [1, 7, 13, 20, 25], + NZD: [1, 7, 14, 20, 30], + CAD: [1, 6, 13, 19, 25], + }, + donationsThermometer: { + id: 1, + title: null, + offset: 10000, + active: true, + type: 'DonationsThermometer', + percentage: 0, + remaining_amounts: { + USD: -90000, + GBP: -90000, + EUR: -90000, + CHF: -90000, + AUD: -90000, + NZD: -90000, + CAD: -90000, + }, + total_donations: { + USD: 10000, + GBP: 10000, + EUR: 10000, + CHF: 10000, + AUD: 10000, + NZD: 10000, + CAD: 10000, + }, + goals: { + USD: 100000, + GBP: 100000, + EUR: 100000, + CHF: 100000, + AUD: 100000, + NZD: 100000, + CAD: 100000, + }, + }, + formValues: { + name: 'Test USer', + action_phone_number: '', + email: 'test@example.com', + country: 'US', + postal: '10000', + }, + location: { + country_name: 'United States', + country_code: 'US', + currency: 'USD', + country: 'US', + }, + member: { + id: 1, + name: 'Test User', + email: 'test@example.com', + country: 'US', + first_name: 'Test', + last_name: 'User', + postal: null, + actionkit_user_id: null, + donor_status: 'nondonor', + consented: null, + registered: false, + full_name: 'Test User', + welcome_name: 'Test User', + }, + }, +}; + +const store = configureStore(champaign); + +export { store, champaign }; From 8ad5ac60ddad44f710ff975e3129ea56c54add83 Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Fri, 23 Nov 2018 13:25:43 +0000 Subject: [PATCH 11/18] Convert amounts to cents before incrementing fundraising counter --- app/models/payment/braintree/transaction.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/payment/braintree/transaction.rb b/app/models/payment/braintree/transaction.rb index 9bee93588..37bbc94cb 100644 --- a/app/models/payment/braintree/transaction.rb +++ b/app/models/payment/braintree/transaction.rb @@ -50,6 +50,6 @@ def publish_subscription_charge def increment_funding_counter return unless status == 'success' - FundingCounter.update(page: page, currency: currency, amount: amount) + FundingCounter.update(page: page, currency: currency, amount: amount * 100) end end From df547825d41c3ed41836409b5e270294baf3eb14 Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Fri, 23 Nov 2018 13:26:24 +0000 Subject: [PATCH 12/18] Fix donations_count -> total_donations --- app/models/campaign.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/campaign.rb b/app/models/campaign.rb index 5d1986ebb..7b55441c1 100644 --- a/app/models/campaign.rb +++ b/app/models/campaign.rb @@ -22,7 +22,7 @@ def action_count pages.sum(:action_count) end - def donations_count + def total_donations pages.sum(:total_donations) end From 45325a25b50022f90e184c103b5c15ab8c363673 Mon Sep 17 00:00:00 2001 From: Omar Sahyoun Date: Fri, 23 Nov 2018 14:54:20 +0000 Subject: [PATCH 13/18] Wrap with VCR block --- spec/liquid/liquid_renderer_spec.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/liquid/liquid_renderer_spec.rb b/spec/liquid/liquid_renderer_spec.rb index c2325e6cb..ce943f5a8 100644 --- a/spec/liquid/liquid_renderer_spec.rb +++ b/spec/liquid/liquid_renderer_spec.rb @@ -9,6 +9,12 @@ let(:renderer) { LiquidRenderer.new(page) } let(:cache_helper) { double(:cache_helper, key_for_data: 'foo', key_for_markup: 'bar') } + around(:each) do |example| + VCR.use_cassette('money_from_oxr') do + example.run + end + end + describe 'new' do it 'receives the correct arguments' do expect do @@ -405,7 +411,7 @@ it "is serializes the donations thermometer plugin's data" do t1 = create :plugins_donations_thermometer, page: page - expected = t1.liquid_data.stringify_keys + expected = t1.liquid_data.deep_stringify_keys actual = LiquidRenderer.new(page).personalization_data['donations_thermometer'] # disagreement over timestamps is not what this test is about [expected, actual].each do |h| @@ -419,7 +425,7 @@ t1 = create :plugins_donations_thermometer, page: page, ref: 'secondary' create :plugins_donations_thermometer, page: page expect(page.plugins.size).to eq 2 - expected = t1.liquid_data.stringify_keys + expected = t1.liquid_data.deep_stringify_keys actual = LiquidRenderer.new(page).personalization_data['donations_thermometer'] # disagreement over timestamps is not what this test is about [expected, actual].each do |h| From 0e79a405db758368eb555cd214b41a95bcf00108 Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Fri, 23 Nov 2018 15:56:13 +0000 Subject: [PATCH 14/18] Donations count -> total donations --- app/controllers/api/pages_controller.rb | 2 +- spec/models/campaign_spec.rb | 2 +- spec/requests/api/pages_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/pages_controller.rb b/app/controllers/api/pages_controller.rb index 15c17fc67..132f6a4dc 100644 --- a/app/controllers/api/pages_controller.rb +++ b/app/controllers/api/pages_controller.rb @@ -64,7 +64,7 @@ def total_donations amount = @page.total_donations goal = @page.fundraising_goal else - amount = @page.campaign.donations_count + amount = @page.campaign.total_donations goal = @page.campaign.fundraising_goal end diff --git a/spec/models/campaign_spec.rb b/spec/models/campaign_spec.rb index d2543c46f..4d7b3dc32 100644 --- a/spec/models/campaign_spec.rb +++ b/spec/models/campaign_spec.rb @@ -32,7 +32,7 @@ let!(:page_b) { create(:page, campaign: campaign, total_donations: 1000) } it 'returns sum of counts from associated pages' do - expect(campaign.donations_count).to eq(1500) + expect(campaign.total_donations).to eq(1500) end end diff --git a/spec/requests/api/pages_spec.rb b/spec/requests/api/pages_spec.rb index 8a235e4cd..4c4a251fe 100644 --- a/spec/requests/api/pages_spec.rb +++ b/spec/requests/api/pages_spec.rb @@ -227,7 +227,7 @@ def json let!(:page3) { create(:page, total_donations: 30_000, campaign: campaign, fundraising_goal: 100_000) } it 'returns the total amount of donations and the donations goal for the campaign of the page' do - donation_args = { amount: campaign.donations_count, currency: 'EUR' } + donation_args = { amount: campaign.total_donations, currency: 'EUR' } goal_args = { amount: campaign.fundraising_goal, currency: 'EUR' } allow(FundingCounter).to receive(:convert).with(donation_args).and_return(Money.from_amount(148_159.20, 'EUR')) allow(FundingCounter).to receive(:convert).with(goal_args).and_return(Money.from_amount(162_521.20, 'EUR')) From ee656de215dd057f2a38db4a377fe43ec2f68fef Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Fri, 23 Nov 2018 17:53:22 +0000 Subject: [PATCH 15/18] Convert go cardless tx to cents for funding counter. update specs --- app/models/payment/go_cardless/transaction.rb | 2 +- spec/models/page_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/payment/go_cardless/transaction.rb b/app/models/payment/go_cardless/transaction.rb index 72e4c9932..6fc9613a1 100644 --- a/app/models/payment/go_cardless/transaction.rb +++ b/app/models/payment/go_cardless/transaction.rb @@ -94,6 +94,6 @@ def publish_failed_subscription_charge end def increment_funding_counter - FundingCounter.update(page: page, currency: currency, amount: amount) + FundingCounter.update(page: page, currency: currency, amount: amount * 100) end end diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 7c592ca45..77820231e 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -542,13 +542,15 @@ let!(:page_with_donations) { create :page, total_donations: 1010 } it 'increments the total donations counter' do - FactoryBot.create(:payment_braintree_transaction, page: page_with_donations, amount: 10, currency: 'USD') + # FIXME: Figure out how to make this work. The `Money` gem is changing + # amounts to cents here. + FactoryGirl.create(:payment_braintree_transaction, page: page_with_donations, amount: 10, currency: 'USD') expect(page_with_donations.reload.total_donations.to_s).to eq '1020.0' end it 'updates the total donations counter when a GoCardless transaction is created' do expect(page.total_donations).to eq 0 - FactoryBot.create(:payment_go_cardless_transaction, page: page, amount: 10, currency: 'USD') + FactoryGirl.create(:payment_go_cardless_transaction, page: page, amount: 10, currency: 'USD') expect(page.total_donations.to_s).to eq '10.0' end From e78796580fb285d53e5de47a0f27207e9d382f65 Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Fri, 23 Nov 2018 18:43:39 +0000 Subject: [PATCH 16/18] GoCardless transaction: Rescue if amount.nil? --- app/models/payment/go_cardless/transaction.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/payment/go_cardless/transaction.rb b/app/models/payment/go_cardless/transaction.rb index 6fc9613a1..77289f459 100644 --- a/app/models/payment/go_cardless/transaction.rb +++ b/app/models/payment/go_cardless/transaction.rb @@ -94,6 +94,10 @@ def publish_failed_subscription_charge end def increment_funding_counter - FundingCounter.update(page: page, currency: currency, amount: amount * 100) + FundingCounter.update(page: page, currency: currency, amount: (begin + amount * 100 + rescue + nil + end)) end end From 1fdba494c58e1845dd7353078d4847e6deb2f74b Mon Sep 17 00:00:00 2001 From: Vince Martinez Fasoro Date: Fri, 23 Nov 2018 19:40:04 +0000 Subject: [PATCH 17/18] Update Thermometer.js styling --- app/javascript/components/Thermometer.js | 21 +++++++++++++++++-- .../{Thermometer.css => Thermometer.scss} | 19 ++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) rename app/javascript/components/{Thermometer.css => Thermometer.scss} (73%) diff --git a/app/javascript/components/Thermometer.js b/app/javascript/components/Thermometer.js index 13e268ac2..0ae98a4b7 100644 --- a/app/javascript/components/Thermometer.js +++ b/app/javascript/components/Thermometer.js @@ -1,10 +1,11 @@ // @flow import React from 'react'; import { connect } from 'react-redux'; +import { FormattedMessage } from 'react-intl'; import { isEmpty, min } from 'lodash'; import CurrencyAmount from './CurrencyAmount'; -import './Thermometer.css'; import type { AppState } from '../state/reducers'; +import './Thermometer.scss'; type Props = | {} @@ -24,6 +25,14 @@ export function Thermometer(props: Props) { // Prevent overflow when donations > goal. const donations = min([props.donations, props.goal]); + const remaining = props.goal - donations; + + const $remaining = ( + + ); + const $goal = ( + + ); return (
@@ -33,7 +42,15 @@ export function Thermometer(props: Props) {
- + {remaining > 0 ? ( + + ) : ( + $goal + )}
diff --git a/app/javascript/components/Thermometer.css b/app/javascript/components/Thermometer.scss similarity index 73% rename from app/javascript/components/Thermometer.css rename to app/javascript/components/Thermometer.scss index c8336618d..6f013b081 100644 --- a/app/javascript/components/Thermometer.css +++ b/app/javascript/components/Thermometer.scss @@ -1,7 +1,17 @@ + +.Thermometer { + +} + +.Thermometer-title { + font-weight: bold; + margin-bottom: 1em; +} + .Thermometer-stats { width: 100%; line-height: 14px; - font-size: 0.9em; + font-size: 12px; font-weight: bold; text-transform: uppercase; } @@ -36,3 +46,10 @@ height: 100%; min-width: 8px; } + +/* Free standing version */ +.fundraiser-bar--freestanding { + .Thermometer-bg { + background-color: #f5f5f5; + } +} From 6fa42bc00f7c2ada71d213fb20a1815631783e23 Mon Sep 17 00:00:00 2001 From: tuuli Date: Mon, 26 Nov 2018 15:55:35 +0100 Subject: [PATCH 18/18] Retain the donations thermometer plugin upon switching between fundraiser templates, make it clear which columns are cents --- app/lib/funding_counter.rb | 6 ++--- app/liquid/page_plugin_switcher.rb | 15 ++++++++---- app/models/page.rb | 6 ++--- app/models/payment/braintree/transaction.rb | 3 ++- app/models/payment/go_cardless/transaction.rb | 17 +++++++------- app/models/plugins/donations_thermometer.rb | 3 ++- spec/factories/pages.rb | 4 ++-- spec/models/page_spec.rb | 23 ++++++++----------- 8 files changed, 42 insertions(+), 35 deletions(-) diff --git a/app/lib/funding_counter.rb b/app/lib/funding_counter.rb index b591faa95..bd958ec32 100644 --- a/app/lib/funding_counter.rb +++ b/app/lib/funding_counter.rb @@ -17,10 +17,10 @@ def self.convert(currency:, amount:) def update return if @page.blank? - original_amount = Money.from_amount(@page.total_donations, Settings.default_currency) - converted_amount = Money.from_amount(@amount, @currency).exchange_to(Settings.default_currency) - @page.update_attributes(total_donations: (original_amount + converted_amount).to_d) + original_amount = Money.new(@page.total_donations, Settings.default_currency) + converted_amount = Money.from_amount(@amount, @currency).exchange_to(Settings.default_currency) + @page.update_attributes(total_donations: (original_amount.cents + converted_amount.cents)) end def convert diff --git a/app/liquid/page_plugin_switcher.rb b/app/liquid/page_plugin_switcher.rb index d2d9ebb0b..53846bb61 100644 --- a/app/liquid/page_plugin_switcher.rb +++ b/app/liquid/page_plugin_switcher.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + class PagePluginSwitcher def initialize(page) @page = page @@ -15,14 +16,13 @@ def switch(new_layout, new_layout_2 = nil) def new_refs(layout_1, layout_2) return layout_1.plugin_refs if layout_2.blank? + (layout_1.plugin_refs + layout_2.plugin_refs).uniq end def delete_quitters(quitters) @page.plugins.each do |plugin| - if quitters.include? [plugin.name.underscore, plugin.ref.to_s] - plugin.destroy! - end + plugin.destroy! if quitters.include? [plugin.name.underscore, plugin.ref.to_s] end end @@ -35,12 +35,19 @@ def create_starters(starters) def find_overlap(old_plugin_refs, new_plugin_refs) old_plugin_refs = standardize_blank_refs(old_plugin_refs) new_plugin_refs = standardize_blank_refs(new_plugin_refs) - keepers = old_plugin_refs & new_plugin_refs + keepers = old_plugin_refs & new_plugin_refs + # If there's a fundraiser plugin that's kept, also keep the first donations thermometer + keepers.append(donations_thermometer) if keepers.flatten.include? 'fundraiser' quitters = old_plugin_refs - keepers starters = new_plugin_refs - keepers [keepers, quitters, starters] end + def donations_thermometer + thermometer = Plugins::DonationsThermometer.where(page_id: @page.id).first + ['donations_thermometer', thermometer.ref.to_s] + end + def plugin_refs_from_plugins(plugins) plugins.map { |p| [p.name.underscore, p.ref] } end diff --git a/app/models/page.rb b/app/models/page.rb index 7ac025396..646852b24 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -34,8 +34,8 @@ # publish_actions :integer default("0"), not null # meta_tags :string # meta_description :string -# total_donations :double default("0") -# fundraising_goal :double default("0") +# total_donations :double default("0") -> Total donations in CENTS +# fundraising_goal :double default("0") -> Fundraising goal in CENTS # class Page < ApplicationRecord @@ -165,7 +165,7 @@ def subscriptions_count def switch_plugins fields = %w[liquid_layout_id follow_up_liquid_layout_id follow_up_plan] - if fields.any? { |f| saved_changes.keys.include?(f) } + if fields.any? { |f| saved_changes.key?(f) } secondary = follow_up_plan == 'with_liquid' ? follow_up_liquid_layout : nil PagePluginSwitcher.new(self).switch(liquid_layout, secondary) end diff --git a/app/models/payment/braintree/transaction.rb b/app/models/payment/braintree/transaction.rb index 37bbc94cb..6bf7f8751 100644 --- a/app/models/payment/braintree/transaction.rb +++ b/app/models/payment/braintree/transaction.rb @@ -50,6 +50,7 @@ def publish_subscription_charge def increment_funding_counter return unless status == 'success' - FundingCounter.update(page: page, currency: currency, amount: amount * 100) + + FundingCounter.update(page: page, currency: currency, amount: amount) end end diff --git a/app/models/payment/go_cardless/transaction.rb b/app/models/payment/go_cardless/transaction.rb index 77289f459..978cc8ab1 100644 --- a/app/models/payment/go_cardless/transaction.rb +++ b/app/models/payment/go_cardless/transaction.rb @@ -37,12 +37,12 @@ class Payment::GoCardless::Transaction < ApplicationRecord scope :one_off, -> { where(subscription_id: nil) } ACTION_FROM_STATE = { - submitted: :submit, - confirmed: :confirm, - cancelled: :cancel, - failed: :fail, - charged_back: :charge_back, - paid_out: :pay_out + submitted: :submit, + confirmed: :confirm, + cancelled: :cancel, + failed: :fail, + charged_back: :charge_back, + paid_out: :pay_out }.freeze aasm do @@ -81,6 +81,7 @@ class Payment::GoCardless::Transaction < ApplicationRecord def publish_failed_subscription_charge return if subscription.blank? + ChampaignQueue.push({ type: 'subscription-payment', params: { @@ -95,8 +96,8 @@ def publish_failed_subscription_charge def increment_funding_counter FundingCounter.update(page: page, currency: currency, amount: (begin - amount * 100 - rescue + amount + rescue StandardError nil end)) end diff --git a/app/models/plugins/donations_thermometer.rb b/app/models/plugins/donations_thermometer.rb index 34ed54a7e..24ce8a7fa 100644 --- a/app/models/plugins/donations_thermometer.rb +++ b/app/models/plugins/donations_thermometer.rb @@ -7,7 +7,7 @@ # id :integer not null, primary key # type :string not null # title :string -# offset :integer +# offset :integer -> Offset in CENTS # page_id :integer # active :boolean default("false") # created_at :datetime not null @@ -20,6 +20,7 @@ class Plugins::DonationsThermometer < Plugins::Thermometer def current_progress return 0 if fundraising_goal.zero? + total_donations / fundraising_goal * 100 end diff --git a/spec/factories/pages.rb b/spec/factories/pages.rb index 5243e68a6..dd06c3c05 100644 --- a/spec/factories/pages.rb +++ b/spec/factories/pages.rb @@ -34,8 +34,8 @@ # publish_actions :integer default("0"), not null # meta_tags :string # meta_description :string -# total_donations :double default("0") -# fundraising_goal :double default("0") +# total_donations :double default("0") -> Total donations in CENTS +# fundraising_goal :double default("0") -> Fundraising goal in CENTS # FactoryBot.define do diff --git a/spec/models/page_spec.rb b/spec/models/page_spec.rb index 77820231e..15a57275c 100644 --- a/spec/models/page_spec.rb +++ b/spec/models/page_spec.rb @@ -539,32 +539,29 @@ end describe 'total donations counter' do - let!(:page_with_donations) { create :page, total_donations: 1010 } + let!(:page_with_donations) { create :page, total_donations: 101_000 } it 'increments the total donations counter' do - # FIXME: Figure out how to make this work. The `Money` gem is changing - # amounts to cents here. - FactoryGirl.create(:payment_braintree_transaction, page: page_with_donations, amount: 10, currency: 'USD') - expect(page_with_donations.reload.total_donations.to_s).to eq '1020.0' + FactoryBot.create(:payment_braintree_transaction, page: page_with_donations, amount: 10, currency: 'USD') + expect(page_with_donations.reload.total_donations.to_s).to eq '102000.0' end it 'updates the total donations counter when a GoCardless transaction is created' do expect(page.total_donations).to eq 0 - FactoryGirl.create(:payment_go_cardless_transaction, page: page, amount: 10, currency: 'USD') - expect(page.total_donations.to_s).to eq '10.0' + FactoryBot.create(:payment_go_cardless_transaction, page: page, amount: 10, currency: 'USD') + expect(page.total_donations.to_s).to eq '1000.0' end it 'updates the total donations counter with a converted amount when a donation is created in another currency' do converted_amount = double(amount: 10, currency: 'EUR') - # Money.from_amount gets called twice in FundingCounter.update to create two Money objects - first to get - # page.total_donations, then the amount of the new donation. That's why I stub it to return - # two different things. - allow(Money).to receive(:from_amount).and_return(page.total_donations, converted_amount) - allow(converted_amount).to receive(:exchange_to).with('USD').and_return(12) + total_donations = double(cents: page.total_donations) + allow(Money).to receive(:new).and_return(total_donations) + allow(Money).to receive(:from_amount).and_return(converted_amount) + allow(converted_amount).to receive(:exchange_to).with('USD').and_return(double(cents: 1200)) expect(page.total_donations).to eq 0 FactoryBot.create(:payment_braintree_transaction, page: page, amount: 10, currency: 'EUR') - expect(page.total_donations.to_s).to eq '12.0' + expect(page.total_donations.to_s).to eq '1200.0' end end end