[BUG FIX] Follow up fix for configure section roles (#875)

* fix an issue where admin roles cant configure course section

* update changelog and mix.exs

* update changelog

* fix role label

* fix an issue with role text showing incorrectly

lib/oli_web/templates/delivery/course_not_configured.html.eex

@@ -1,8 +1,4 @@
 <div class="my-5 text-center">
   <h3>We're not quite ready yet...</h3>
   <p>Your instructor has not configured this course section. Please check back soon.</p>
-  <p>
-    <img src="<%= Routes.static_path(@conn, "/images/construction.png") %>" />
-    <p class="font-italic">[vrrrrrrrmmmmmmmmmmmmmmmmmmm, nnaaaawwwwwwwwww...]<br />[clink, clang...  thump, thump, thump]</p>
-  </p>
-</div>
+</div>

lib/oli_web/templates/layout/_delivery_header.html.eex

@@ -10,36 +10,24 @@
     </div>
 
     <%= if assigns[:lti_params] && user_signed_in?(@conn) do %>
-      <%= if user_role_is_student(@conn, @current_user) do %>
-        <div class="nav-item dropdown form-inline my-2 my-lg-0">
-          <a class="user block nav-link dropdown-toggle" href="#" id="navbarDropdown" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
-            <span class="block lg:inline-block lg:mt-0 text-grey-darkest no-underline hover:underline my-1 mr-3">
+      <div class="nav-item dropdown form-inline my-2 my-lg-0">
+        <a class="user nav-link dropdown-toggle" href="#" id="navbarDropdown" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
+          <div class="block lg:inline-block lg:mt-0 text-grey-darkest no-underline hover:underline mr-4">
+            <div class="username">
               <%= "#{@current_user.name}" %>
-            </span>
-            <div class="user-icon">
-              <img src="<%= @current_user.picture %>" class="rounded-circle" />
             </div>
-          </a>
-          <div class="dropdown-menu dropdown-menu-right" aria-labelledby="navbarDropdown">
-            <%= link "Sign out", to: Routes.delivery_path(@conn, :signout), id: "signout-link", class: "dropdown-item btn" %>
-          </div>
-        </div>
-      <% else %>
-        <div class="nav-item dropdown form-inline my-2 my-lg-0">
-          <a class="user nav-link dropdown-toggle" href="#" id="navbarDropdown" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
-            <div class="block lg:inline-block lg:mt-0 text-grey-darkest no-underline hover:underline mr-4">
-              <div class="username">
-                <%= "#{@current_user.name}" %>
-              </div>
-              <div class="role text-right" style="color: <%= user_role_color @conn, @current_user %>;">
-                <%= user_role_text @conn, @current_user %>
-              </div>
+            <div class="role text-right" style="color: <%= user_role_color @conn, @current_user %>;">
+              <%= user_role_text @conn, @current_user %>
             </div>
-            <div class="user-icon">
-              <img src="<%= @current_user.picture %>" class="rounded-circle" />
-            </div>
-          </a>
-          <div class="dropdown-menu dropdown-menu-right" aria-labelledby="navbarDropdown">
+          </div>
+          <div class="user-icon">
+            <img src="<%= @current_user.picture %>" class="rounded-circle" />
+          </div>
+        </a>
+        <div class="dropdown-menu dropdown-menu-right" aria-labelledby="navbarDropdown">
+          <%= if user_role_is_student(@conn, @current_user) do %>
+            <%= link "Sign out", to: Routes.delivery_path(@conn, :signout), id: "signout-link", class: "dropdown-item btn" %>
+          <% else %>
             <%= if account_linked?(@current_user) do %>
               <h6 class="dropdown-header">Linked: <%= @current_user.author.email %></h6>
               <a class="dropdown-item" href="<%= Routes.live_path(OliWeb.Endpoint, OliWeb.Projects.ProjectsLive) %>" target="_blank">Go to Course Author <i class="fas fa-external-link-alt float-right" style="margin-top: 2px"></i></a>
@@ -49,9 +37,9 @@
             <% else %>
               <a class="dropdown-item" href="<%= Routes.delivery_path(@conn, :link_account) %>" target="_blank">Link Existing Account</a>
             <% end %>
-          </div>
+          <% end %>
         </div>
-      <% end %>
+      </div>
     <% else %>
       <%= if preview_mode(@conn) == true do %>
         <div class="nav-item dropdown form-inline my-2 my-lg-0">

lib/oli_web/views/delivery_view.ex

@@ -6,6 +6,23 @@ defmodule OliWeb.DeliveryView do
   alias Lti_1p3.Tool.ContextRoles
   alias Lti_1p3.Tool.PlatformRoles
 
+  @admin_roles [
+    PlatformRoles.get_role(:system_administrator),
+    PlatformRoles.get_role(:institution_administrator),
+    ContextRoles.get_role(:context_administrator),
+  ]
+
+  @instructor_roles [
+    PlatformRoles.get_role(:institution_instructor),
+    ContextRoles.get_role(:context_instructor),
+  ]
+
+  @student_roles [
+    PlatformRoles.get_role(:institution_student),
+    PlatformRoles.get_role(:institution_learner),
+    ContextRoles.get_role(:context_learner),
+  ]
+
   def get_section_slug(conn) do
     case Sections.get_section_from_lti_params(conn.assigns.lti_params) do
       %Section{slug: slug} -> slug
@@ -17,37 +34,33 @@ defmodule OliWeb.DeliveryView do
   def user_role_is_student(conn, user) do
     section_slug = get_section_slug(conn)
 
-    ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_learner))
+    PlatformRoles.has_roles?(user, @student_roles, :any) || ContextRoles.has_roles?(user, section_slug, @student_roles, :any)
   end
 
   def user_role_text(conn, user) do
     section_slug = get_section_slug(conn)
 
     cond do
