Merge branch 'request-page-pro-upsell' into develop

app/assets/stylesheets/responsive/_sidebar_layout.scss

@@ -1,7 +1,6 @@
 /* Display of sidebars */
 .sidebar {
   h2 {
-    margin-top: 3.5em;
     margin-bottom: 0.875em;
   }
   .link_button_green {
@@ -10,8 +9,24 @@
   .report-this-request {
     width: 100%;
   }
+
+  .sidebar__section {
+    margin-bottom: 3.5em;
+  }
+}
+
+.new-request-cta {
+  .new-request__title {
+    margin-top: 0;
+  }
+}
+
+.act_link {
+  margin-bottom: 0.5em;
 }
 
+/* TODO: Not sure this is still used – we use the request header action bar for
+   tracks */
 /* #track-request .feed_link is here to override specificity in the request sidebar,
    normally .feed_link should be enough */
 #track-request .feed_link,
@@ -22,12 +37,7 @@
   }
 }
 
-.new-request-cta {
-  .new-request__title {
-    margin-top: 0;
-  }
-}
-
+/* TODO: Why is this in the layout file for the request page sidebar? */
 .authority__body__sidebar {
   .feed_link_sidebar {
     .track-request-action {
@@ -36,12 +46,10 @@
   }
 }
 
+/* TODO: I think this is only used in _request_listing_short_via_event, which is
+   only used in search_ahead */
 .request_short_listing {
   time {
     display: block;
   }
 }
-
-.act_link {
-  margin-bottom: 0.5em;
-}

app/helpers/application_helper.rb

@@ -213,4 +213,20 @@ def site_wide_announcement
       session[:announcement_dismissals]
     ).first
   end
+
+  def show_pro_upsell?(user)
+    feature_enabled?(:alaveteli_pro) && (!user || user && !user.is_pro?)
+  end
+
+  def pro_upsell_text
+    pro_link = link_to(account_request_index_path) do
+      AlaveteliConfiguration.pro_site_name
+    end
+
+    pro_site_name_link = content_tag(:strong, pro_link)
+
+    _('{{pro_site_name}} is a powerful, fully-featured FOI toolkit for ' \
+      'journalists.',
+      pro_site_name: pro_site_name_link)
+  end
 end

app/views/request/_batch.html.erb

@@ -1,5 +1,5 @@
 <% if @info_request.info_request_batch %>
