Remove if_user and similar methods

Rather than hiding features based on CSS, just avoid including them
in the output. Fixes #1862

app/assets/stylesheets/common.scss

@@ -1668,10 +1668,6 @@ tr.turn:hover {
   }
 }
 
-.content-heading .hide_unless_logged_in { // hacky selector, better to just add a new class to this div
-  display: inline;
-}
-
 .pagination {
   padding-top: $lineheight;
 }

app/helpers/application_helper.rb

@@ -17,44 +17,6 @@ module ApplicationHelper
     link_to(image_tag("RSS.png", :size => "16x16", :border => 0), Hash[*args], :class => "rsssmall")
   end
 
-  def style_rules
-    css = ""
-
-    css << ".hidden { display: none !important }"
-    css << ".hide_unless_logged_in { display: none !important }" unless current_user
-    css << ".hide_if_logged_in { display: none !important }" if current_user
-    css << ".hide_if_user_#{current_user.id} { display: none !important }" if current_user
-    css << ".show_if_user_#{current_user.id} { display: inline !important }" if current_user
-    css << ".hide_unless_administrator { display: none !important }" unless current_user && current_user.administrator?
-    css << ".hide_unless_moderator { display: none !important }" unless current_user && current_user.moderator?
-
-    content_tag(:style, css, :type => "text/css")
-  end
-
-  def if_logged_in(tag = :div, &block)
-    content_tag(tag, capture(&block), :class => "hide_unless_logged_in")
-  end
-
-  def if_not_logged_in(tag = :div, &block)
-    content_tag(tag, capture(&block), :class => "hide_if_logged_in")
-  end
-
-  def if_user(user, tag = :div, &block)
-    content_tag(tag, capture(&block), :class => "hidden show_if_user_#{user.id}") if user
-  end
-
-  def unless_user(user, tag = :div, &block)
-    if user
-      content_tag(tag, capture(&block), :class => "hide_if_user_#{user.id}")
-    else
-      content_tag(tag, capture(&block))
-    end
-  end
-
-  def if_administrator(tag = :div, &block)
-    content_tag(tag, capture(&block), :class => "hide_unless_administrator")
-  end
-
   def richtext_area(object_name, method, options = {})
     id = "#{object_name}_#{method}"
     type = options.delete(:format) || "markdown"

app/views/browse/changeset.html.erb

@@ -13,15 +13,17 @@
 
   <h4 class="comments-header"><%= t('.discussion') %></h4>
 
+  <% if current_user %>
     <div class="buttons clearfix subscribe-buttons">
-    <form action="#" class="hide_unless_logged_in">
-      <% if current_user and @changeset.subscribers.exists?(current_user.id) %>
+      <form action="#">
+        <% if @changeset.subscribers.exists?(current_user.id) %>
           <input class="action-button" type="submit" name="unsubscribe" value="<%= t('javascripts.changesets.show.unsubscribe') %>" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>" />
         <% else %>
           <input class="action-button" type="submit" name="subscribe" value="<%= t('javascripts.changesets.show.subscribe') %>" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>" />
         <% end %>
       </form>
     </div>
+  <% end %>
 
   <div class="clearfix"></div>
 
@@ -59,22 +61,26 @@
     </div>
   <% end %>
 
-  <div class="notice hide_if_logged_in">
+  <% unless current_user %>
+    <div class="notice">
       <%= link_to(t(".join_discussion"), :controller => 'user', :action => 'login', :referer => request.fullpath) %>
     </div>
+  <% end %>
 
+  <% if current_user %>
     <% unless @changeset.is_open? %>
-    <form action="#" class="hide_unless_logged_in">
+      <form action="#">
         <textarea class="comment" name="text" cols="40" rows="5"></textarea>
         <div class="buttons clearfix">
           <input type="submit" name="comment" value="<%= t('javascripts.changesets.show.comment') %>" data-changeset-id="<%= @changeset.id %>" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled="1"/>
         </div>
       </form>
     <% else %>
-    <div class="notice hide_unless_logged_in">
+      <div class="notice">
       <%= t('.still_open') %>
       </div>
     <% end %>
+  <% end %>
 
   <% unless @ways.empty? %>
     <h4>

