From: Tom Hughes Date: Thu, 21 Dec 2023 11:01:48 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/4428' X-Git-Tag: live~1300 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/a7ba87340d91a6eeef096783b04a9b0a963bfa99?hp=9d13066c4cebecf3b2f5856d76db7ce32432b4d8 Merge remote-tracking branch 'upstream/pull/4428' --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bd2f92309..e6772b8a4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -61,7 +61,7 @@ Metrics/BlockNesting: # Offense count: 26 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 297 + Max: 299 # Offense count: 59 # Configuration parameters: AllowedMethods, AllowedPatterns. diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index f9348f68e..b9da5d08a 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -47,13 +47,14 @@ class Ability can [:show], :dashboard can [:new, :create, :edit, :update, :comment, :subscribe, :unsubscribe], DiaryEntry can [:make_friend, :remove_friend], Friendship - can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message + can [:new, :create, :reply, :show, :inbox, :outbox, :muted, :mark, :unmute, :destroy], Message can [:close, :reopen], Note can [:show, :edit, :update], :preference can [:edit, :update], :profile can [:new, :create], Report can [:mine, :new, :create, :edit, :update, :destroy], Trace can [:account, :go_public], User + can [:index, :create, :destroy], UserMute if user.moderator? can [:hide, :unhide, :hidecomment, :unhidecomment], DiaryEntry diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index adb53b43b..2ca86fc02 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -47,7 +47,7 @@ class MessagesController < ApplicationController render :action => "new" elsif @message.save flash[:notice] = t ".message_sent" - UserMailer.message_notification(@message).deliver_later + UserMailer.message_notification(@message).deliver_later if @message.notify_recipient? redirect_to :action => :inbox else @title = t "messages.new.title" @@ -107,6 +107,13 @@ class MessagesController < ApplicationController @title = t ".title" end + # Display the list of muted messages received by the user. + def muted + @title = t ".title" + + redirect_to inbox_messages_path if current_user.muted_messages.none? + end + # Set the message as being read or unread. def mark @message = Message.where(:recipient => current_user).or(Message.where(:sender => current_user)).find(params[:message_id]) @@ -127,6 +134,23 @@ class MessagesController < ApplicationController render :action => "no_such_message", :status => :not_found end + # Moves message into Inbox by unsetting the muted-flag + def unmute + message = current_user.muted_messages.find(params[:message_id]) + + if message.unmute + flash[:notice] = t(".notice") + else + flash[:error] = t(".error") + end + + if current_user.muted_messages.none? + redirect_to inbox_messages_path + else + redirect_to muted_messages_path + end + end + private ## diff --git a/app/controllers/user_mutes_controller.rb b/app/controllers/user_mutes_controller.rb new file mode 100644 index 000000000..2068ab6a3 --- /dev/null +++ b/app/controllers/user_mutes_controller.rb @@ -0,0 +1,45 @@ +class UserMutesController < ApplicationController + include UserMethods + + layout "site" + + before_action :authorize_web + before_action :set_locale + + authorize_resource + + before_action :lookup_user, :only => [:create, :destroy] + before_action :check_database_readable + before_action :check_database_writable, :only => [:create, :destroy] + + def index + @muted_users = current_user.muted_users + @title = t ".title" + + redirect_to edit_account_path unless @muted_users.any? + end + + def create + user_mute = current_user.mutes.build(:subject => @user) + + if user_mute.save + flash[:notice] = t(".notice", :name => user_mute.subject.display_name) + else + flash[:error] = t(".error", :name => user_mute.subject.display_name, :full_message => user_mute.errors.full_messages.to_sentence.humanize) + end + + redirect_back_or_to user_mutes_path(current_user) + end + + def destroy + user_mute = current_user.mutes.find_by!(:subject => @user) + + if user_mute.destroy + flash[:notice] = t(".notice", :name => user_mute.subject.display_name) + else + flash[:error] = t(".error") + end + + redirect_back_or_to user_mutes_path(current_user) + end +end diff --git a/app/models/message.rb b/app/models/message.rb index 7c12769d3..665e2d721 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -12,6 +12,7 @@ # to_user_visible :boolean default(TRUE), not null # from_user_visible :boolean default(TRUE), not null # body_format :enum default("markdown"), not null +# muted :boolean default(FALSE), not null # # Indexes # @@ -32,6 +33,10 @@ class Message < ApplicationRecord validates :body, :sent_on, :presence => true validates :title, :body, :characters => true + scope :muted, -> { where(:muted => true) } + + before_create :set_muted + def self.from_mail(mail, from, to) if mail.multipart? if mail.text_part @@ -65,4 +70,18 @@ class Message < ApplicationRecord sha256 << id.to_s Base64.urlsafe_encode64(sha256.digest)[0, 8] end + + def notify_recipient? + !muted? + end + + def unmute + update(:muted => false) + end + + private + + def set_muted + self.muted ||= UserMute.active?(:owner => recipient, :subject => sender) + end end diff --git a/app/models/user.rb b/app/models/user.rb index 1942a25cc..5790d81e5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,9 +51,10 @@ class User < ApplicationRecord has_many :diary_comments, -> { order(:created_at => :desc) }, :inverse_of => :user has_many :diary_entry_subscriptions, :class_name => "DiaryEntrySubscription" has_many :diary_subscriptions, :through => :diary_entry_subscriptions, :source => :diary_entry - has_many :messages, -> { where(:to_user_visible => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :foreign_key => :to_user_id - has_many :new_messages, -> { where(:to_user_visible => true, :message_read => false).order(:sent_on => :desc) }, :class_name => "Message", :foreign_key => :to_user_id + has_many :messages, -> { where(:to_user_visible => true, :muted => false).order(:sent_on => :desc).preload(:sender, :recipient) }, :foreign_key => :to_user_id + has_many :new_messages, -> { where(:to_user_visible => true, :muted => false, :message_read => false).order(:sent_on => :desc) }, :class_name => "Message", :foreign_key => :to_user_id has_many :sent_messages, -> { where(:from_user_visible => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :class_name => "Message", :foreign_key => :from_user_id + has_many :muted_messages, -> { where(:to_user_visible => true, :muted => true).order(:sent_on => :desc).preload(:sender, :recipient) }, :class_name => "Message", :foreign_key => :to_user_id has_many :friendships, -> { joins(:befriendee).where(:users => { :status => %w[active confirmed] }) } has_many :friends, :through => :friendships, :source => :befriendee has_many :tokens, :class_name => "UserToken", :dependent => :destroy @@ -75,6 +76,9 @@ class User < ApplicationRecord has_many :blocks_created, :class_name => "UserBlock", :foreign_key => :creator_id, :inverse_of => :creator has_many :blocks_revoked, :class_name => "UserBlock", :foreign_key => :revoker_id, :inverse_of => :revoker + has_many :mutes, -> { order(:created_at => :desc) }, :class_name => "UserMute", :foreign_key => :owner_id, :inverse_of => :owner + has_many :muted_users, :through => :mutes, :source => :subject + has_many :roles, :class_name => "UserRole" has_many :issues, :class_name => "Issue", :foreign_key => :reported_user_id, :inverse_of => :reported_user diff --git a/app/models/user_mute.rb b/app/models/user_mute.rb new file mode 100644 index 000000000..9bee39b8d --- /dev/null +++ b/app/models/user_mute.rb @@ -0,0 +1,34 @@ +# == Schema Information +# +# Table name: user_mutes +# +# id :bigint(8) not null, primary key +# owner_id :bigint(8) not null +# subject_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_user_mutes_on_owner_id_and_subject_id (owner_id,subject_id) UNIQUE +# +# Foreign Keys +# +# fk_rails_... (owner_id => users.id) +# fk_rails_... (subject_id => users.id) +# +class UserMute < ApplicationRecord + belongs_to :owner, :class_name => "User" + belongs_to :subject, :class_name => "User" + + validates :subject, :uniqueness => { :scope => :owner_id, :message => :is_already_muted } + + def self.active?(owner:, subject:) + !subject.administrator? && + !subject.moderator? && + exists?( + :owner => owner, + :subject => subject + ) + end +end diff --git a/app/views/application/_settings_menu.html.erb b/app/views/application/_settings_menu.html.erb index 9ce9755a2..8477a11a0 100644 --- a/app/views/application/_settings_menu.html.erb +++ b/app/views/application/_settings_menu.html.erb @@ -14,5 +14,10 @@
  • <%= link_to t(".oauth2_authorizations"), oauth_authorized_applications_path, :class => "nav-link #{'active' if controller_name == 'oauth2_authorized_applications'}" %>
    • + <% if current_user.muted_users.any? %> +
  • + <%= link_to t(".muted_users"), user_mutes_path, :class => "nav-link #{'active' if controller_name == 'user_mutes'}" %> +
    • + <% end %> <% end %> diff --git a/app/views/messages/_heading.html.erb b/app/views/messages/_heading.html.erb new file mode 100644 index 000000000..90995ed88 --- /dev/null +++ b/app/views/messages/_heading.html.erb @@ -0,0 +1,18 @@ +<% content_for :heading_class, "pb-0" %> + +<% content_for :heading do %> +

    <%= t("users.show.my messages") %>

    + +<% end %> diff --git a/app/views/messages/_message_summary.html.erb b/app/views/messages/_message_summary.html.erb index b2a1bc268..4a552e83e 100644 --- a/app/views/messages/_message_summary.html.erb +++ b/app/views/messages/_message_summary.html.erb @@ -6,5 +6,8 @@ <%= button_to t(".unread_button"), message_mark_path(message_summary, :mark => "unread"), :remote => true, :class => "btn btn-sm btn-primary", :form => { :class => "inbox-mark-unread", :hidden => !message_summary.message_read? } %> <%= button_to t(".read_button"), message_mark_path(message_summary, :mark => "read"), :remote => true, :class => "btn btn-sm btn-primary", :form => { :class => "inbox-mark-read", :hidden => message_summary.message_read? } %> <%= button_to t(".destroy_button"), message_path(message_summary, :referer => request.fullpath), :method => :delete, :remote => true, :class => "btn btn-sm btn-danger", :form_class => "inbox-destroy" %> + <% if message_summary.muted? %> + <%= button_to t(".unmute_button"), message_unmute_path(message_summary), :method => :patch, :class => "btn btn-sm btn-secondary" %> + <% end %> diff --git a/app/views/messages/_messages_table.html.erb b/app/views/messages/_messages_table.html.erb new file mode 100644 index 000000000..2e3396232 --- /dev/null +++ b/app/views/messages/_messages_table.html.erb @@ -0,0 +1,13 @@ + + + + <% columns.each do |column| %> + + <% end %> + + + + + <%= render :partial => inner_partial, :collection => messages %> + +
    <%= t ".#{column}" %><%= t ".actions" %>
    diff --git a/app/views/messages/inbox.html.erb b/app/views/messages/inbox.html.erb index 54089c34a..db807d2df 100644 --- a/app/views/messages/inbox.html.erb +++ b/app/views/messages/inbox.html.erb @@ -2,35 +2,12 @@ <%= javascript_include_tag "messages" %> <% end %> -<% content_for :heading_class, "pb-0" %> +<%= render :partial => "heading", :locals => { :active_link_path => inbox_messages_path } %> -<% content_for :heading do %> -

    <%= t("users.show.my messages") %>

    - -<% end %> - -

    <%= render :partial => "message_count" %>

    +

    <%= render :partial => "message_count" %>

    <% if current_user.messages.size > 0 %> - - - - - - - - - - <%= render :partial => "message_summary", :collection => current_user.messages %> - -
    <%= t ".from" %><%= t ".subject" %><%= t ".date" %>
    + <%= render :partial => "messages_table", :locals => { :columns => %w[from subject date], :messages => current_user.messages, :inner_partial => "message_summary" } %> <% else %>
    <%= t(".no_messages_yet_html", :people_mapping_nearby_link => link_to(t(".people_mapping_nearby"), user_path(current_user))) %>
    <% end %> diff --git a/app/views/messages/muted.html.erb b/app/views/messages/muted.html.erb new file mode 100644 index 000000000..40c74e915 --- /dev/null +++ b/app/views/messages/muted.html.erb @@ -0,0 +1,9 @@ +<% content_for :head do %> + <%= javascript_include_tag "messages" %> +<% end %> + +<%= render :partial => "heading", :locals => { :active_link_path => muted_messages_path } %> + +

    <%= t ".messages", :count => current_user.muted_messages.size %>

    + +<%= render :partial => "messages_table", :locals => { :columns => %w[from subject date], :messages => current_user.muted_messages, :inner_partial => "message_summary" } %> diff --git a/app/views/messages/outbox.html.erb b/app/views/messages/outbox.html.erb index e246f9292..ae8a899c4 100644 --- a/app/views/messages/outbox.html.erb +++ b/app/views/messages/outbox.html.erb @@ -2,36 +2,12 @@ <%= javascript_include_tag "messages" %> <% end %> -<% content_for :heading_class, "pb-0" %> - -<% content_for :heading do %> -

    <%= t("users.show.my messages") %>

    - - -<% end %> +<%= render :partial => "heading", :locals => { :active_link_path => outbox_messages_path } %>

    <%= t ".messages", :count => current_user.sent_messages.size %>

    <% if current_user.sent_messages.size > 0 %> - - - - - - - - - - <%= render :partial => "sent_message_summary", :collection => current_user.sent_messages %> - -
    <%= t ".to" %><%= t ".subject" %><%= t ".date" %>
    + <%= render :partial => "messages_table", :locals => { :columns => %w[to subject date], :messages => current_user.sent_messages, :inner_partial => "sent_message_summary" } %> <% else %>
    <%= t(".no_sent_messages_html", :people_mapping_nearby_link => link_to(t(".people_mapping_nearby"), user_path(current_user))) %>
    <% end %> diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb index 4761aac60..f0742280f 100644 --- a/app/views/messages/show.html.erb +++ b/app/views/messages/show.html.erb @@ -22,9 +22,8 @@ <%= link_to t(".reply_button"), message_reply_path(@message), :class => "btn btn-primary" %> <%= link_to t(".unread_button"), message_mark_path(@message, :mark => "unread"), :method => "post", :class => "btn btn-primary" %> <%= link_to t(".destroy_button"), message_path(@message), :method => "delete", :class => "btn btn-danger" %> - <%= link_to t(".back"), inbox_messages_path, :class => "btn btn-link" %> <% else %> <%= link_to t(".destroy_button"), message_path(@message), :method => "delete", :class => "btn btn-danger" %> - <%= link_to t(".back"), outbox_messages_path, :class => "btn btn-link" %> <% end %> + <%= link_to t(".back"), :back, :class => "btn btn-link" %> diff --git a/app/views/user_mutes/index.html.erb b/app/views/user_mutes/index.html.erb new file mode 100644 index 000000000..cf9e7ed6b --- /dev/null +++ b/app/views/user_mutes/index.html.erb @@ -0,0 +1,38 @@ +<% content_for :heading do %> +

    <%= t ".my_muted_users" %>

    +<% end %> + +<%= render :partial => "settings_menu" %> + +

    + <%= t ".you_have_muted_n_users", :count => @muted_users.size %> +

    +

    + <%= t ".user_mute_explainer" %> + <%= t ".user_mute_admins_and_moderators" %> +

    + +<% if @muted_users.any? %> + + + + + + + + + <% @muted_users.each do |user| %> + + + + + <% end %> + +
    <%= t ".table.thead.muted_user" %><%= t ".table.thead.actions" %>
    + <%= user_thumbnail_tiny user %> + <%= link_to user.display_name, user_path(user) %> + + <%= link_to t(".table.tbody.unmute"), user_mute_path(user), :method => :delete, :class => "btn btn-sm btn-primary" %> + <%= link_to t(".table.tbody.send_message"), new_message_path(user), :class => "btn btn-sm btn-secondary" %> +
    +<% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 81fda926b..0c803ebb0 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -112,6 +112,16 @@ <%= report_link(t(".report"), @user) %> <% end %> + + <% if current_user and UserMute.exists?(owner: current_user, subject: @user) %> +
  • + <%= link_to t(".destroy_mute"), user_mute_path(@user), :method => :delete %> +
    • + <% elsif current_user %> +
  • + <%= link_to t(".create_mute"), user_mute_path(@user), :method => :post, :title => t("user_mutes.index.user_mute_explainer") %> +
    • + <% end %> <% end %> diff --git a/config/application.rb b/config/application.rb index e568c8540..4517e8adf 100644 --- a/config/application.rb +++ b/config/application.rb @@ -35,6 +35,8 @@ module OpenStreetMap # Enable locale fallbacks for I18n (makes lookups for any locale fall back to # the I18n.default_locale when a translation cannot be found). config.i18n.fallbacks = true + # Enables custom error message formats + config.active_model.i18n_customize_full_message = true # Use logstash for logging if required if Settings.key?(:logstash_path) diff --git a/config/locales/en.yml b/config/locales/en.yml index 18beafa13..7ff2bfd06 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -40,6 +40,12 @@ en: messages: invalid_email_address: does not appear to be a valid e-mail address email_address_not_routable: is not routable + models: + user_mute: + attributes: + subject: + format: "%{message}" + is_already_muted: "is already muted" # Translates all the model names, which is used in error handling on the website models: acl: "Access Control List" @@ -1673,8 +1679,6 @@ en: messages: inbox: title: "Inbox" - my_inbox: "My Inbox" - my_outbox: "My Outbox" messages: "You have %{new_messages} and %{old_messages}" new_messages: one: "%{count} new message" @@ -1682,16 +1686,20 @@ en: old_messages: one: "%{count} old message" other: "%{count} old messages" + no_messages_yet_html: "You have no messages yet. Why not get in touch with some of the %{people_mapping_nearby_link}?" + people_mapping_nearby: "people mapping nearby" + messages_table: from: "From" + to: "To" subject: "Subject" date: "Date" - no_messages_yet_html: "You have no messages yet. Why not get in touch with some of the %{people_mapping_nearby_link}?" - people_mapping_nearby: "people mapping nearby" + actions: "Actions" message_summary: unread_button: "Mark as unread" read_button: "Mark as read" reply_button: "Reply" destroy_button: "Delete" + unmute_button: "Move to Inbox" new: title: "Send message" send_message_to_html: "Send a new message to %{name}" @@ -1705,16 +1713,17 @@ en: body: "Sorry there is no message with that id." outbox: title: "Outbox" - my_inbox: "My Inbox" - my_outbox: "My Outbox" + actions: "Actions" messages: one: "You have %{count} sent message" other: "You have %{count} sent messages" - to: "To" - subject: "Subject" - date: "Date" no_sent_messages_html: "You have no sent messages yet. Why not get in touch with some of the %{people_mapping_nearby_link}?" people_mapping_nearby: "people mapping nearby" + muted: + title: "Muted Messages" + messages: + one: "%{count} muted message" + other: "You have %{count} muted messages" reply: wrong_user: "You are logged in as `%{user}' but the message you have asked to reply to was not sent to that user. Please login as the correct user in order to reply." show: @@ -1726,9 +1735,16 @@ en: wrong_user: "You are logged in as `%{user}' but the message you have asked to read was not sent by or to that user. Please login as the correct user in order to read it." sent_message_summary: destroy_button: "Delete" + heading: + my_inbox: "My Inbox" + my_outbox: "My Outbox" + muted_messages: "Muted messages" mark: as_read: "Message marked as read" as_unread: "Message marked as unread" + unmute: + notice: "Message has been moved to Inbox" + error: "The message could not be moved to the Inbox." destroy: destroyed: "Message deleted" passwords: @@ -2558,6 +2574,7 @@ en: oauth1_settings: OAuth 1 settings oauth2_applications: OAuth 2 applications oauth2_authorizations: OAuth 2 authorizations + muted_users: Muted Users oauth: authorize: title: "Authorize access to your account" @@ -2745,6 +2762,8 @@ en: my_dashboard: My Dashboard blocks on me: Blocks on Me blocks by me: Blocks by Me + create_mute: Mute this User + destroy_mute: Unmute this User edit_profile: Edit Profile send message: Send Message diary: Diary @@ -2935,6 +2954,29 @@ en: showing_page: "Page %{page}" next: "Next »" previous: "« Previous" + user_mutes: + index: + title: "Muted Users" + my_muted_users: "My muted users" + you_have_muted_n_users: + one: "You have muted %{count} User" + other: "You have muted %{count} users" + user_mute_explainer: "Messages of muted users are moved into a separate Inbox and you won't receive email notifications." + user_mute_admins_and_moderators: "You can mute Admins and Moderators but their messages will not be muted." + table: + thead: + muted_user: "Muted User" + actions: "Actions" + tbody: + unmute: "Unmute" + send_message: "Send message" + + create: + notice: "You muted %{name}." + error: "%{name} could not be muted. %{full_message}." + destroy: + notice: "You unmuted %{name}." + error: "User could not be unmuted. Please try again." notes: index: title: "Notes submitted or commented on by %{user}" diff --git a/config/routes.rb b/config/routes.rb index 2b67e360e..110a67a49 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -282,9 +282,12 @@ OpenStreetMap::Application.routes.draw do # messages resources :messages, :only => [:create, :show, :destroy] do post :mark + patch :unmute + match :reply, :via => [:get, :post] collection do get :inbox + get :muted get :outbox end end @@ -293,6 +296,12 @@ OpenStreetMap::Application.routes.draw do get "/message/new/:display_name" => "messages#new", :as => "new_message" get "/message/read/:message_id", :to => redirect(:path => "/messages/%{message_id}") + # muting users + scope "/user/:display_name" do + resource :user_mute, :only => [:create, :destroy], :path => "mute" + end + resources :user_mutes, :only => [:index] + # oauth admin pages (i.e: for setting up new clients, etc...) scope "/user/:display_name" do resources :oauth_clients diff --git a/config/settings/test.yml b/config/settings/test.yml index 0cfa74cd7..fe5aa5d89 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -20,8 +20,8 @@ trace_file_storage: "test" trace_image_storage: "test" trace_icon_storage: "test" # Lower some rate limits for testing -max_changeset_comments_per_hour: 30 -moderator_changeset_comments_per_hour: 60 +max_changeset_comments_per_hour: 10 +moderator_changeset_comments_per_hour: 15 # Private key for signing id_tokens doorkeeper_signing_key: | -----BEGIN PRIVATE KEY----- diff --git a/db/migrate/20231010201451_create_user_mutes.rb b/db/migrate/20231010201451_create_user_mutes.rb new file mode 100644 index 000000000..8cb6ff87e --- /dev/null +++ b/db/migrate/20231010201451_create_user_mutes.rb @@ -0,0 +1,15 @@ +class CreateUserMutes < ActiveRecord::Migration[7.0] + def change + create_table :user_mutes do |t| + t.references :owner, :null => false, :index => false + t.references :subject, :null => false, :index => false + + t.timestamps + + t.foreign_key :users, :column => :owner_id + t.foreign_key :users, :column => :subject_id + + t.index [:owner_id, :subject_id], :unique => true + end + end +end diff --git a/db/migrate/20231010203028_add_muted_flag_to_messages.rb b/db/migrate/20231010203028_add_muted_flag_to_messages.rb new file mode 100644 index 000000000..e517aea86 --- /dev/null +++ b/db/migrate/20231010203028_add_muted_flag_to_messages.rb @@ -0,0 +1,5 @@ +class AddMutedFlagToMessages < ActiveRecord::Migration[7.0] + def change + add_column :messages, :muted, :boolean, :default => false, :null => false + end +end diff --git a/db/structure.sql b/db/structure.sql index ba60918f0..0563417cd 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -951,7 +951,8 @@ CREATE TABLE public.messages ( to_user_id bigint NOT NULL, to_user_visible boolean DEFAULT true NOT NULL, from_user_visible boolean DEFAULT true NOT NULL, - body_format public.format_enum DEFAULT 'markdown'::public.format_enum NOT NULL + body_format public.format_enum DEFAULT 'markdown'::public.format_enum NOT NULL, + muted boolean DEFAULT false NOT NULL ); @@ -1454,6 +1455,38 @@ CREATE SEQUENCE public.user_blocks_id_seq ALTER SEQUENCE public.user_blocks_id_seq OWNED BY public.user_blocks.id; +-- +-- Name: user_mutes; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.user_mutes ( + id bigint NOT NULL, + owner_id bigint NOT NULL, + subject_id bigint NOT NULL, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + +-- +-- Name: user_mutes_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.user_mutes_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: user_mutes_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.user_mutes_id_seq OWNED BY public.user_mutes.id; + + -- -- Name: user_preferences; Type: TABLE; Schema: public; Owner: - -- @@ -1835,6 +1868,13 @@ ALTER TABLE ONLY public.reports ALTER COLUMN id SET DEFAULT nextval('public.repo ALTER TABLE ONLY public.user_blocks ALTER COLUMN id SET DEFAULT nextval('public.user_blocks_id_seq'::regclass); +-- +-- Name: user_mutes id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_mutes ALTER COLUMN id SET DEFAULT nextval('public.user_mutes_id_seq'::regclass); + + -- -- Name: user_roles id; Type: DEFAULT; Schema: public; Owner: - -- @@ -2216,6 +2256,14 @@ ALTER TABLE ONLY public.user_blocks ADD CONSTRAINT user_blocks_pkey PRIMARY KEY (id); +-- +-- Name: user_mutes user_mutes_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_mutes + ADD CONSTRAINT user_mutes_pkey PRIMARY KEY (id); + + -- -- Name: user_preferences user_preferences_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -2734,6 +2782,13 @@ CREATE INDEX index_reports_on_user_id ON public.reports USING btree (user_id); CREATE INDEX index_user_blocks_on_user_id ON public.user_blocks USING btree (user_id); +-- +-- Name: index_user_mutes_on_owner_id_and_subject_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX index_user_mutes_on_owner_id_and_subject_id ON public.user_mutes USING btree (owner_id, subject_id); + + -- -- Name: messages_from_user_id_idx; Type: INDEX; Schema: public; Owner: - -- @@ -3107,6 +3162,14 @@ ALTER TABLE ONLY public.oauth_access_grants ADD CONSTRAINT fk_rails_330c32d8d9 FOREIGN KEY (resource_owner_id) REFERENCES public.users(id) NOT VALID; +-- +-- Name: user_mutes fk_rails_591dad3359; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_mutes + ADD CONSTRAINT fk_rails_591dad3359 FOREIGN KEY (owner_id) REFERENCES public.users(id); + + -- -- Name: oauth_access_tokens fk_rails_732cb83ab7; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -3155,6 +3218,14 @@ ALTER TABLE ONLY public.oauth_applications ADD CONSTRAINT fk_rails_cc886e315a FOREIGN KEY (owner_id) REFERENCES public.users(id) NOT VALID; +-- +-- Name: user_mutes fk_rails_e9dd4fb6c3; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.user_mutes + ADD CONSTRAINT fk_rails_e9dd4fb6c3 FOREIGN KEY (subject_id) REFERENCES public.users(id); + + -- -- Name: oauth_access_tokens fk_rails_ee63f25419; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -3514,6 +3585,8 @@ INSERT INTO "schema_migrations" (version) VALUES ('20231117170422'), ('20231101222146'), ('20231029151516'), +('20231010203028'), +('20231010201451'), ('20231010194809'), ('20231007141103'), ('20230830115220'), diff --git a/script/deliver-message b/script/deliver-message index 28d755b24..81de3ef58 100755 --- a/script/deliver-message +++ b/script/deliver-message @@ -33,6 +33,6 @@ mail = Mail.new($stdin.read message = Message.from_mail(mail, from, to) message.save! -UserMailer.message_notification(message).deliver +UserMailer.message_notification(message).deliver if message.notify_recipient? exit 0 diff --git a/test/controllers/user_mutes_controller_test.rb b/test/controllers/user_mutes_controller_test.rb new file mode 100644 index 000000000..cc22faaaa --- /dev/null +++ b/test/controllers/user_mutes_controller_test.rb @@ -0,0 +1,60 @@ +require "test_helper" + +class UserMutesControllerTest < ActionDispatch::IntegrationTest + def test_routes + assert_routing( + { :path => "/user/username/mute", :method => :post }, + { :controller => "user_mutes", :action => "create", :display_name => "username" } + ) + assert_routing( + { :path => "/user/username/mute", :method => :delete }, + { :controller => "user_mutes", :action => "destroy", :display_name => "username" } + ) + assert_routing( + { :path => "/user_mutes", :method => :get }, + { :controller => "user_mutes", :action => "index" } + ) + end + + def test_index + user = create(:user) + user.mutes.create(:subject => create(:user)) + session_for(user) + + get user_mutes_path + assert_match "You have muted 1 User", @response.body + end + + def test_create + user = create(:user) + session_for(user) + + assert_equal 0, user.muted_users.count + subject = create(:user, :display_name => "Bob") + post user_mute_path(subject) + assert_match "You muted Bob", flash[:notice] + + assert_equal 1, user.muted_users.count + assert_equal subject, user.muted_users.first + + post user_mute_path(subject) + assert_match "Bob could not be muted. Is already muted", flash[:error] + assert_equal 1, user.muted_users.count + end + + def test_destroy + user = create(:user) + session_for(user) + + subject = create(:user, :display_name => "Bob") + user.mutes.create(:subject => subject) + assert_equal 1, user.muted_users.count + + delete user_mute_path(subject) + assert_match "You unmuted Bob", flash[:notice] + assert_equal 0, user.muted_users.count + + delete user_mute_path(subject) + assert_response :not_found + end +end diff --git a/test/factories/messages.rb b/test/factories/messages.rb index 4f8ee5437..906c4dea9 100644 --- a/test/factories/messages.rb +++ b/test/factories/messages.rb @@ -14,5 +14,9 @@ FactoryBot.define do trait :read do message_read { true } end + + trait :muted do + muted { true } + end end end diff --git a/test/factories/user_mute.rb b/test/factories/user_mute.rb new file mode 100644 index 000000000..4beaf3a23 --- /dev/null +++ b/test/factories/user_mute.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :user_mute do + owner :factory => :user + subject :factory => :user + end +end diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index 21e751bca..8ec0dc9bc 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -18,149 +18,121 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_create_user_form - I18n.with_locale "en" do - I18n.available_locales.each do |locale| - reset! - get "/user/new", :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } - follow_redirect! - assert_response :success - assert_template "users/new" - end - end + get "/user/new" + follow_redirect! + assert_response :success + assert_template "users/new" end def test_user_create_submit_duplicate_email - I18n.with_locale "en" do - Locale.available.each do |locale| - dup_email = create(:user).email - display_name = "#{locale}_new_tester" - assert_difference("User.count", 0) do - assert_difference("ActionMailer::Base.deliveries.size", 0) do - perform_enqueued_jobs do - post "/user/new", - :params => { :user => { :email => dup_email, - :email_confirmation => dup_email, - :display_name => display_name, - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest" } }, - :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } - end - end + dup_email = create(:user).email + display_name = "new_tester" + assert_difference("User.count", 0) do + assert_difference("ActionMailer::Base.deliveries.size", 0) do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => dup_email, + :email_confirmation => dup_email, + :display_name => display_name, + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest" } } end - assert_response :success - assert_template "users/new" - assert_equal locale.to_s, response.headers["Content-Language"] - assert_select "form" - assert_select "form > div > input.is-invalid#user_email" - assert_no_missing_translations end end + assert_response :success + assert_template "users/new" + assert_select "form" + assert_select "form > div > input.is-invalid#user_email" end def test_user_create_submit_duplicate_username - I18n.with_locale "en" do - I18n.available_locales.each do |locale| - dup_display_name = create(:user).display_name - email = "#{locale}_new_tester" - assert_difference("User.count", 0) do - assert_difference("ActionMailer::Base.deliveries.size", 0) do - perform_enqueued_jobs do - post "/user/new", - :params => { :user => { :email => email, - :email_confirmation => email, - :display_name => dup_display_name, - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest" } }, - :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } - end - end + dup_display_name = create(:user).display_name + email = "new_tester" + assert_difference("User.count", 0) do + assert_difference("ActionMailer::Base.deliveries.size", 0) do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => email, + :email_confirmation => email, + :display_name => dup_display_name, + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest" } } end - assert_response :success - assert_template "users/new" - assert_select "form > div > input.is-invalid#user_display_name" - assert_no_missing_translations end end + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_display_name" end def test_user_create_success - I18n.with_locale "en" do - I18n.available_locales.each do |locale| - new_email = "#{locale}newtester@osm.org" - display_name = "#{locale}_new_tester" - - assert_difference("User.count", 0) do - assert_difference("ActionMailer::Base.deliveries.size", 0) do - perform_enqueued_jobs do - post "/user/new", - :params => { :user => { :email => new_email, - :email_confirmation => new_email, - :display_name => display_name, - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest" } } - end - end + new_email = "newtester@osm.org" + display_name = "new_tester" + + assert_difference("User.count", 0) do + assert_difference("ActionMailer::Base.deliveries.size", 0) do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest" } } end + end + end - assert_redirected_to "/user/terms" - - assert_difference("User.count") do - assert_difference("ActionMailer::Base.deliveries.size", 1) do - perform_enqueued_jobs do - post "/user/save", - :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s }, - :params => { :read_ct => 1, :read_tou => 1 } - follow_redirect! - end - end + assert_redirected_to "/user/terms" + + assert_difference("User.count") do + assert_difference("ActionMailer::Base.deliveries.size", 1) do + perform_enqueued_jobs do + post "/user/save", + :params => { :read_ct => 1, :read_tou => 1 } + follow_redirect! end + end + end - # Check the e-mail - register_email = ActionMailer::Base.deliveries.first + # Check the e-mail + register_email = ActionMailer::Base.deliveries.first - assert_equal register_email.to.first, new_email - # Check that the confirm account url is correct - assert_match(/#{@url}/, register_email.body.to_s) + assert_equal register_email.to.first, new_email + # Check that the confirm account url is correct + assert_match(/#{@url}/, register_email.body.to_s) - # Check the page - assert_response :success - assert_template "confirmations/confirm" + # Check the page + assert_response :success + assert_template "confirmations/confirm" - ActionMailer::Base.deliveries.clear - end - end + ActionMailer::Base.deliveries.clear end def test_user_create_no_tou_failure - I18n.with_locale "en" do - I18n.available_locales.each do |locale| - new_email = "#{locale}newtester@osm.org" - display_name = "#{locale}_new_tester" - - assert_difference("User.count", 0) do - assert_difference("ActionMailer::Base.deliveries.size", 0) do - perform_enqueued_jobs do - post "/user/new", - :params => { :user => { :email => new_email, - :email_confirmation => new_email, - :display_name => display_name, - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest" } } - end - end - end - - assert_redirected_to "/user/terms" + new_email = "#newtester@osm.org" + display_name = "new_tester" + assert_difference("User.count", 0) do + assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post "/user/save", - :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } - assert_redirected_to "/user/terms" + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest" } } end - - ActionMailer::Base.deliveries.clear end end + + assert_redirected_to "/user/terms" + + perform_enqueued_jobs do + post "/user/save" + assert_redirected_to "/user/terms" + end + + ActionMailer::Base.deliveries.clear end # Check that the user can successfully recover their password diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 97afad56e..83cc1251d 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -175,6 +175,26 @@ class MessageTest < ActiveSupport::TestCase assert_equal "text", message.body_format end + def test_notify_recipient + message = create(:message) + assert_not_predicate message, :muted? + assert_predicate message, :notify_recipient? + end + + def test_notify_recipient_for_muted_messages + message = create(:message, :muted) + assert_predicate message, :muted? + assert_not_predicate message, :notify_recipient? + end + + def test_unmuting_a_muted_message + message = create(:message, :muted) + assert_predicate message, :muted? + + message.unmute + assert_not_predicate message, :muted? + end + private def make_message(char, count) diff --git a/test/models/user_mute_test.rb b/test/models/user_mute_test.rb new file mode 100644 index 000000000..ccc68110a --- /dev/null +++ b/test/models/user_mute_test.rb @@ -0,0 +1,24 @@ +require "test_helper" + +class UserMuteTest < ActiveSupport::TestCase + def test_messages_by_muted_users_are_muted + user = create(:user) + muted_user = create(:user) + create(:user_mute, :owner => user, :subject => muted_user) + + message = create(:message, :sender => muted_user, :recipient => user) + assert_predicate message, :muted? + end + + def test_messages_by_admins_or_moderators_are_never_muted + user = create(:user) + + [create(:administrator_user), create(:moderator_user)].each do |admin_or_moderator| + create(:user_mute, :owner => user, :subject => admin_or_moderator) + + message = create(:message, :sender => admin_or_moderator, :recipient => user) + + assert_not_predicate message, :muted? + end + end +end diff --git a/test/system/user_muting_test.rb b/test/system/user_muting_test.rb new file mode 100644 index 000000000..ce4346115 --- /dev/null +++ b/test/system/user_muting_test.rb @@ -0,0 +1,44 @@ +require "application_system_test_case" + +class UserMutingTest < ApplicationSystemTestCase + # NB: loads helpers to verify mailer-related behaviour e.g. via assert_no_emails + include ActionMailer::TestHelper + + test "users can mute and unmute other users" do + user = create(:user) + other_user = create(:user) + sign_in_as(user) + + visit user_path(other_user) + click_link "Mute this User" + assert_content "You muted #{other_user.display_name}" + + visit edit_account_path + assert_content "Muted Users" + click_link "Muted Users" + assert_content "You have muted 1 User" + click_link "Unmute" + + assert_content "You unmuted #{other_user.display_name}" + refute_content "Muted Users" + assert_current_path edit_account_path + end + + test "messages sent by muted users are set `muted` and do not cause notification emails" do + user = create(:user) + muted_user = create(:user) + create(:user_mute, :owner => user, :subject => muted_user) + sign_in_as(muted_user) + + visit new_message_path(user) + fill_in "Subject", :with => "Hey Hey" + fill_in "Body", :with => "some message" + + assert_no_emails do + click_button "Send" + end + + message = Message.find_by(:sender => muted_user, :recipient => user) + assert_predicate message, :muted? + end +end