[#58144] Show email on the UI for users with permission

https://community.openproject.org/work_packages/58144
Remove hide_mail flag and replace with :view_user_email permission checks

app/components/members/row_component.rb

@@ -50,8 +50,7 @@ module Members
     end
 
     def mail
-      return unless user?
-      return if principal.pref.hide_mail
+      return unless user? && current_user.allowed_globally?(:view_user_email)
 
       link = mail_to(principal.mail)
 

app/components/users/profile/attributes_component.html.erb

@@ -4,7 +4,7 @@
 
     component_wrapper do
       flex_layout do |details_container|
-        if @user.pref.can_expose_mail?
+        if current_user.allowed_globally?(:view_user_email)
           details_container.with_row(font_weight: :bold) do
             User.human_attribute_name(:mail)
           end

app/components/users/profile/attributes_component.rb

@@ -40,7 +40,7 @@ module Users
       end
 
       def render?
-        @user.pref.can_expose_mail? || @user.visible_custom_field_values.any? { _1.value.present? }
+        current_user.allowed_globally?(:view_user_email) || @user.visible_custom_field_values.any? { _1.value.present? }
       end
 
       def visible_custom_fields

app/models/permitted_params.rb

@@ -263,7 +263,7 @@ class PermittedParams
   end
 
   def pref
-    params.fetch(:pref, {}).permit(:hide_mail, :time_zone, :theme,
+    params.fetch(:pref, {}).permit(:time_zone, :theme,
                                    :comments_sorting, :warn_on_leaving_unsaved,
                                    :auto_hide_popups)
   end

app/models/user_preference.rb

@@ -90,14 +90,6 @@ class UserPreference < ApplicationRecord
     settings.fetch(:diff_type, "inline")
   end
 
-  def hide_mail
-    settings.fetch(:hide_mail, true)
-  end
-
-  def can_expose_mail?
-    !hide_mail
-  end
-
   def auto_hide_popups=(value)
     settings[:auto_hide_popups] = to_boolean(value)
   end

app/services/user_preferences/set_attributes_service.rb

@@ -29,8 +29,6 @@
 module UserPreferences
   class SetAttributesService < ::BaseServices::SetAttributes
     def set_attributes(params)
-      set_boolean_value(:hide_mail, params.delete(:hide_mail)) if params.key?(:hide_mail)
-
       params.each do |k, v|
         model[k] = v if model.supported_settings_method?(k)
       end

app/views/common/feed.atom.builder

@@ -61,7 +61,9 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom" do
       if author
         xml.author do
           xml.name(author)
-          xml.email(author.mail) if author.is_a?(User) && author.mail.present? && author.pref.can_expose_mail?
+          if author.is_a?(User) && author.mail.present? && User.current.allowed_globally?(:view_user_email)
+            xml.email(author.mail)
+          end
         end
       end
       xml.content "type" => "html" do

app/views/journals/index.atom.builder

@@ -44,7 +44,8 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom" do
       xml.updated change.created_at.xmlschema
       xml.author do
         xml.name change.user.name
-        xml.email(change.user.mail) if change.user.is_a?(User) && change.user.mail.present? && change.user.pref.can_expose_mail?
+        if change.user.is_a?(User) && change.user.mail.present? && User.current.allowed_globally?(:view_user_email)
+          xml.email(change.user.mail)
       end
       xml.content "type" => "html" do
         xml.text! "<ul>"

app/views/my/account.html.erb

@@ -79,10 +79,6 @@ See COPYRIGHT and LICENSE files for more details.
       <% end %>
     </div>
 
-    <%= fields_for :pref, @user.pref, builder: TabularFormBuilder, lang: current_language do |pref_fields| %>
-      <div class="form--field"><%= pref_fields.check_box :hide_mail %></div>
-    <% end %>
-
     <%= call_hook(:view_my_account, user: @user, form: f) %>
 
     <%= render partial: "customizable/form",

config/locales/en.yml

@@ -877,7 +877,6 @@ en:
         consented_at: "Consented at"
       user_preference:
         comments_sorting: "Display comments"
-        hide_mail: "Hide my email address"
         impaired: "Accessibility mode"
         time_zone: "Time zone"
         auto_hide_popups: "Auto-hide success notifications"

config/schemas/user_preferences.schema.json

@@ -6,9 +6,6 @@
             "type": "object",
             "additionalProperties": false,
             "properties": {
-                "hide_mail": {
-                    "type": "boolean"
-                },
                 "time_zone": {
                     "type": ["string", "null"]
                 },

db/migrate/20241002151949_remove_hide_mail_from_user_preferences.rb

@@ -0,0 +1,8 @@
+class RemoveHideMailFromUserPreferences < ActiveRecord::Migration[7.1]
+  def up
+    execute <<~SQL.squish
+      UPDATE user_preferences
+      SET settings =  settings - 'hide_mail'
+    SQL
+  end
+end

lib/api/v3/user_preferences/user_preference_representer.rb

@@ -53,7 +53,6 @@ module API
           }
         end
 
-        property :hide_mail
         property :time_zone,
                  render_nil: true
 

spec/contracts/user_preferences/params_contract_spec.rb

@@ -45,7 +45,6 @@ RSpec.describe UserPreferences::ParamsContract do
   end
   let(:params) do
     {
-      hide_mail: true,
       auto_hide_popups: true,
       comments_sorting: "desc",
       daily_reminders: {
@@ -204,7 +203,6 @@ RSpec.describe UserPreferences::ParamsContract do
     context "when notification_settings empty" do
       let(:params) do
         {
-          hide_mail: true,
           auto_hide_popups: true,
           comments_sorting: "desc",
           daily_reminders: {

spec/contracts/user_preferences/update_contract_spec.rb

@@ -41,7 +41,6 @@ RSpec.describe UserPreferences::UpdateContract do
   end
   let(:settings) do
     {
-      hide_mail: true,
       auto_hide_popups: true,
       comments_sorting: "desc",
       daily_reminders: {
@@ -107,16 +106,6 @@ RSpec.describe UserPreferences::UpdateContract do
       it_behaves_like "contract is valid"
     end
 
-    context "with a string for hide_mail" do
-      let(:settings) do
-        {
-          hide_mail: "yes please"
-        }
-      end
-
-      it_behaves_like "contract is invalid", hide_mail: :type_mismatch
-    end
-
     context "with a field within the daily_reminders having the wrong type" do
       let(:settings) do
         {
@@ -205,7 +194,6 @@ RSpec.describe UserPreferences::UpdateContract do
     context "without a time_zone" do
       let(:settings) do
         {
-          hide_mail: true,
           auto_hide_popups: true,
           comments_sorting: "desc",
           daily_reminders: {

spec/controllers/users_controller_spec.rb

@@ -629,7 +629,6 @@ RSpec.describe UsersController do
             force_password_change: true
           },
           pref: {
-            hide_mail: "1",
             comments_sorting: "desc"
           }
         }
@@ -650,7 +649,6 @@ RSpec.describe UsersController do
         expect(some_user_from_db.firstname).to eq("Changed")
         expect(some_user_from_db.login).to eq("changed_login")
         expect(some_user_from_db.force_password_change).to be(true)
-        expect(some_user_from_db.pref[:hide_mail]).to be_truthy
         expect(some_user_from_db.pref[:comments_sorting]).to eq("desc")
       end
 

spec/factories/user_preference_factory.rb

@@ -29,7 +29,6 @@
 FactoryBot.define do
   factory :user_preference do
     user
-    hide_mail { true }
     transient do
       others { {} }
     end

spec/features/users/my_spec.rb

@@ -458,14 +458,12 @@ RSpec.describe "my", :js, :with_cuprite do
         expect(page).to have_text(I18n.t("user.text_change_disabled_for_ldap_login"), count: 3)
 
         fill_in "Hobbies", with: "Ruby, DCS"
-        uncheck "pref[hide_mail]"
         click_on "Save"
 
         expect(page).to have_content I18n.t(:notice_account_updated)
 
         user.reload
         expect(user.custom_values.find_by(custom_field_id: string_cf).value).to eql "Ruby, DCS"
-        expect(user.pref.hide_mail).to be false
       end
     end