Skip to content

Commit

Permalink
[BUG FIX] [MER-3681] 500 error accessing course sections with customi…
Browse files Browse the repository at this point in the history
…zed content (#5051)

* fix an issue where section resource records were being assigned the incorrect project id and spp records were not properly updating the publication id when cloning products

* format

* update docs

* fix test
  • Loading branch information
eliknebel authored Aug 26, 2024
1 parent ae73602 commit 5beb9d1
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 25 deletions.
6 changes: 3 additions & 3 deletions lib/oli/authoring/clone.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ defmodule Oli.Authoring.Clone do
alias Oli.Authoring.Authors.AuthorProject
alias Oli.Delivery.Sections.Blueprint

def clone_blueprints(blueprints, from_base_project_id) do
Enum.map(blueprints, &Blueprint.duplicate(&1, %{}, from_base_project_id))
def clone_blueprints(blueprints, cloned_from_project_publication_ids) do
Enum.map(blueprints, &Blueprint.duplicate(&1, %{}, cloned_from_project_publication_ids))
end

def clone_project(project_slug, author, opts \\ []) do
Expand Down Expand Up @@ -54,7 +54,7 @@ defmodule Oli.Authoring.Clone do
_ <- clone_all_project_activity_registrations(base_project.id, cloned_project.id),
Blueprint.get_blueprint_by_base_project(base_project)
|> Enum.map(fn blueprint -> %{blueprint | base_project_id: cloned_project.id} end)
|> clone_blueprints(base_project.id) do
|> clone_blueprints({base_project.id, cloned_publication.id}) do
cloned_project
else
{:error, error} -> Repo.rollback(error)
Expand Down
63 changes: 44 additions & 19 deletions lib/oli/delivery/sections/blueprint.ex
Original file line number Diff line number Diff line change
Expand Up @@ -195,22 +195,23 @@ defmodule Oli.Delivery.Sections.Blueprint do
This method supports duplication of enrollable sections to create a blueprint.
If this method is called with a `from_base_project_id` parameter, it will update the
section_project_publications record corresponding to that project id. This is
useful when duplicating a blueprint that is a product of a project that is being
cloned. If this parameter is not provided, the `base_project_id` of the section
will be used.
If this method is called with a `cloned_from_project_publication_ids` parameter, it will update the
section_project_publications an section_resource records corresponding to that project id.
This is useful when duplicating a blueprint/product that was part of a project that is being
cloned. If this parameter is not provided, then the project_id of the section_project_publications
will remain the same as the original project id.
"""
def duplicate(%Section{} = section, attrs \\ %{}, from_base_project_id \\ nil) do
def duplicate(%Section{} = section, attrs \\ %{}, cloned_from_project_publication_ids \\ nil) do
Repo.transaction(fn _ ->
with {:ok, blueprint} <- dupe_section(section, attrs),
{:ok, _} <-
dupe_section_project_publications(
section,
blueprint,
from_base_project_id || section.base_project_id
cloned_from_project_publication_ids
),
{:ok, duplicated_root_resource} <- dupe_section_resources(section, blueprint),
{:ok, duplicated_root_resource} <-
dupe_section_resources(section, blueprint, cloned_from_project_publication_ids),
{:ok, blueprint} <-
Sections.update_section(blueprint, %{
root_section_resource_id: duplicated_root_resource.id
Expand Down Expand Up @@ -268,7 +269,8 @@ defmodule Oli.Delivery.Sections.Blueprint do

defp dupe_section_resources(
%Section{id: id, root_section_resource_id: root_id},
%Section{} = blueprint
%Section{} = blueprint,
cloned_from_project_publication_ids
) do
Repo.transaction(fn ->
query =
Expand All @@ -285,10 +287,26 @@ defmodule Oli.Delivery.Sections.Blueprint do
resources_to_create =
Enum.reverse(resources)
|> Enum.reduce([], fn p, resources_to_create ->
# same process as sections_project_publications, if this blueprint was duplicated
# as part of a project cloning, we need to update the project_id of the section_resource
# if the section_resource belongs to the original project
project_id =
case cloned_from_project_publication_ids do
{cloned_from_project_id, _} ->
if p.project_id == cloned_from_project_id do
blueprint.base_project_id
else
p.project_id
end

_ ->
p.project_id
end

resource =
Map.merge(Sections.SectionResource.to_map(p), %{
section_id: blueprint.id,
project_id: blueprint.base_project_id
project_id: project_id
})
|> Map.delete(:id)

Expand Down Expand Up @@ -333,7 +351,7 @@ defmodule Oli.Delivery.Sections.Blueprint do
defp dupe_section_project_publications(
%Section{id: id},
%Section{} = blueprint,
from_base_project_id
cloned_from_project_publication_ids
) do
query =
from(
Expand All @@ -345,22 +363,29 @@ defmodule Oli.Delivery.Sections.Blueprint do
Repo.all(query)
|> Enum.reduce_while({:ok, []}, fn spp, {:ok, all} ->
# In the case where a project is cloned with products, these product blueprints are no longer associated with the
# original project id. So we must use the provided `from_base_project_id` to identify the
# section_project_publication record associated with the original project id and update it to the new project id.
# original project id. So we must use the provided project id in `cloned_from_project_publication_ids` to identify the
# section_project_publication record associated with the original project id and update it to
# the new project and publication ids.
#
# In the other cases where we are simply duplicating a blueprint, the project_id should remain
# the same as the original project id, which is the base_project_id of the blueprint
project_id =
if spp.project_id == from_base_project_id do
blueprint.base_project_id
else
spp.project_id
{project_id, publication_id} =
case cloned_from_project_publication_ids do
{cloned_from_project_id, cloned_publication_id} ->
if spp.project_id == cloned_from_project_id do
{blueprint.base_project_id, cloned_publication_id}
else
{spp.project_id, spp.publication_id}
end

_ ->
{spp.project_id, spp.publication_id}
end

attrs = %{
section_id: blueprint.id,
project_id: project_id,
publication_id: spp.publication_id
publication_id: publication_id
}

case Sections.create_section_project_publication(attrs) do
Expand Down
14 changes: 11 additions & 3 deletions test/oli/clone_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ defmodule Oli.CloneTest do
# Clone the project
{:ok, duplicated_project} = Clone.clone_project(project.slug, author2)

duplicated_publication =
Publishing.project_working_publication(duplicated_project.slug)

%{}
|> Oli.Utils.Seeder.Project.ensure_published(duplicated_publication,
publication_tag: :publication
)

# Check if the products are cloned
duplicated_product_1 =
Sections.get_section_by(base_project_id: duplicated_project.id, title: "Product 1 Copy")
Expand All @@ -96,13 +104,13 @@ defmodule Oli.CloneTest do
assert Repo.get_by(Sections.SectionsProjectsPublications,
project_id: duplicated_project.id,
section_id: duplicated_product_1.id,
publication_id: publication.id
publication_id: duplicated_publication.id
)

assert Repo.get_by(Sections.SectionsProjectsPublications,
project_id: duplicated_project.id,
section_id: duplicated_product_2.id,
publication_id: publication.id
publication_id: duplicated_publication.id
)

# Create a new section from duplicated product 1
Expand All @@ -114,7 +122,7 @@ defmodule Oli.CloneTest do
assert Repo.get_by(Sections.SectionsProjectsPublications,
project_id: duplicated_project.id,
section_id: section.id,
publication_id: publication.id
publication_id: duplicated_publication.id
)
end

Expand Down

0 comments on commit 5beb9d1

Please sign in to comment.