Bug fixes for roles (bigbluebutton#729)

app/controllers/admins_controller.rb

@@ -205,17 +205,7 @@ def new_role
       return redirect_to admin_roles_path
     end
 
-    # Create the new role with the second highest priority
-    # This means that it will only be more important than the user role
-    # This also updates the user role to have the highest priority
-    new_role = Role.create(name: new_role_name, provider: @user_domain)
-    user_role = Role.find_by(name: 'user', provider: @user_domain)
-
-    new_role.priority = user_role.priority
-    user_role.priority += 1
-
-    new_role.save!
-    user_role.save!
+    new_role = Role.create_new_role(new_role_name, @user_domain)
 
     redirect_to admin_roles_path(selected_role: new_role.id)
   end
@@ -283,6 +273,9 @@ def update_role
                                 :colour
                               )
 
+    # Role is a default role so users can't change the name
+    role_params[:name] = role.name if Role::RESERVED_ROLE_NAMES.include?(role.name)
+
     # Make sure if the user is updating the role name that the role name is valid
     if role.name != role_params[:name] && !Role.duplicate_name(role_params[:name], @user_domain) &&
        !role_params[:name].strip.empty?

app/mailers/user_mailer.rb

@@ -42,7 +42,7 @@ def user_promoted(user, role, url, image, color)
     @admin_url = url + "admins"
     @image = image
     @color = color
-    @role = role.name
+    @role = translated_role_name(role)
     mail to: user.email, subject: t('mailer.user.promoted.subtitle', role: translated_role_name(role))
   end
 
@@ -51,7 +51,7 @@ def user_demoted(user, role, url, image, color)
     @root_url = url
     @image = image
     @color = color
-    @role = role.name
+    @role = translated_role_name(role)
     mail to: user.email, subject: t('mailer.user.demoted.subtitle', role: translated_role_name(role))
   end
 

app/models/role.rb

@@ -40,4 +40,20 @@ def self.create_default_roles(provider)
       send_promoted_email: true, send_demoted_email: true, can_edit_site_settings: true,
       can_edit_roles: true, can_manage_users: true, colour: "#cd201f")
   end
+
+  def self.create_new_role(role_name, provider)
+    # Create the new role with the second highest priority
+    # This means that it will only be more important than the user role
+    # This also updates the user role to have the highest priority
+    role = Role.create(name: role_name, provider: provider)
+    user_role = Role.find_by(name: 'user', provider: provider)
+
+    role.priority = user_role.priority
+    user_role.priority += 1
+
+    role.save!
+    user_role.save!
+
+    role
+  end
 end

app/models/user.rb

@@ -248,7 +248,15 @@ def add_role(role)
     unless has_role?(role)
       role_provider = Rails.configuration.loadbalanced_configuration ? provider : "greenlight"
 
-      roles << Role.find_or_create_by(name: role, provider: role_provider)
+      new_role = Role.find_by(name: role, provider: role_provider)
+
+      if new_role.nil?
+        return if Role.duplicate_name(role, role_provider) || role.strip.empty?
+
+        new_role = Role.create_new_role(role, role_provider)
+      end
+
+      roles << new_role
 
       save!
     end

app/views/admins/components/_roles.html.erb

@@ -15,12 +15,13 @@
 
 <div class="container">
   <div class="row">
+    <% current_role = current_user.highest_priority_role%>
     <div class="col-lg-3 mb-4">
         <div class="list-group list-group-transparent mb-0">
             <div id="rolesSelect" data-url="<%= admin_roles_order_path %>">
                 <% @roles.each do |role| %>
                     <%= link_to admin_roles_path(selected_role: role.id), 
-                    class: "#{"sort-disabled" if role.name == "user" || role.name == "admin" || role.priority <= current_user.highest_priority_role.priority } dropdown-item list-group-item list-group-item-action #{"active" if @selected_role.id == role.id}", 
+                    class: "#{"sort-disabled" if role.name == "user" || role.name == "admin" || role.priority <= current_role.priority } dropdown-item list-group-item list-group-item-action #{"active" if @selected_role.id == role.id}", 
                     id: dom_id(role) do %>
                         <%= translated_role_name(role) %>
                     <% end %>