app/views/diary_entry/_diary_comment.html.erb

@@ -2,7 +2,9 @@
   <%= user_thumbnail diary_comment.user %>
   <p class="deemphasize comment-heading" id="comment<%= diary_comment.id %>"><%= raw(t('.comment_from', :link_user => (link_to h(diary_comment.user.display_name), user_path(diary_comment.user)), :comment_created_at => link_to(l(diary_comment.created_at, :format => :friendly), :anchor => "comment#{diary_comment.id}"))) %></p>
   <div class="richtext"><%= diary_comment.body.to_html %></div>
-  <%= if_administrator(:span) do %>
+  <% if current_user && current_user.administrator? %>
+    <span>
       <%= link_to t('.hide_link'), hide_diary_comment_path(:display_name => diary_comment.diary_entry.user.display_name, :id => diary_comment.diary_entry.id, :comment => diary_comment.id), :method => :post, :data=> { :confirm => t('.confirm') } %>
+    </span>
   <% end %>
 </div>

app/views/diary_entry/_diary_entry.html.erb

@@ -27,12 +27,12 @@
       <li><%= link_to t('.comment_count', :count => diary_entry.visible_comments.count), :action => 'view', :display_name => diary_entry.user.display_name, :id => diary_entry.id, :anchor => 'comments' %></li>
     <% end %>
 
-    <%= if_user(diary_entry.user, :li) do %>
-      <%= link_to t('.edit_link'), :action => 'edit', :display_name => diary_entry.user.display_name, :id => diary_entry.id %>
+    <% if current_user && current_user == diary_entry.user %>
+      <li><%= link_to t('.edit_link'), :action => 'edit', :display_name => diary_entry.user.display_name, :id => diary_entry.id %></li>
     <% end %>
 
-    <%= if_administrator(:li) do %>
-      <%= link_to t('.hide_link'), hide_diary_entry_path(:display_name => diary_entry.user.display_name, :id => diary_entry.id), :method => :post, :data => { :confirm => t('.confirm') } %>
+    <% if current_user && current_user.administrator? %>
+      <li><%= link_to t('.hide_link'), hide_diary_entry_path(:display_name => diary_entry.user.display_name, :id => diary_entry.id), :method => :post, :data => { :confirm => t('.confirm') } %></li>
     <% end %>
   </ul>
 </div>

app/views/diary_entry/list.html.erb

@@ -11,12 +11,16 @@
       <% end -%>
 
       <% if @user %>
-        <%= if_user(@user) do %>
+        <% if @user == current_user %>
+          <div>
             <li><%= link_to image_tag("new.png", :class => "small_icon", :border=>0) + t('.new'), {:controller => 'diary_entry', :action => 'new'}, {:title => t('.new_title')} %></li>
+          </div>
         <% end %>
       <% else %>
-        <%= if_logged_in do %>
+        <% if current_user %>
+          <div>
             <li><%= link_to image_tag("new.png", :class => "small_icon", :border=>0) + t('.new'), {:controller => 'diary_entry', :action => 'new'}, {:title => t('.new_title')} %></li>
+          </div>
         <% end %>
       <% end %>
     </ul>

app/views/diary_entry/view.html.erb

@@ -12,7 +12,10 @@
 <div class='comments'>
 <%= render :partial => 'diary_comment', :collection => @entry.visible_comments %>
 </div>
-<%= if_logged_in(:div) do %>
+
+
+<div>
+  <% if current_user %>
     <h3 id="newcomment"><%= t '.leave_a_comment' %></h3>
 
     <%= error_messages_for 'diary_comment' %>
@@ -21,16 +24,15 @@
       <%= richtext_area :diary_comment, :body, :cols => 80, :rows => 15 %>
       <%= submit_tag t('.save_button') %>
     <% end %>
-  <% if current_user and @entry.subscribers.exists?(current_user.id) %>
+    <% if @entry.subscribers.exists?(current_user.id) %>
       <div class="diary-subscribe-buttons"><%= link_to t('javascripts.changesets.show.unsubscribe'), diary_entry_unsubscribe_path(:display_name => @entry.user.display_name, :id => @entry.id), :method => :post, :class => :button %></div>