-      PlatformRoles.has_role?(user, PlatformRoles.get_role(:system_administrator))
-      || PlatformRoles.has_role?(user, PlatformRoles.get_role(:institution_administrator))
-      || ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_administrator)) ->
+      PlatformRoles.has_roles?(user, @admin_roles, :any) || ContextRoles.has_roles?(user, section_slug, @admin_roles, :any) ->
         "Administrator"
-      ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_instructor)) ->
+      PlatformRoles.has_roles?(user, @instructor_roles, :any) || ContextRoles.has_roles?(user, section_slug, @instructor_roles, :any) ->
         "Instructor"
-      ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_student)) ->
+      PlatformRoles.has_roles?(user, @student_roles, :any) || ContextRoles.has_roles?(user, section_slug, @student_roles, :any) ->
         "Student"
       true ->
-        "Student"
+        ""
     end
   end
 
   def user_role_color(conn, user) do
     section_slug = get_section_slug(conn)
 
     cond do
-      PlatformRoles.has_role?(user, PlatformRoles.get_role(:system_administrator))
-      || PlatformRoles.has_role?(user, PlatformRoles.get_role(:institution_administrator))
-      || ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_administrator)) ->
+      PlatformRoles.has_roles?(user, @admin_roles, :any) || ContextRoles.has_roles?(user, section_slug, @admin_roles, :any) ->
         "#f39c12"
-      ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_instructor)) ->
+      PlatformRoles.has_roles?(user, @instructor_roles, :any) || ContextRoles.has_roles?(user, section_slug, @instructor_roles, :any) ->
         "#2ecc71"
-      ContextRoles.has_role?(user, section_slug, ContextRoles.get_role(:context_learner)) ->
+      PlatformRoles.has_roles?(user, @student_roles, :any) || ContextRoles.has_roles?(user, section_slug, @student_roles, :any) ->
         "#3498db"
       true ->
         "#3498db"