@@ -47,34 +48,34 @@
                 </div>
             </div>
 
-            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block">
+            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block <%="form-disable" if !current_role.can_create_rooms %>">
                 <span class="ml-0 custom-switch-description"><%= t("administrator.roles.can_create_rooms")%></span>
-                    <%= f.check_box :can_create_rooms, class: "custom-switch-input", disabled: edit_disabled %>
+                    <%= f.check_box :can_create_rooms, class: "custom-switch-input", disabled: edit_disabled || !current_role.can_create_rooms %>
                 <span class="custom-switch-indicator float-right"></span>
             </label>
-            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block">
+            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block <%="form-disable" if !current_role.send_promoted_email %>">
                 <span class="ml-0 custom-switch-description"><%= t("administrator.roles.promote_email")%></span>
-                    <%= f.check_box :send_promoted_email, class: "custom-switch-input", disabled: edit_disabled %>
+                    <%= f.check_box :send_promoted_email, class: "custom-switch-input", disabled: edit_disabled || !current_role.send_promoted_email %>
                 <span class="custom-switch-indicator float-right"></span>
             </label>
-            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block">
+            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block <%="form-disable" if !current_role.send_demoted_email %>">
                 <span class="ml-0 custom-switch-description"><%= t("administrator.roles.demote_email")%></span>
-                    <%= f.check_box :send_demoted_email, class: "custom-switch-input", disabled: edit_disabled %>
+                    <%= f.check_box :send_demoted_email, class: "custom-switch-input", disabled: edit_disabled || !current_role.send_demoted_email %>
                 <span class="custom-switch-indicator float-right"></span>
             </label>
-            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block">
+            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block <%="form-disable" if !current_role.can_edit_site_settings %>">
                 <span class="ml-0 custom-switch-description"><%= t("administrator.roles.edit_site_settings")%></span>
-                    <%= f.check_box :can_edit_site_settings, class: "custom-switch-input", disabled: edit_disabled %>
+                    <%= f.check_box :can_edit_site_settings, class: "custom-switch-input", disabled: edit_disabled || !current_role.can_edit_site_settings %>
                 <span class="custom-switch-indicator float-right"></span>
             </label>
-            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block">
+            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block <%="form-disable" if !current_role.can_edit_roles %>">
                 <span class="ml-0 custom-switch-description"><%= t("administrator.roles.edit_roles")%></span>
-                    <%= f.check_box :can_edit_roles, class: "custom-switch-input", disabled: edit_disabled %>
+                    <%= f.check_box :can_edit_roles, class: "custom-switch-input", disabled: edit_disabled || !current_role.can_edit_roles %>
                 <span class="custom-switch-indicator float-right"></span>
             </label>
-            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block">
+            <label class="custom-switch pl-0 mt-3 mb-3 w-100 text-left d-inline-block <%="form-disable" if !current_role.can_manage_users %>">
                 <span class="ml-0 custom-switch-description"><%= t("administrator.roles.manage_users")%></span>
-                    <%= f.check_box :can_manage_users, class: "custom-switch-input", disabled: edit_disabled %>
+                    <%= f.check_box :can_manage_users, class: "custom-switch-input", disabled: edit_disabled || !current_role.can_manage_users %>
                 <span class="custom-switch-indicator float-right"></span>
             </label>
 

app/views/shared/settings/_account.html.erb

@@ -64,7 +64,7 @@
     <div class="row">
       <div class="col-2">
         <% if @user.image.blank? %>
-          <span class="avatar avatar-xxl mr-5 mt-2"><%= @user.name.first %></span>
+          <span class="avatar avatar-xxl mr-5 mt-2 bg-primary"><%= @user.name.first %></span>
         <% else %>
           <span class="avatar avatar-xxl mr-5 mt-2" style="background-image: url(<%= @user.image %>)"></span>
         <% end %>