-  <% elsif current_user %>
+    <% else %>
       <div class="diary-subscribe-buttons"><%= link_to t('javascripts.changesets.show.subscribe'), diary_entry_subscribe_path(:display_name => @entry.user.display_name, :id => @entry.id), :method => :post, :class => :button %></div>
     <% end %>
-<% end %>
-
-<%= if_not_logged_in(:div) do %>
+  <% else %>
     <h3 id="newcomment"><%= raw t(".login_to_leave_a_comment", :login_link => link_to(t(".login"), :controller => 'user', :action => 'login', :referer => request.fullpath)) %></h3>
-<% end %>
+  <% end %>
+</div>
 
 <% content_for :auto_discovery_link_tag do -%>
 <%= auto_discovery_link_tag :rss, :action => :rss, :display_name => @entry.user.display_name %>

app/views/layouts/_head.html.erb

@@ -33,7 +33,6 @@
   <% if flash[:piwik_goal] -%>
   <%= tag("meta", :name => "piwik-goal", :content => flash[:piwik_goal]) %>
   <% end -%>
-  <%= style_rules %>
   <%= yield :head %>
   <%= yield :auto_discovery_link_tag %>
   <%= csrf_meta_tag %>

app/views/traces/list.html.erb

@@ -11,8 +11,8 @@
       <% if @display_name %>
         <li><%= link_to t('.see_all_traces'), :controller => 'traces', :action => 'list', :display_name => nil, :tag => nil, :page => nil %></li>
       <% end %>
-      <%= unless_user(@target_user, :li) do %>
-        <%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %>
+      <% if current_user && current_user != @target_user %>
+        <li><%= link_to t('.see_my_traces'), :action => 'mine', :tag => nil, :page => nil %></li>
       <% end %>
     <% end %>
   </ul>

app/views/traces/view.html.erb

@@ -56,8 +56,10 @@
 
 <% if current_user && (current_user==@trace.user || current_user.administrator? || current_user.moderator?)%>
   <div class="buttons">
-    <%= if_user(@trace.user) do %>
+    <% if current_user == @trace.user %>
+      <div>
         <%= button_to t('.edit_track'), trace_edit_path(@trace) %>
+      </div>
     <% end %>
     <%= button_to t('.delete_track'), { :controller => 'traces', :action => 'delete', :id => @trace.id }, :data => { :confirm => t('.confirm_delete') } %>
   </div>

test/controllers/diary_entry_controller_test.rb

@@ -349,9 +349,7 @@ class DiaryEntryControllerTest < ActionController::TestCase
       assert_select "p", :text => /#{new_body}/, :count => 1
       assert_select "abbr[class=geo][title='#{number_with_precision(new_latitude, :precision => 4)}; #{number_with_precision(new_longitude, :precision => 4)}']", :count => 1
       # As we're not logged in, check that you cannot edit
-      assert_select "li[class='hidden show_if_user_#{entry.user.id}']", :count => 1 do
-        assert_select "a[href='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit']", :text => "Edit this entry", :count => 1
-      end
+      assert_select "a[href='/user/#{ERB::Util.u(entry.user.display_name)}/diary/#{entry.id}/edit']", false
     end
   end
 

test/helpers/application_helper_test.rb

@@ -47,108 +47,6 @@ class ApplicationHelperTest < ActionView::TestCase
     assert_dom_equal "<a class=\"rsssmall\" href=\"/history/feed\"><img alt=\"Rss\" border=\"0\" height=\"16\" src=\"/images/RSS.png\" width=\"16\" /></a>", link
   end
 