-  <div class="sidebar__part-of-batch">
+  <div class="sidebar__section sidebar__part-of-batch">
     <h2><%= _('More requests in this batch') %></h2>
     <p>
       <%= _('This request is part of ' \

app/views/request/_sidebar.html.erb

@@ -1,49 +1,54 @@
 <div id="right_column" class="sidebar right_column" role="complementary">
-  <div class="new-request-cta">
+  <div class="sidebar__section new-request-cta">
     <%= render :partial => "general/new_request" %>
   </div>
 
+  <%= render partial: 'sidebar_pro_upsell' %>
+
   <% if (feature_enabled?(:alaveteli_pro) &&
          can?(:create_embargo, @info_request)) %>
     <%= render partial: "alaveteli_pro/info_requests/embargo_form",
                locals: { info_request: @info_request } %>
   <% end %>
 
-  <div class="request__special-status">
   <% if @info_request.attention_requested %>
-    <% if @info_request.prominence(:decorate => true).is_hidden? %>
-      <h2><%= _('Request hidden') %></h2>
-      <p>
-        <%= _("This request has prominence 'hidden'. You can only see it " \
-              "because you are logged in as a super user.") %>
-      </p>
-    <% elsif @info_request.prominence(:decorate => true).is_requester_only? %>
-      <h2><%= _('Request hidden') %></h2>
-      <%# The eccentric formatting of the following string is in order that it be identical
-          to the corresponding string in request/show.html.erb %>
-      <p><%= _('This request is hidden, so that only you the requester ' \
-                  'can see it. Please <a href="{{url}}">contact us</a> ' \
-                  'if you are not sure why.',
-               :url => help_requesting_path.html_safe) %></p>
-    <% elsif @info_request.described_state != "attention_requested" %>
-      <h2><%= _('Offensive? Unsuitable?') %></h2>
-      <p><%= _('This request has been marked for review by the site ' \
-               'administrators, who have not hidden it at this time. ' \
-               'If you believe it should be hidden, please ' \
-               '<a href="{{url}}">contact us</a>.',
-              :url => help_requesting_path.html_safe) %></p>
-    <% end %>
+    <div class="sidebar__section request__special-status">
+      <% if @info_request.prominence(:decorate => true).is_hidden? %>
+        <h2><%= _('Request hidden') %></h2>
+        <p>
+          <%= _("This request has prominence 'hidden'. You can only see it " \
+                "because you are logged in as a super user.") %>
+        </p>
+      <% elsif @info_request.prominence(:decorate => true).is_requester_only? %>
+        <h2><%= _('Request hidden') %></h2>
+        <%# The eccentric formatting of the following string is in order that it
+            be identical to the corresponding string in request/show.html.erb %>
+        <p><%= _('This request is hidden, so that only you the requester ' \
+                    'can see it. Please <a href="{{url}}">contact us</a> ' \
+                    'if you are not sure why.',
+                 :url => help_requesting_path.html_safe) %></p>
+      <% elsif @info_request.described_state != "attention_requested" %>
+        <h2><%= _('Offensive? Unsuitable?') %></h2>
+        <p><%= _('This request has been marked for review by the site ' \
+                 'administrators, who have not hidden it at this time. ' \
+                 'If you believe it should be hidden, please ' \
+                 '<a href="{{url}}">contact us</a>.',
+                :url => help_requesting_path.html_safe) %></p>
+      <% end %>
+    </div>
   <% end %>
+
+  <div class="sidebar__section next-actions">
+    <%= render :partial => 'request/act' %>
+    <%= render :partial => 'request/next_actions' %>
   </div>
 
-  <%= render :partial => 'request/act' %>
-  <%= render :partial => 'request/next_actions' %>
   <%= render :partial => 'request/batch',
              :locals => { :info_request => @info_request } %>
 
   <% similar_requests, similar_more = @info_request.similar_requests %>
   <% unless similar_requests.empty? %>
-    <div class="sidebar__similar-requests">
+    <div class="sidebar__section sidebar__similar-requests">
       <h2><%= _('Requests like this')%></h2>
 
       <% utm_params = { :utm_source => site_name.downcase,
@@ -59,15 +64,20 @@
 
       <% if similar_more %>
         <p class="sidebar_similar_requests__more-link">
-          <%= link_to similar_request_path(@info_request.url_title, utm_params) do %>
-            <%= _("More similar requests") %>
-          <% end %>
+          <% path = similar_request_path(@info_request.url_title, utm_params) %>
+          <%= link_to _("More similar requests"), path %>
         </p>
       <% end %>
     </div>
   <% end %>
 
-  <!-- this link with this wording is here for legal reasons, discuss with
-    board and our lawyer before changing or removing it -->
-  <p class="copyright-notice"><small><%= link_to _('Are you the owner of any commercial copyright on this page?'), help_officers_path(:anchor => "copyright") %></small></p>
+  <div class="sidebar__section">
+    <!-- this link with this wording is here for legal reasons, discuss with
+         board and our lawyer before changing or removing it -->
+    <p class="copyright-notice">
+      <small><%= link_to _('Are you the owner of any commercial copyright ' \
+                           'on this page?'),
+                         help_officers_path(anchor: 'copyright') %></small>
+    </p>
+  </div>
 </div>

app/views/request/_sidebar_pro_upsell.html.erb

@@ -0,0 +1,5 @@
+<% if show_pro_upsell?(@user) %>
+  <div class="sidebar__section pro-upsell">
+    <%= pro_upsell_text %>
+  </div>
+<% end %>

spec/helpers/application_helper_spec.rb

@@ -126,6 +126,41 @@
 
   end
 
+  describe '#show_pro_upsell?' do
+    subject { show_pro_upsell?(user) }
+
+    context 'when the user is not logged in', feature: :alaveteli_pro do
+      let(:user) { nil }
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when a regular user is logged in', feature: :alaveteli_pro do
+      let(:user) { FactoryBot.create(:user) }
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when a pro user is logged in', feature: :alaveteli_pro do
+      let(:user) { FactoryBot.create(:pro_user) }
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when pro is disabled' do
+      let(:user) { FactoryBot.create(:user) }
+      it { with_feature_disabled(:alaveteli_pro) { is_expected.to eq(false) } }
+    end
+  end
+
+  describe '#pro_upsell_text' do
+    subject { pro_upsell_text }
+
+    let(:expected) do
+      '<strong><a href="/pro">Alaveteli Professional</a></strong> is a ' \
+      'powerful, fully-featured FOI toolkit for journalists.'
+    end
+
+    it { is_expected.to eq(expected) }
+  end
+
   describe 'when creating an event description' do
 
     it 'should generate a description for a request' do