From d852f98e4fce11613485104df90212671f09f095 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 23 Feb 2017 20:23:33 -0600 Subject: [PATCH] /dmails: remove unused `search[owner_id]` param. /dmails is restricted to viewing dmails for CurrentUser only (due to Dmail.visible in the index action). Remove owner_id from subnavbar links in /dmails, and don't support it in /dmails?search[owner_id], since it doesn't actually do anything. Also removes related dead methods and fixes tests that didn't test owner_id properly. --- app/helpers/dmails_helper.rb | 20 ++++++++++++++++---- app/models/dmail.rb | 16 ---------------- app/views/dmails/_secondary_links.html.erb | 6 +++--- test/factories/post_disapproval.rb | 6 ++++++ test/functional/dmails_controller_test.rb | 6 +++--- 5 files changed, 28 insertions(+), 26 deletions(-) create mode 100644 test/factories/post_disapproval.rb diff --git a/app/helpers/dmails_helper.rb b/app/helpers/dmails_helper.rb index 869a21820..3e700b95d 100644 --- a/app/helpers/dmails_helper.rb +++ b/app/helpers/dmails_helper.rb @@ -2,11 +2,23 @@ module DmailsHelper def dmails_current_folder_path case cookies[:dmail_folder] when "sent" - dmails_path(:search => {:owner_id => CurrentUser.id, :from_id => CurrentUser.id}, :folder => "sent") - when "all" - dmails_path(:search => {:owner_id => CurrentUser.id}, :folder => "all") + sent_dmails_path + when "received" + received_dmails_path else - dmails_path(:search => {:owner_id => CurrentUser.id, :to_id => CurrentUser.id}, :folder => "received") + all_dmails_path end end + + def all_dmails_path(params = {}) + dmails_path(folder: "all", **params) + end + + def sent_dmails_path(params = {}) + dmails_path(search: {from_id: CurrentUser.id}, folder: "sent", **params) + end + + def received_dmails_path(params = {}) + dmails_path(search: {to_id: CurrentUser.id}, folder: "received", **params) + end end diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 4a235a830..b60b207f5 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -86,18 +86,6 @@ class Dmail < ActiveRecord::Base end module SearchMethods - def for(user) - where("owner_id = ?", user) - end - - def inbox - where("to_id = owner_id") - end - - def sent - where("from_id = owner_id") - end - def active where("is_deleted = ?", false) end @@ -139,10 +127,6 @@ class Dmail < ActiveRecord::Base q = q.search_message(params[:message_matches]) end - if params[:owner_id].present? - q = q.for(params[:owner_id].to_i) - end - if params[:to_name].present? q = q.to_name_matches(params[:to_name]) end diff --git a/app/views/dmails/_secondary_links.html.erb b/app/views/dmails/_secondary_links.html.erb index 7b7206f31..9ea0bbe68 100644 --- a/app/views/dmails/_secondary_links.html.erb +++ b/app/views/dmails/_secondary_links.html.erb @@ -1,9 +1,9 @@ <% content_for(:secondary_links) do %>
  • <%= render "quick_search" %>
    • -
  • <%= link_to "All", dmails_path(:search => {:owner_id => CurrentUser.id}, :folder => "all", :set_default_folder => true) %>
    • -
  • <%= link_to "Received", dmails_path(:search => {:owner_id => CurrentUser.id, :to_id => CurrentUser.id}, :folder => "received", :set_default_folder => true) %>
    • -
  • <%= link_to "Sent", dmails_path(:search => {:owner_id => CurrentUser.id, :from_id => CurrentUser.id}, :folder => "sent", :set_default_folder => true) %>
    • +
  • <%= link_to "All", all_dmails_path(set_default_folder: true) %>
    • +
  • <%= link_to "Received", received_dmails_path(set_default_folder: true) %>
    • +
  • <%= link_to "Sent", sent_dmails_path(set_default_folder: true) %>
  • <%= link_to "New", new_dmail_path %>
  • <%= link_to "Search", search_dmails_path %>
  • <%= link_to "Mark all as read", {:controller => "dmails", :action => "mark_all_as_read"}, :method => :post, :remote => true %>
    • diff --git a/test/factories/post_disapproval.rb b/test/factories/post_disapproval.rb new file mode 100644 index 000000000..2f81d8783 --- /dev/null +++ b/test/factories/post_disapproval.rb @@ -0,0 +1,6 @@ +FactoryGirl.define do + factory(:post_disapproval) do + reason { %w(breaks_rules poor_quality disinterest).sample } + message { FFaker::Lorem.sentence } + end +end diff --git a/test/functional/dmails_controller_test.rb b/test/functional/dmails_controller_test.rb index a89f1764c..a697a0b75 100644 --- a/test/functional/dmails_controller_test.rb +++ b/test/functional/dmails_controller_test.rb @@ -48,17 +48,17 @@ class DmailsControllerTest < ActionController::TestCase context "index action" do should "show dmails owned by the current user" do - get :index, {:owner_id_equals => @dmail.owner_id, :folder => "sent"}, {:user_id => @dmail.owner_id} + get :index, {:search => {:owner_id => @dmail.owner_id, :folder => "sent"}}, {:user_id => @dmail.owner_id} assert_response :success assert_equal(1, assigns[:dmails].size) - get :index, {:owner_id_equals => @dmail.owner_id, :folder => "received"}, {:user_id => @dmail.owner_id} + get :index, {:search => {:owner_id => @dmail.owner_id, :folder => "received"}}, {:user_id => @dmail.owner_id} assert_response :success assert_equal(1, assigns[:dmails].size) end should "not show dmails not owned by the current user" do - get :index, {:owner_id_equals => @dmail.owner_id}, {:user_id => @unrelated_user.id} + get :index, {:search => {:owner_id => @dmail.owner_id}}, {:user_id => @unrelated_user.id} assert_response :success assert_equal(0, assigns[:dmails].size) end