Merge remote-tracking branch 'origin/master' into aws-eb-master

.ebextensions/cronjob.config

@@ -103,9 +103,10 @@ files:
             #
             0 08 * * 1 root run-webapp-job i18n:transifex_load
             #
-            # Delete old alert content
+            # Clean ups
             #
             0 1 * * 7 root run-webapp-job alerts:delete_alert_content
+            15 5 * * * root run-as-webapp bin/rails r Rails.cache.cleanup
             #
             # Send Good Job queue metrics to AWS Cloudwatch
             #

app/assets/stylesheets/base.scss

@@ -201,6 +201,11 @@ input, progress {
   color: $grey-light;
 }
 
+.badge-grey-light {
+  background-color: $grey-light;
+  color: $white;
+}
+
 .text-dark-blue {
   color: $blue-very-dark;
 }

app/controllers/admin/prob_data_reports_controller.rb

@@ -12,7 +12,7 @@ def find_prob_data
                          .joins('LEFT JOIN school_groups on schools.school_group_id = school_groups.id')
                          .where(status: 'PROB')
                          .group('school_groups.name', 'schools.name', 'schools.slug', 'meters.meter_type', 'meters.name', :mpan_mprn, 'meters.id')
-                         .order('count(mpan_mprn) DESC')
+                         .order('count(mpan_mprn) DESC, school_groups.name, schools.name')
                          .count
                          .map { |row| row.to_a.flatten }
     end

app/controllers/concerns/newsletter_subscriber.rb

@@ -3,14 +3,37 @@ module NewsletterSubscriber
 
   private
 
-  def subscribe_newsletter(user)
-    MailchimpSubscriber.new(MailchimpApi.new).subscribe(user)
-  rescue MailchimpSubscriber::Error => e
-    Rails.logger.error e.backtrace.join("\n")
-    Rollbar.error(e, school_id: user.school_id, school_name: user.school_name)
+  def create_contact_from_user(user, sign_up_params)
+    Mailchimp::Contact.from_user(user, interests: sign_up_params[:interests].transform_values {|v| v == 'true' })
   end
 
-  def auto_subscribe_newsletter?
-    params[:user] && params[:user].key?(:auto_subscribe_newsletter)
+  def subscribe_contact(contact, show_errors: true)
+    resp = nil
+    if contact.valid?
+      begin
+        resp = audience_manager.subscribe_or_update_contact(contact)
+      rescue => e
+        Rails.logger.error(e)
+        Rollbar.error(e)
+        flash[:error] = 'Unable to process Mailchimp newsletter subscription' if show_errors
+      end
+    elsif show_errors
+      flash[:error] = contact.errors.full_messages.join(', ')
+    end
+    resp
+  end
+
+  def audience_manager
+    @audience_manager ||= Mailchimp::AudienceManager.new
+  end
+
+  def list_of_email_types
+    category = audience_manager.categories.detect {|c| c.title == 'Interests' }
+    return [] unless category
+    return audience_manager.interests(category.id)
+  rescue => e
+    Rails.logger.error(e)
+    Rollbar.error(e)
+    []
   end
 end

app/controllers/home_controller.rb

@@ -111,7 +111,7 @@ def videos
   end
 
   def set_newsletters
-    @newsletters = Newsletter.order(published_on: :desc).limit(3)
+    @newsletters = Newsletter.order(published_on: :desc).limit(4)
   end
 
   def set_case_studies

app/controllers/mailchimp_signups_controller.rb

@@ -1,49 +1,54 @@
 class MailchimpSignupsController < ApplicationController
+  include NewsletterSubscriber
+
   skip_before_action :authenticate_user!
 
   def new
-    @config = mailchimp_signup_params(params)
-    @onboarding_complete = params[:onboarding_complete]
-    @list = mailchimp_api.list_with_interests
+    audience_manager.list # load to ensure config is set
+    @email_types = list_of_email_types
+    @contact = populate_contact_for_form(current_user, params)
   rescue => e
-    flash[:error] = 'Mailchimp API is not configured'
     Rails.logger.error "Mailchimp API is not configured - #{e.message}"
     Rollbar.error(e)
-  end
-
-  def index
+    raise e
   end
 
   def create
-    list_id = params[:list_id]
-    @config = mailchimp_signup_params(params)
-    if @config.valid?
-      begin
-        mailchimp_api.subscribe(list_id, @config)
-        redirect_to mailchimp_signups_path and return
-      rescue MailchimpApi::Error => e
-        flash[:error] = e.message
-      end
+    if params[:contact_source]
+      @contact = create_contact_from_user(current_user, sign_up_params)
     else
-      flash[:error] = @config.errors.full_messages.join(', ')
+      @contact = create_contact(sign_up_params)
     end
-
-    @list = mailchimp_api.list_with_interests
+    resp = subscribe_contact(@contact)
+    if resp
+      redirect_to subscribed_mailchimp_signups_path and return
+    end
+    @email_types = list_of_email_types
     render :new
   end
 
   private
 
-  def mailchimp_api
-    @mailchimp_api ||= MailchimpApi.new
+  def populate_contact_for_form(user, params)
+    if user
+      contact = Mailchimp::Contact.new(user.email, user.name)
+      contact.school = user&.school&.name
+      contact
+    else
+      Mailchimp::Contact.new(params[:email_address], nil)
+    end
+  end
+
+  def create_contact(sign_up_params)
+    existing_user = User.find_by_email(sign_up_params[:email_address].downcase)
+    if existing_user
+      create_contact_from_user(existing_user, sign_up_params)
+    else
+      Mailchimp::Contact.from_params(sign_up_params)
+    end
   end
 
