[FEATURE] Adds new assessment settings to schemas and adjust delivery…

… settings approach (Simon-Initiative#3527)

* first pass

* all settings encoded

* more cleanup, restructured delivery settings

* fixed all remaining unit tests

* wire in scoring strategy and retake mode from effective delivery settings

* lint cleanup

* add password

* add unit test for setting combiner

* Apply suggestions from code review

Apply suggestions

Co-authored-by: Nicolás Cirio <nicolas.cirio@wyeworks.com>

* fix merge error

---------

Co-authored-by: Nicolás Cirio <nicolas.cirio@wyeworks.com>

config/test.exs

@@ -23,6 +23,7 @@ config :oli, Oli.Repo,
   database: "oli_test",
   pool: Ecto.Adapters.SQL.Sandbox,
   timeout: 600_000,
+  pool_size: 30,
   ownership_timeout: 600_000
 
 config :oli, Oban,

lib/oli/delivery.ex

@@ -1,7 +1,8 @@
 defmodule Oli.Delivery do
   alias Lti_1p3.Tool.ContextRoles
   alias Lti_1p3.Tool.Services.{AGS, NRPS}
-  alias Oli.Delivery.{DeliverySetting, Sections}
+  alias Oli.Delivery.Settings.StudentException
+  alias Oli.Delivery.Sections
   alias Oli.Delivery.Sections.{Section, SectionsProjectsPublications}
   alias Oli.Institutions
   alias Oli.Lti.LtiParams
@@ -182,7 +183,7 @@ defmodule Oli.Delivery do
       []
   """
   def search_delivery_settings(filter) do
-    from(ds in DeliverySetting, where: ^filter_conditions(filter))
+    from(ds in StudentException, where: ^filter_conditions(filter))
     |> Repo.all()
   end
 
@@ -192,14 +193,14 @@ defmodule Oli.Delivery do
   ## Examples
 
       iex> create_delivery_setting(%{field: new_value})
-      {:ok, %DeliverySetting{}}
+      {:ok, %StudentException{}}
 
       iex> create_delivery_setting(%{field: bad_value})
       {:error, %Ecto.Changeset{}}
   """
   def create_delivery_setting(attrs \\ %{}) do
-    %DeliverySetting{}
-    |> DeliverySetting.changeset(attrs)
+    %StudentException{}
+    |> StudentException.changeset(attrs)
     |> Repo.insert()
   end
 
@@ -209,28 +210,28 @@ defmodule Oli.Delivery do
   ## Examples
 
       iex> get_delivery_setting_by(%{id: 1})
-      %DeliverySetting{}
+      %StudentException{}
 
       iex> get_delivery_setting_by(%{id: 123})
       nil
   """
   def get_delivery_setting_by(clauses),
-    do: Repo.get_by(DeliverySetting, clauses)
+    do: Repo.get_by(StudentException, clauses)
 
   @doc """
   Updates a delivery setting.
 
   ## Examples
 
       iex> update_delivery_setting(delivery_setting, %{field: new_value})
-      {:ok, %DeliverySetting{}}
+      {:ok, %StudentException{}}
 
       iex> update_delivery_setting(delivery_setting, %{field: bad_value})
       {:error, %Ecto.Changeset{}}
   """
-  def update_delivery_setting(%DeliverySetting{} = delivery_setting, attrs) do
+  def update_delivery_setting(%StudentException{} = delivery_setting, attrs) do
     delivery_setting
-    |> DeliverySetting.changeset(attrs)
+    |> StudentException.changeset(attrs)
     |> Repo.update()
   end
 
@@ -240,10 +241,10 @@ defmodule Oli.Delivery do
   ## Examples
 
       iex> change_delivery_setting(delivery_setting)
-      %Ecto.Changeset{data: %DeliverySetting{}}
+      %Ecto.Changeset{data: %StudentException{}}
   """
-  def change_delivery_setting(%DeliverySetting{} = delivery_setting, attrs \\ %{}) do
-    DeliverySetting.changeset(delivery_setting, attrs)
+  def change_delivery_setting(%StudentException{} = delivery_setting, attrs \\ %{}) do
+    StudentException.changeset(delivery_setting, attrs)
   end
 
   @doc """
@@ -253,7 +254,7 @@ defmodule Oli.Delivery do
   ## Examples
 
       iex> upsert_delivery_setting(%{field: new_value})
-      {:ok, %DeliverySetting{}}
+      {:ok, %StudentException{}}
 
       iex> upsert_delivery_setting(%{field: bad_value})
       {:error, %Ecto.Changeset{}}

lib/oli/delivery/attempts/manual_grading.ex

@@ -313,8 +313,13 @@ defmodule Oli.Delivery.Attempts.ManualGrading do
     case Oli.Delivery.Attempts.PageLifecycle.Graded.roll_up_activities_to_resource_attempt(
            resource_attempt_guid
          ) do
-      {:ok, %ResourceAttempt{lifecycle_state: :evaluated, resource_access_id: resource_access_id}} ->
+      {:ok, %ResourceAttempt{lifecycle_state: :evaluated, revision: revision, resource_access_id: resource_access_id}} ->
+
+        resource_access = Oli.Repo.get(ResourceAccess, resource_access_id)
+        effective_settings = Oli.Delivery.Settings.get_combined_settings(revision, section.id, resource_access.user_id)
+
         Oli.Delivery.Attempts.PageLifecycle.Graded.roll_up_resource_attempts_to_access(
+          effective_settings,
           section.slug,
           resource_access_id
         )

lib/oli/delivery/attempts/page_lifecycle.ex

@@ -49,6 +49,7 @@ defmodule Oli.Delivery.Attempts.PageLifecycle do
         section_slug,
         datashop_session_id,
         user,
+        effective_settings,
         activity_provider
       ) do
     Repo.transaction(fn ->
@@ -72,7 +73,8 @@ defmodule Oli.Delivery.Attempts.PageLifecycle do
             user: user,
             audience_role: Oli.Delivery.Audience.audience_role(user, section_slug),
             datashop_session_id: datashop_session_id,
-            activity_provider: activity_provider
+            activity_provider: activity_provider,
+            effective_settings: effective_settings
           }
 
           impl = determine_page_impl(page_revision.graded)
@@ -104,7 +106,7 @@ defmodule Oli.Delivery.Attempts.PageLifecycle do
 
   `{:ok, {:not_started, %HistorySummary{}}}`
   """
-  @spec visit(%Revision{}, String.t(), String.t(), User.t(), any) ::
+  @spec visit(%Revision{}, String.t(), String.t(), User.t(), any, any) ::
           {:ok, {:in_progress | :revised, %AttemptState{}}}
           | {:ok, {:not_started, %HistorySummary{}}}
           | {:error, any}
@@ -113,6 +115,7 @@ defmodule Oli.Delivery.Attempts.PageLifecycle do
         section_slug,
         datashop_session_id,
         user,
+        effective_settings,
         activity_provider
       ) do
     Repo.transaction(fn ->
@@ -132,7 +135,8 @@ defmodule Oli.Delivery.Attempts.PageLifecycle do
         user: user,
         audience_role: Oli.Delivery.Audience.audience_role(user, section_slug),
         datashop_session_id: datashop_session_id,
-        activity_provider: activity_provider
+        activity_provider: activity_provider,
+        effective_settings: effective_settings
       }
 
       impl = determine_page_impl(graded)
