Skip to content

Commit 006454b

Browse files
committed
feat: adds new fork migration strategy
1 parent df1d91e commit 006454b

File tree

4 files changed

+189
-31
lines changed

4 files changed

+189
-31
lines changed

cms/djangoapps/modulestore_migrator/api.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from openedx.core.types.user import AuthUser
1212

1313
from . import tasks
14-
from .data import RepeatHandlingStrategy
1514
from .models import ModulestoreSource
1615

1716
__all__ = (
@@ -35,9 +34,6 @@ def start_migration_to_library(
3534
"""
3635
Import a course or legacy library into a V2 library (or, a collection within a V2 library).
3736
"""
38-
# Can raise NotImplementedError for the Fork strategy
39-
assert RepeatHandlingStrategy(repeat_handling_strategy).is_implemented()
40-
4137
source, _ = ModulestoreSource.objects.get_or_create(key=source_key)
4238
target_library = get_library(target_library_key)
4339
# get_library ensures that the library is connected to a learning package.

cms/djangoapps/modulestore_migrator/data.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,3 @@ def default(cls) -> RepeatHandlingStrategy:
7070
Returns the default repeat handling strategy.
7171
"""
7272
return cls.Skip
73-
74-
def is_implemented(self) -> bool:
75-
"""
76-
Returns True if the repeat handling strategy is implemented.
77-
"""
78-
if self == self.Fork:
79-
raise NotImplementedError("Forking is not implemented yet.")
80-
81-
return True

cms/djangoapps/modulestore_migrator/tasks.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,13 @@ def should_update_strategy(self) -> bool:
132132
"""
133133
return self.repeat_handling_strategy is RepeatHandlingStrategy.Update
134134

135+
@property
136+
def should_fork_strategy(self) -> bool:
137+
"""
138+
Determines whether the repeat handling strategy should fork the entity.
139+
"""
140+
return self.repeat_handling_strategy is RepeatHandlingStrategy.Fork
141+
135142

136143
@shared_task(base=_MigrationTask, bind=True)
137144
# Note: The decorator @set_code_owner_attribute cannot be used here because the UserTaskMixin
@@ -601,8 +608,9 @@ def _get_distinct_target_container_key(
601608
Returns:
602609
LibraryContainerLocator: The target container key.
603610
"""
604-
# Check if we already processed this block
605-
if context.is_already_migrated(source_key):
611+
# Check if we already processed this block and we are not forking. If we are forking, we will
612+
# want a new target key.
613+
if context.is_already_migrated(source_key) and not context.should_fork_strategy:
606614
existing_version = context.get_existing_target(source_key)
607615

608616
return LibraryContainerLocator(
@@ -646,8 +654,9 @@ def _get_distinct_target_usage_key(
646654
Raises:
647655
ValueError: If source_key is invalid
648656
"""
649-
# Check if we already processed this block
650-
if context.is_already_migrated(source_key):
657+
# Check if we already processed this block and we are not forking. If we are forking, we will
658+
# want a new target key.
659+
if context.is_already_migrated(source_key) and not context.should_fork_strategy:
651660
log.debug(f"Block {source_key} already exists, reusing existing target")
652661
existing_target = context.get_existing_target(source_key)
653662
block_id = existing_target.component.local_key

cms/djangoapps/modulestore_migrator/tests/test_api.py

Lines changed: 176 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
from common.djangoapps.student.tests.factories import UserFactory
1616
from openedx.core.djangoapps.content_libraries import api as lib_api
1717

18+
from xmodule.modulestore.tests.factories import BlockFactory, LibraryFactory
19+
1820

1921
@pytest.mark.django_db
2022
class TestModulestoreMigratorAPI(LibraryTestCase):
@@ -95,24 +97,184 @@ def test_start_migration_to_library_with_collection(self):
9597
modulestoremigration = ModulestoreMigration.objects.get()
9698
assert modulestoremigration.target_collection.key == collection_key
9799

98-
def test_forking_is_not_implemented(self):
100+
def test_start_migration_to_library_with_strategy_skip(self):
99101
"""
100-
Test that the API raises NotImplementedError for the Fork strategy.
102+
Test that the API can start a migration to a library with a skip strategy.
101103
"""
102-
source = ModulestoreSourceFactory()
104+
library = LibraryFactory.create(modulestore=self.store)
105+
library_block = BlockFactory.create(
106+
parent=library,
107+
category="html",
108+
display_name="Original Block",
109+
publish_item=False,
110+
)
111+
source = ModulestoreSourceFactory(key=library.context_key)
112+
user = UserFactory()
113+
114+
# Start a migration with the Skip strategy
115+
api.start_migration_to_library(
116+
user=user,
117+
source_key=source.key,
118+
target_library_key=self.library_v2.library_key,
119+
composition_level=CompositionLevel.Component.value,
120+
repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
121+
preserve_url_slugs=True,
122+
forward_source_to_target=False,
123+
)
124+
125+
modulestoremigration = ModulestoreMigration.objects.get()
126+
assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Skip.value
127+
128+
migrated_components = lib_api.get_library_components(self.library_v2.library_key)
129+
assert len(migrated_components) == 1
130+
131+
# Update the block, changing its name
132+
library_block.display_name = "Updated Block"
133+
self.store.update_item(library_block, user.id)
134+
135+
# Migrate again using the Skip strategy
136+
api.start_migration_to_library(
137+
user=user,
138+
source_key=source.key,
139+
target_library_key=self.library_v2.library_key,
140+
composition_level=CompositionLevel.Component.value,
141+
repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
142+
preserve_url_slugs=True,
143+
forward_source_to_target=False,
144+
)
145+
146+
modulestoremigration = ModulestoreMigration.objects.last()
147+
assert modulestoremigration is not None
148+
assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Skip.value
149+
150+
migrated_components_fork = lib_api.get_library_components(self.library_v2.library_key)
151+
assert len(migrated_components_fork) == 1
152+
153+
component = lib_api.LibraryXBlockMetadata.from_component(
154+
self.library_v2.library_key, migrated_components_fork[0]
155+
)
156+
assert component.display_name == "Original Block"
157+
158+
def test_start_migration_to_library_with_strategy_update(self):
159+
"""
160+
Test that the API can start a migration to a library with a update strategy.
161+
"""
162+
library = LibraryFactory.create(modulestore=self.store)
163+
library_block = BlockFactory.create(
164+
parent=library,
165+
category="html",
166+
display_name="Original Block",
167+
publish_item=False,
168+
)
169+
source = ModulestoreSourceFactory(key=library.context_key)
170+
user = UserFactory()
171+
172+
# Start a migration with the Skip strategy
173+
api.start_migration_to_library(
174+
user=user,
175+
source_key=source.key,
176+
target_library_key=self.library_v2.library_key,
177+
composition_level=CompositionLevel.Component.value,
178+
repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
179+
preserve_url_slugs=True,
180+
forward_source_to_target=False,
181+
)
182+
183+
modulestoremigration = ModulestoreMigration.objects.get()
184+
assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Skip.value
185+
186+
migrated_components = lib_api.get_library_components(self.library_v2.library_key)
187+
assert len(migrated_components) == 1
188+
189+
# Update the block, changing its name
190+
library_block.display_name = "Updated Block"
191+
self.store.update_item(library_block, user.id)
192+
193+
# Migrate again using the Skip strategy
194+
api.start_migration_to_library(
195+
user=user,
196+
source_key=source.key,
197+
target_library_key=self.library_v2.library_key,
198+
composition_level=CompositionLevel.Component.value,
199+
repeat_handling_strategy=RepeatHandlingStrategy.Update.value,
200+
preserve_url_slugs=True,
201+
forward_source_to_target=False,
202+
)
203+
204+
modulestoremigration = ModulestoreMigration.objects.last()
205+
assert modulestoremigration is not None
206+
assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Update.value
207+
208+
migrated_components_fork = lib_api.get_library_components(self.library_v2.library_key)
209+
assert len(migrated_components_fork) == 1
210+
211+
component = lib_api.LibraryXBlockMetadata.from_component(
212+
self.library_v2.library_key, migrated_components_fork[0]
213+
)
214+
assert component.display_name == "Updated Block"
215+
216+
def test_start_migration_to_library_with_strategy_forking(self):
217+
"""
218+
Test that the API can start a migration to a library with a forking strategy.
219+
"""
220+
library = LibraryFactory.create(modulestore=self.store)
221+
library_block = BlockFactory.create(
222+
parent=library,
223+
category="html",
224+
display_name="Original Block",
225+
publish_item=False,
226+
)
227+
source = ModulestoreSourceFactory(key=library.context_key)
103228
user = UserFactory()
104229

105-
with pytest.raises(NotImplementedError):
106-
api.start_migration_to_library(
107-
user=user,
108-
source_key=source.key,
109-
target_library_key=self.library_v2.library_key,
110-
target_collection_slug=None,
111-
composition_level=CompositionLevel.Component.value,
112-
repeat_handling_strategy=RepeatHandlingStrategy.Fork.value,
113-
preserve_url_slugs=True,
114-
forward_source_to_target=False,
115-
)
230+
# Start a migration with the Skip strategy
231+
api.start_migration_to_library(
232+
user=user,
233+
source_key=source.key,
234+
target_library_key=self.library_v2.library_key,
235+
composition_level=CompositionLevel.Component.value,
236+
repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
237+
preserve_url_slugs=True,
238+
forward_source_to_target=False,
239+
)
240+
241+
modulestoremigration = ModulestoreMigration.objects.get()
242+
assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Skip.value
243+
244+
migrated_components = lib_api.get_library_components(self.library_v2.library_key)
245+
assert len(migrated_components) == 1
246+
247+
# Update the block, changing its name
248+
library_block.display_name = "Updated Block"
249+
self.store.update_item(library_block, user.id)
250+
251+
# Migrate again using the Fork strategy
252+
api.start_migration_to_library(
253+
user=user,
254+
source_key=source.key,
255+
target_library_key=self.library_v2.library_key,
256+
composition_level=CompositionLevel.Component.value,
257+
repeat_handling_strategy=RepeatHandlingStrategy.Fork.value,
258+
preserve_url_slugs=True,
259+
forward_source_to_target=False,
260+
)
261+
262+
modulestoremigration = ModulestoreMigration.objects.last()
263+
assert modulestoremigration is not None
264+
assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Fork.value
265+
266+
migrated_components_fork = lib_api.get_library_components(self.library_v2.library_key)
267+
assert len(migrated_components_fork) == 2
268+
269+
first_component = lib_api.LibraryXBlockMetadata.from_component(
270+
self.library_v2.library_key, migrated_components_fork[0]
271+
)
272+
assert first_component.display_name == "Original Block"
273+
274+
second_component = lib_api.LibraryXBlockMetadata.from_component(
275+
self.library_v2.library_key, migrated_components_fork[1]
276+
)
277+
assert second_component.display_name == "Updated Block"
116278

117279
def test_get_migration_info(self):
118280
"""

0 commit comments

Comments
 (0)