[BUG FIX] [MER-2932] filter-insights-by-product-course-section (Simon…

…-Initiative#4777)

* [MER-2932] correct filter in tab by page

* [MER-2932] IO inspect removed

* [MER-2932] correct remix section removing the child activities associated to a revision

* [MER-2932] remove repo all and some improvements

* merge with master

* [MER-2932] small fix

* merge with master

* [MER-2932] add test

lib/oli/analytics/by_activity.ex

@@ -18,7 +18,7 @@ defmodule Oli.Analytics.ByActivity do
   defp get_base_query(project_slug, filtered_sections) do
     subquery =
       if filtered_sections != [] do
-        DeliveryResolver.revisions_filter_by_section_ids(
+        DeliveryResolver.revisions_by_section_ids(
           filtered_sections,
           ResourceType.id_for_activity()
         )
@@ -39,7 +39,6 @@ defmodule Oli.Analytics.ByActivity do
         number_of_attempts: analytics.number_of_attempts,
         relative_difficulty: analytics.relative_difficulty
       },
-      preload: [:resource_type],
-      distinct: [activity]
+      preload: [:resource_type]
   end
 end

lib/oli/analytics/by_objective.ex

@@ -20,7 +20,7 @@ defmodule Oli.Analytics.ByObjective do
   defp get_base_query(project_slug, activity_objectives, filtered_sections) do
     subquery =
       if filtered_sections != [] do
-        DeliveryResolver.revisions_filter_by_section_ids(
+        DeliveryResolver.revisions_by_section_ids(
           filtered_sections,
           ResourceType.id_for_objective()
         )

lib/oli/analytics/by_page.ex

@@ -22,7 +22,7 @@ defmodule Oli.Analytics.ByPage do
   defp get_base_query(project_slug, activity_pages, filtered_sections) do
     subquery =
       if filtered_sections != [] do
-        DeliveryResolver.revisions_filter_by_section_ids(
+        DeliveryResolver.revisions_by_section_ids(
           filtered_sections,
           ResourceType.id_for_page()
         )
@@ -35,9 +35,9 @@ defmodule Oli.Analytics.ByPage do
 
     subquery_activity =
       if filtered_sections != [] do
-        DeliveryResolver.revisions_filter_by_section_ids(
+        DeliveryResolver.revisions_by_section_ids(
           filtered_sections,
-          ResourceType.id_for_page()
+          ResourceType.id_for_activity()
         )
       else
         Publishing.query_unpublished_revisions_by_type(
@@ -62,8 +62,7 @@ defmodule Oli.Analytics.ByPage do
         number_of_attempts: analytics.number_of_attempts,
         relative_difficulty: analytics.relative_difficulty
       },
-      preload: [:resource_type],
-      distinct: [activity]
+      preload: [:resource_type]
     )
   end
 

lib/oli/delivery/sections.ex

@@ -2335,7 +2335,8 @@ defmodule Oli.Delivery.Sections do
       # create any remaining section resources which are not in the hierarchy
       create_nonstructural_section_resources(section.id, [publication_id],
         skip_resource_ids: processed_ids,
-        required_survey_resource_id: survey_id
+        required_survey_resource_id: survey_id,
+        reference_activity_ids: []
       )
 
       root_section_resource_id = section_resources_by_resource_id[root_resource_id].id
@@ -2823,10 +2824,15 @@ defmodule Oli.Delivery.Sections do
         |> select([p], p.required_survey_resource_id)
         |> Repo.one()
 
-      create_nonstructural_section_resources(section_id, publication_ids,
-        skip_resource_ids: processed_resource_ids,
-        required_survey_resource_id: survey_id
-      )
+      reference_activity_ids = build_reference_activity_ids(hierarchy.children)
+
+      if length(processed_resource_ids) != 1 do
+        create_nonstructural_section_resources(section_id, publication_ids,
+          skip_resource_ids: processed_resource_ids,
+          required_survey_resource_id: survey_id,
+          reference_activity_ids: reference_activity_ids
+        )
+      end
 
       # Rebuild section previous next index
       PreviousNextIndex.rebuild(section, hierarchy)
@@ -2838,6 +2844,62 @@ defmodule Oli.Delivery.Sections do
     end)
   end
 
+  defp build_reference_activity_ids([]), do: []
+
+  defp build_reference_activity_ids(children_activities) do
+    tuple_of_children_activities =
+      process_activity_ids(children_activities)
+
+    tuple_of_children_activities
+    |> Tuple.to_list()
+    |> Enum.concat()
+  end
+
+  defp process_activity_ids(children_activities) do
+    children_activities
+    |> Enum.reduce({[], []}, fn r, {children_unit_activities_ids, children_activities_ids} ->
+      new_children_unit_activities_ids = build_child_activity_ids_for_units(r.children)
+
+      if Map.has_key?(r.revision, :content) do
+        new_children_activities_ids = build_child_activity_ids(r.revision.content)
+
+        {children_unit_activities_ids ++ new_children_unit_activities_ids,
+         children_activities_ids ++ new_children_activities_ids}
+      else
+        {children_unit_activities_ids ++ new_children_unit_activities_ids,
+         children_activities_ids ++ []}
+      end
+    end)
+  end
+
+  defp build_child_activity_ids_for_units([]), do: []
+
+  defp build_child_activity_ids_for_units(children) do
+    children
+    |> Enum.map(fn c ->
+      if Map.has_key?(c.revision, :content) do
+        build_child_activity_ids(c.revision.content)
+      else
+        []
+      end
+    end)
+    |> List.flatten()
+  end
+
+  defp build_child_activity_ids(%{"model" => nil}), do: []
+
+  defp build_child_activity_ids(%{"model" => model}) do
+    model
+    |> Enum.reduce([], fn item, activity_ids ->
+      case item["activity_id"] do
+        nil -> activity_ids
+        activity_id -> [activity_id | activity_ids]
+      end
+    end)
+  end
+
+  defp build_child_activity_ids(_), do: []
+
   def get_contained_pages(%Section{id: section_id}) do
     from(cp in ContainedPage,
       where: cp.section_id == ^section_id
@@ -3630,10 +3692,11 @@ defmodule Oli.Delivery.Sections do
   # any that belong to the resource ids in skip_resource_ids
   defp create_nonstructural_section_resources(section_id, publication_ids,
          skip_resource_ids: skip_resource_ids,
-         required_survey_resource_id: required_survey_resource_id
+         required_survey_resource_id: required_survey_resource_id,
+         reference_activity_ids: reference_activity_ids
        ) do
     published_resources_by_resource_id =
-      MinimalHierarchy.published_resources_map(publication_ids)
+      build_published_resources_by_resource_id(publication_ids, reference_activity_ids)
 
     now = DateTime.utc_now() |> DateTime.truncate(:second)
 
@@ -3677,6 +3740,14 @@ defmodule Oli.Delivery.Sections do
     Database.batch_insert_all(SectionResource, section_resource_rows)
   end
 
+  defp build_published_resources_by_resource_id(publication_ids, []),
+    do: MinimalHierarchy.published_resources_map(publication_ids)
+
+  defp build_published_resources_by_resource_id(publication_ids, list_children_section_ids),
+    do:
+      MinimalHierarchy.published_resources_map(publication_ids)
+      |> Map.take(list_children_section_ids)
+
   def is_structural?(%Revision{resource_type_id: resource_type_id}) do
     container = ResourceType.id_for_container()
 

lib/oli/publishing/delivery_resolver.ex

@@ -263,7 +263,7 @@ defmodule Oli.Publishing.DeliveryResolver do
     |> emit([:oli, :resolvers, :delivery], :duration)
   end
 
-  def revisions_filter_by_section_ids(section_ids, resource_type_id) do
+  def revisions_by_section_ids(section_ids, resource_type_id) do
     from(sr in SectionResource,
       join: s in Section,
       on: s.id == sr.section_id,
@@ -277,7 +277,8 @@ defmodule Oli.Publishing.DeliveryResolver do
       join: rev in Revision,
       on: rev.id == pr.revision_id,
       where: rev.resource_type_id == ^resource_type_id and rev.deleted == false,
-      select: rev
+      select: rev,
+      distinct: [rev]
     )
   end
 

lib/oli/utils/db_seeder.ex

@@ -198,14 +198,34 @@ defmodule Oli.Seeder do
 
     create_published_resource(publication, container_resource, container_revision)
 
+    %{resource: _activity_resource, revision: activity_revision} =
+      create_activity(
+        %{
+          activity_type_id: Activities.get_registration_by_slug("oli_short_answer").id,
+          content: %{}
+        },
+        publication,
+        project,
+        author
+      )
+
     %{resource: page1, revision: revision1} =
       create_page("Page one", publication, project, author)
 
     %{resource: page2, revision: revision2} =
       create_page("Page two", publication, project, author, create_sample_content())
 
+    %{resource: page3, revision: revision3} =
+      create_page(
+        "Page three",
+        publication,
+        project,
+        author,
+        create_activity_content(activity_revision.resource_id)
+      )
+
     container_revision =
-      attach_pages_to([page1, page2], container_resource, container_revision, publication)
+      attach_pages_to([page1, page2, page3], container_resource, container_revision, publication)
 
     {:ok, pub1} = Publishing.publish_project(project, "some changes", author.id)
 
@@ -229,8 +249,10 @@ defmodule Oli.Seeder do
     |> Map.put(:container, %{resource: container_resource, revision: container_revision})
     |> Map.put(:page1, page1)
     |> Map.put(:page2, page2)
+    |> Map.put(:page3, page3)
     |> Map.put(:revision1, revision1)
     |> Map.put(:revision2, revision2)
+    |> Map.put(:revision3, revision3)
     |> Map.put(:section, section)
   end
 
@@ -1290,6 +1312,19 @@ defmodule Oli.Seeder do
     }
   end
 
+  def create_activity_content(activity_resource_id) do
+    %{
+      "model" => [
+        %{
+          "type" => "activity-reference",
+          "activity_id" => activity_resource_id,
+          "custom" => %{}
+        }
+      ],
+      "advancedDelivery" => false
+    }
+  end
+
   def create_sample_content() do
     %{
       "model" => [

test/oli/delivery/analytics/analytics_test.exs

@@ -100,7 +100,7 @@ defmodule Oli.Delivery.Analytics.AnalyticsTest do
     } do
       assert length(activity_query) == 4
       assert length(objective_query) == 2
-      assert length(page_query) == 4
+      assert length(page_query) == 5
     end
   end
 
@@ -310,7 +310,7 @@ defmodule Oli.Delivery.Analytics.AnalyticsTest do
       # Parent course should still have analytics after duplicating project
       assert Enum.count(objective_query) == 2
       assert Enum.count(activity_query) == 4
-      assert Enum.count(page_query) == 4
+      assert Enum.count(page_query) == 5
 
       # Duplicated course should not have analytics
 
@@ -341,7 +341,7 @@ defmodule Oli.Delivery.Analytics.AnalyticsTest do
 
       # 3 pages with no analytics
       page_insights = ByPage.query_against_project_slug(duplicated.slug, [])
-      assert Enum.count(page_insights) == 1
+      assert Enum.count(page_insights) == 3
 
       assert Enum.all?(page_insights, fn obj ->
                Enum.all?(insights_from_struct.(obj), &is_nil(&1))

test/oli_web/live/delivery/instructor_dashboard/insights/content_tab_test.exs

@@ -336,7 +336,15 @@ defmodule OliWeb.Delivery.InstructorDashboard.ContentTabTest do
            instructor: instructor,
            conn: conn
          } do
-      %{page1: page1, revision1: revision1, page2: page2, revision2: revision2, section: section} =
+      %{
+        page1: page1,
+        revision1: revision1,
+        page2: page2,
+        revision2: revision2,
+        page3: page3,
+        revision3: revision3,
+        section: section
+      } =
         Oli.Seeder.base_project_with_pages()
 
       user_1 = insert(:user, %{given_name: "Lionel", family_name: "Messi"})
@@ -362,6 +370,11 @@ defmodule OliWeb.Delivery.InstructorDashboard.ContentTabTest do
       set_progress(section.id, page2.id, user_3.id, 0.8, revision2)
       set_progress(section.id, page2.id, user_4.id, 0.8, revision2)
 
+      set_progress(section.id, page3.id, user_1.id, 0.9, revision3)
+      set_progress(section.id, page3.id, user_2.id, 0.6, revision3)
+      set_progress(section.id, page3.id, user_3.id, 0, revision3)
+      set_progress(section.id, page3.id, user_4.id, 0.3, revision3)
+
       {:ok, view, _html} = live(conn, live_view_content_route(section.slug))
 
       progress =
@@ -371,7 +384,7 @@ defmodule OliWeb.Delivery.InstructorDashboard.ContentTabTest do
         |> Floki.find(~s{.instructor_dashboard_table tr [data-progress-check]})
         |> Enum.map(fn div_tag -> Floki.text(div_tag) |> String.trim() end)
 
-      assert progress == ["45%", "80%"]
+      assert progress == ["45%", "80%", "45%"]
 
       ### links to students tab with page_id as url param
       links =
@@ -397,6 +410,14 @@ defmodule OliWeb.Delivery.InstructorDashboard.ContentTabTest do
                  :insights,
                  :content,
                  %{page_id: page2.id}
+               ),
+               Routes.live_path(
+                 OliWeb.Endpoint,
+                 OliWeb.Delivery.InstructorDashboard.InstructorDashboardLive,
+                 section.slug,
+                 :insights,
+                 :content,
+                 %{page_id: page3.id}
                )
              ]