@@ -199,10 +203,18 @@ defmodule Oli.Delivery.Attempts.PageLifecycle do
             Repo.rollback({:not_found})
 
           resource_attempt ->
+
+            resource_access = Oli.Repo.get(ResourceAccess, resource_attempt.resource_access_id)
+
             context = %FinalizationContext{
               resource_attempt: resource_attempt,
               section_slug: section_slug,
-              datashop_session_id: datashop_session_id
+              datashop_session_id: datashop_session_id,
+              effective_settings: Oli.Delivery.Settings.get_combined_settings(
+                  resource_attempt.revision,
+                  resource_access.section_id,
+                  resource_access.user_id
+                )
             }
 
             impl = determine_page_impl(resource_attempt.revision.graded)

lib/oli/delivery/attempts/page_lifecycle/finalization_context.ex

@@ -9,18 +9,21 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.FinalizationContext do
   @enforce_keys [
     :section_slug,
     :resource_attempt,
-    :datashop_session_id
+    :datashop_session_id,
+    :effective_settings
   ]
 
   defstruct [
     :section_slug,
     :resource_attempt,
-    :datashop_session_id
+    :datashop_session_id,
+    :effective_settings
   ]
 
   @type t() :: %__MODULE__{
           section_slug: String.t(),
           resource_attempt: any(),
-          datashop_session_id: String.t()
+          datashop_session_id: String.t(),
+          effective_settings: Oli.Delivery.Settings.Combined.t()
         }
 end

lib/oli/delivery/attempts/page_lifecycle/graded.ex

@@ -13,7 +13,6 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.Graded do
   }
 
   alias Oli.Delivery.Attempts.Scoring
-  alias Oli.Publishing.DeliveryResolver
   alias Oli.Delivery.Attempts.ActivityLifecycle.{Evaluate, Persistence}
   alias Oli.Delivery.Evaluation.Result
   alias Oli.Delivery.Attempts.PageLifecycle.Common
@@ -110,7 +109,8 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.Graded do
   def finalize(%FinalizationContext{
         resource_attempt: %ResourceAttempt{lifecycle_state: :active} = resource_attempt,
         section_slug: section_slug,
-        datashop_session_id: datashop_session_id
+        datashop_session_id: datashop_session_id,
+        effective_settings: effective_settings
       }) do
     # Collect all of the part attempt guids for all of the activities that get finalized
     with {:ok, part_attempt_guids} <-
@@ -119,6 +119,7 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.Graded do
       case resource_attempt do
         %ResourceAttempt{lifecycle_state: :evaluated} ->
           case roll_up_resource_attempts_to_access(
+                 effective_settings,
                  section_slug,
                  resource_attempt.resource_access_id
                ) do
@@ -295,18 +296,15 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.Graded do
     ensure_valid_grade({score, out_of})
   end
 