-  def test_style_rules
-    self.current_user = nil
-
-    css = style_rules
-    assert_match /\.hidden /, css
-    assert_match /\.hide_unless_logged_in /, css
-    assert_no_match /\.hide_if_logged_in /, css
-    assert_no_match /\.hide_if_user_/, css
-    assert_no_match /\.show_if_user_/, css
-    assert_match /\.hide_unless_administrator /, css
-    assert_match /\.hide_unless_moderator /, css
-
-    self.current_user = create(:user)
-
-    css = style_rules
-    assert_match /\.hidden /, css
-    assert_no_match /\.hide_unless_logged_in /, css
-    assert_match /\.hide_if_logged_in /, css
-    assert_match /\.hide_if_user_#{current_user.id} /, css
-    assert_match /\.show_if_user_#{current_user.id} /, css
-    assert_match /\.hide_unless_administrator /, css
-    assert_match /\.hide_unless_moderator /, css
-
-    self.current_user = create(:moderator_user)
-
-    css = style_rules
-    assert_match /\.hidden /, css
-    assert_no_match /\.hide_unless_logged_in /, css
-    assert_match /\.hide_if_logged_in /, css
-    assert_match /\.hide_if_user_#{current_user.id} /, css
-    assert_match /\.show_if_user_#{current_user.id} /, css
-    assert_match /\.hide_unless_administrator /, css
-    assert_no_match /\.hide_unless_moderator /, css
-
-    self.current_user = create(:administrator_user)
-
-    css = style_rules
-    assert_match /\.hidden /, css
-    assert_no_match /\.hide_unless_logged_in /, css
-    assert_match /\.hide_if_logged_in /, css
-    assert_match /\.hide_if_user_#{current_user.id} /, css
-    assert_match /\.show_if_user_#{current_user.id} /, css
-    assert_no_match /\.hide_unless_administrator /, css
-    assert_match /\.hide_unless_moderator /, css
-  end
-
-  def test_if_logged_in
-    html = if_logged_in { "Test 1" }
-    assert_dom_equal "<div class=\"hide_unless_logged_in\">Test 1</div>", html
-
-    html = if_logged_in(:span) { "Test 2" }
-    assert_dom_equal "<span class=\"hide_unless_logged_in\">Test 2</span>", html
-  end
-
-  def test_if_not_logged_in
-    html = if_not_logged_in { "Test 1" }
-    assert_dom_equal "<div class=\"hide_if_logged_in\">Test 1</div>", html
-
-    html = if_not_logged_in(:span) { "Test 2" }
-    assert_dom_equal "<span class=\"hide_if_logged_in\">Test 2</span>", html
-  end
-
-  def test_if_user
-    user = create(:user)
-
-    html = if_user(user) { "Test 1" }
-    assert_dom_equal "<div class=\"hidden show_if_user_#{user.id}\">Test 1</div>", html
-
-    html = if_user(user, :span) { "Test 2" }
-    assert_dom_equal "<span class=\"hidden show_if_user_#{user.id}\">Test 2</span>", html
-
-    html = if_user(nil) { "Test 3" }
-    assert_nil html
-
-    html = if_user(nil, :span) { "Test 4" }
-    assert_nil html
-  end
-
-  def test_unless_user
-    user = create(:user)
-
-    html = unless_user(user) { "Test 1" }
-    assert_dom_equal "<div class=\"hide_if_user_#{user.id}\">Test 1</div>", html
-
-    html = unless_user(user, :span) { "Test 2" }
-    assert_dom_equal "<span class=\"hide_if_user_#{user.id}\">Test 2</span>", html
-
-    html = unless_user(nil) { "Test 3" }
-    assert_dom_equal "<div>Test 3</div>", html
-
-    html = unless_user(nil, :span) { "Test 4" }
-    assert_dom_equal "<span>Test 4</span>", html
-  end
-
-  def test_if_administrator
-    html = if_administrator { "Test 1" }
-    assert_dom_equal "<div class=\"hide_unless_administrator\">Test 1</div>", html
-
-    html = if_administrator(:span) { "Test 2" }
-    assert_dom_equal "<span class=\"hide_unless_administrator\">Test 2</span>", html
-  end
-
   def test_richtext_area
     html = richtext_area(:message, :body, :cols => 40, :rows => 20)
     assert_not_nil html

test/integration/user_changeset_comments_test.rb

@@ -12,7 +12,9 @@ class UserChangesetCommentsTest < ActionDispatch::IntegrationTest
       assert_select "div#sidebar" do
         assert_select "div#sidebar_content" do
           assert_select "div.browse-section" do
-            assert_select "div.notice.hide_if_logged_in"
+            assert_select "div.notice" do
+              assert_select "a[href='/login?referer=%2Fchangeset%2F#{changeset.id}']", :text => I18n.t("browse.changeset.join_discussion"), :count => 1
+            end
           end
         end
       end