-  def mailchimp_signup_params(params)
-    MailchimpSignupParams.new(
-      email_address: params[:email_address],
-      tags: params[:tags],
-      interests: params[:interests],
-      merge_fields: params[:merge_fields]
-    )
+  def sign_up_params
+    params.permit(:email_address, :name, :school, interests: {})
   end
 end

app/controllers/passwords_controller.rb

@@ -1,5 +1,6 @@
 class PasswordsController < Devise::PasswordsController
   before_action :set_confirmed
+  before_action :set_email_types
 
   include AlertContactCreator
   include NewsletterSubscriber
@@ -9,7 +10,7 @@ def edit
     # the resource is NOT the actual user - have to find it ourselves
     user = User.with_reset_password_token(params[:reset_password_token])
     if user
-      @allow_newsletters = allow_newletters?(user)
+      @show_newletter_options = show_newletter_options?(user)
       @allow_alerts = allow_alerts?(user)
       @subscribe_alerts = true
       @subscribe_newsletters = true
@@ -20,30 +21,43 @@ def edit
   end
 
   def update
-    super do |resource|
-      @allow_newsletters = allow_newletters?(resource)
-      @allow_alerts = allow_alerts?(resource)
+    super do |user|
+      @show_newletter_options = show_newletter_options?(user)
+      @allow_alerts = allow_alerts?(user)
       @subscribe_alerts = auto_create_alert_contact?
-      @subscribe_newsletters = auto_subscribe_newsletter?
-      resource.preferred_locale = resource_params[:preferred_locale] if resource_params[:preferred_locale]
-      if resource.errors.empty?
-        create_or_update_alert_contact(resource.school, resource) if @subscribe_alerts
-        subscribe_newsletter(resource) if @subscribe_newsletters
+      @subscribe_newsletters = subscribe_newsletter?
+      user.preferred_locale = resource_params[:preferred_locale] if resource_params[:preferred_locale]
+      if user.errors.empty?
+        create_or_update_alert_contact(user.school, resource) if @subscribe_alerts
+        subscribe_newsletter(user, params.permit(interests: {})) if @subscribe_newsletters
       end
     end
   end
 
   private
 
+  def set_email_types
+    @email_types = list_of_email_types
+  end
+
   def set_confirmed
     @confirmed = (params[:confirmed] == 'true')
   end
 
-  def allow_newletters?(user)
+  def subscribe_newsletter?
+    params[:user] && params[:interests]&.value?('true')
+  end
+
+  def show_newletter_options?(user)
     @confirmed && user.present? && (user.school.present? || user.school_group.present?)
   end
 
   def allow_alerts?(user)
     @confirmed && user.present? && user.school.present?
   end
+
+  def subscribe_newsletter(user, sign_up_params)
+    contact = create_contact_from_user(user, sign_up_params)
+    subscribe_contact(contact, show_errors: false)
+  end
 end

app/controllers/pupils/public_displays_controller.rb → app/controllers/pupils/digital_signage_controller.rb

@@ -1,18 +1,20 @@
 module Pupils
-  class PublicDisplaysController < ApplicationController
+  class DigitalSignageController < ApplicationController
     load_resource :school
 
     include SchoolAggregation
     include ActionView::Helpers::TagHelper
     include ActionView::Context
 
     skip_before_action :authenticate_user!
-    before_action :set_fuel_type, except: :index
+    before_action :set_fuel_type
     before_action :check_fuel_type, only: :charts
-    before_action :set_analysis_dates, except: :index
+    before_action :set_analysis_dates
     after_action :allow_iframe, only: [:equivalences, :charts]
 
-    layout 'public_displays'
+    layout 'digital_signage'
+
+    CHART_TYPES = %i[last-week out-of-hours].freeze
 
     rescue_from StandardError do |exception|
       Rollbar.error(exception, school: @school.name, school_id: @school.id)
@@ -23,10 +25,6 @@ class PublicDisplaysController < ApplicationController
       end
     end
 
-    def index
-      render 'index', layout: 'application'
-    end
-
     def equivalences
       meter_types = case @fuel_type
                     when :electricity
@@ -42,18 +40,18 @@ def charts
       raise 'Not data enabled' unless @school.data_enabled?
       @chart_type = params.require(:chart_type).to_sym
 
-      # avoid showing stale date on weekly comparion chart, issue temporary redirect
+      # avoid showing stale date on weekly comparison chart, issue temporary redirect
       # ensures pages don't break in case of lagging data or a meter fault
       if lagging_data?(@chart_type)
-        redirect_to pupils_school_public_displays_equivalences_path(@school, @fuel_type) and return
+        redirect_to pupils_school_digital_signage_equivalences_path(@school, @fuel_type) and return
       end
       @chart = find_chart(@fuel_type, @chart_type)
     end
 
     private
 
     def lagging_data?(chart_type)
-      return false if chart_type == :out_of_hours
+      return false if chart_type == :"out-of-hours"
       (Time.zone.today - @analysis_dates.end_date) > 30
     end
 
@@ -96,14 +94,14 @@ def find_chart(fuel_type, chart_type)
       case fuel_type
       when :electricity
         case chart_type
-        when :out_of_hours
+        when :"out-of-hours"
           :daytype_breakdown_electricity_tolerant
         else
           :public_displays_electricity_weekly_comparison
         end
       when :gas
         case chart_type
-        when :out_of_hours
+        when :"out-of-hours"
           :daytype_breakdown_gas_tolerant
         else
           :public_displays_gas_weekly_comparison