-  def roll_up_resource_attempts_to_access(section_slug, resource_access_id) do
+  def roll_up_resource_attempts_to_access(%{scoring_strategy_id: scoring_strategy_id}, _section_slug, resource_access_id) do
     access = Oli.Repo.get(ResourceAccess, resource_access_id)
 
     graded_attempts =
       get_graded_attempts_from_access(access.id)
       |> Enum.filter(fn ra -> ra.lifecycle_state == :evaluated end)
 
-    %{scoring_strategy_id: strategy_id} =
-      DeliveryResolver.from_resource_id(section_slug, access.resource_id)
-
     {score, out_of} =
-      Scoring.calculate_score(strategy_id, graded_attempts)
+      Scoring.calculate_score(scoring_strategy_id, graded_attempts)
       |> ensure_valid_grade()
 
     update_resource_access(access, %{

lib/oli/delivery/attempts/page_lifecycle/hierarchy.ex

@@ -91,8 +91,9 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.Hierarchy do
        do: []
 
   defp construct_attempt_prototypes(%VisitContext{
+         effective_settings: %{retake_mode: :targeted},
          latest_resource_attempt: latest_resource_attempt,
-         page_revision: %Revision{retake_mode: :targeted} = page_revision
+         page_revision: page_revision
        }) do
     # If the page has changed revisions between attempts, we do not allow previous
     # correct attempts to manifest as constraining prototypes.  The issue here is that

lib/oli/delivery/attempts/page_lifecycle/visit_context.ex

@@ -20,7 +20,8 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.VisitContext do
     :user,
     :audience_role,
     :datashop_session_id,
-    :activity_provider
+    :activity_provider,
+    :effective_settings
   ]
 
   defstruct [
@@ -32,7 +33,8 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.VisitContext do
     :user,
     :audience_role,
     :datashop_session_id,
-    :activity_provider
+    :activity_provider,
+    :effective_settings
   ]
 
   @type t() :: %__MODULE__{
@@ -44,6 +46,7 @@ defmodule Oli.Delivery.Attempts.PageLifecycle.VisitContext do
           user: Oli.Accounts.User.t() | Oli.Accounts.Author.t(),
           audience_role: :student | :instructor,
           datashop_session_id: String.t() | nil,
-          activity_provider: any()
+          activity_provider: any(),
+          effective_settings: Oli.Delivery.Settings.Combined.t()
         }
 end

lib/oli/delivery/page/page_context.ex

@@ -16,7 +16,8 @@ defmodule Oli.Delivery.Page.PageContext do
     :historical_attempts,
     :collab_space_config,
     :is_instructor,
-    :is_student
+    :is_student,
+    :effective_settings
   ]
   defstruct [
     :user,
@@ -31,7 +32,8 @@ defmodule Oli.Delivery.Page.PageContext do
     :historical_attempts,
     :collab_space_config,
     :is_instructor,
-    :is_student
+    :is_student,
+    :effective_settings
   ]
 
   alias Oli.Delivery.Attempts.PageLifecycle
@@ -103,8 +105,11 @@ defmodule Oli.Delivery.Page.PageContext do
           {user_roles.is_instructor?, user_roles.is_student?}
       end
 
+    user_for_attempt = Attempts.get_user_from_attempt(resource_attempt)
+    section = Oli.Delivery.Sections.get_section_by_slug(section_slug)
+
     %PageContext{
-      user: Attempts.get_user_from_attempt(resource_attempt),
+      user: user_for_attempt,
       review_mode: true,
       page: page_revision,
       progress_state: progress_state,
@@ -117,7 +122,8 @@ defmodule Oli.Delivery.Page.PageContext do
       historical_attempts: retrieve_historical_attempts(hd(resource_attempts)),
       collab_space_config: collab_space_config,
       is_instructor: is_instructor,
-      is_student: is_student
+      is_student: is_student,
+      effective_settings: Oli.Delivery.Settings.get_combined_settings(page_revision, section.id, user_for_attempt.id)
     }
   end
 
@@ -150,6 +156,7 @@ defmodule Oli.Delivery.Page.PageContext do
       ) do
     # resolve the page revision per section
     page_revision = DeliveryResolver.from_revision_slug(section_slug, page_slug)
+    effective_settings = Oli.Delivery.Settings.get_combined_settings(page_revision, section_id, user.id)
 
     Attempts.track_access(page_revision.resource_id, section_id, user.id)
 
@@ -161,6 +168,7 @@ defmodule Oli.Delivery.Page.PageContext do
              section_slug,
              datashop_session_id,
              user,
+             effective_settings,
              activity_provider
            ) do
         {:ok, {:not_started, %HistorySummary{resource_attempts: resource_attempts}}} ->
@@ -217,7 +225,8 @@ defmodule Oli.Delivery.Page.PageContext do
       historical_attempts: nil,
       collab_space_config: collab_space_config,
       is_instructor: user_roles.is_instructor?,
-      is_student: user_roles.is_student?
+      is_student: user_roles.is_student?,
+      effective_settings: effective_settings
     }
   end