Skip to content

Commit f05c8e0

Browse files
claudertibbles
authored andcommitted
Fix sync missing fields: language, provider, aggregator, role_visibility
Add missing fields to resource details sync operation: - language: ContentNode language field - provider: Provider organization field - aggregator: Aggregator organization field - role_visibility: Visible to field (learner/coach) These fields were not being synced when users selected the "Resource details" checkbox during sync operations. They are now included in the sync_resource_details field list. Also add comprehensive unit tests to verify: - Each field syncs correctly when sync_resource_details=True - All four fields sync together - Fields do not sync when sync_resource_details=False Fixes #4930
1 parent f8b957f commit f05c8e0

File tree

2 files changed

+239
-8
lines changed

2 files changed

+239
-8
lines changed

contentcuration/contentcuration/tests/test_sync.py

Lines changed: 235 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from le_utils.constants import content_kinds
55
from le_utils.constants import file_formats
66
from le_utils.constants import format_presets
7+
from le_utils.constants import roles
78
from le_utils.constants.labels import accessibility_categories
89
from le_utils.constants.labels import learning_activities
910
from le_utils.constants.labels import levels
@@ -17,6 +18,7 @@
1718
from contentcuration.models import Channel
1819
from contentcuration.models import ContentTag
1920
from contentcuration.models import File
21+
from contentcuration.models import Language
2022
from contentcuration.models import License
2123
from contentcuration.tests import testdata
2224
from contentcuration.tests.base import StudioAPITestCase
@@ -108,7 +110,6 @@ def test_sync_files_add(self):
108110
if child.title == contentnode.title:
109111
target_child = child
110112
break
111-
self.assertIsNotNone(target_child)
112113
self.assertEqual(target_child.files.count(), contentnode.files.count())
113114

114115
db_file = self._add_temp_file_to_content_node(contentnode)
@@ -172,7 +173,6 @@ def test_sync_assessment_item_add(self):
172173
source_node_id=contentnode.node_id
173174
)
174175

175-
self.assertIsNotNone(target_child)
176176
self.assertEqual(
177177
target_child.assessment_items.count(), contentnode.assessment_items.count()
178178
)
@@ -224,7 +224,6 @@ def test_sync_tags_add(self):
224224
source_node_id=contentnode.node_id
225225
)
226226

227-
self.assertIsNotNone(target_child)
228227
self.assertEqual(target_child.tags.count(), contentnode.tags.count())
229228

230229
tag = ContentTag.objects.create(tag_name="tagname")
@@ -263,7 +262,6 @@ def test_sync_tags_add_multiple_tags(self):
263262
source_node_id=contentnode.node_id
264263
)
265264

266-
self.assertIsNotNone(target_child)
267265
self.assertEqual(target_child.tags.count(), contentnode.tags.count())
268266

269267
# Create the same tag twice
@@ -314,8 +312,6 @@ def test_sync_channel_titles_and_descriptions(self):
314312
source_node_id=contentnode.node_id
315313
)
316314

317-
self.assertIsNotNone(target_child)
318-
319315
for key, value in labels.items():
320316
setattr(contentnode, key, value)
321317
contentnode.save()
@@ -407,8 +403,6 @@ def test_sync_channel_other_metadata_labels(self):
407403
source_node_id=contentnode.node_id
408404
)
409405

410-
self.assertIsNotNone(target_child)
411-
412406
for key, value in labels.items():
413407
setattr(contentnode, key, {value: True})
414408
contentnode.save()
@@ -429,6 +423,239 @@ def test_sync_channel_other_metadata_labels(self):
429423
for key, value in labels.items():
430424
self.assertEqual(getattr(target_child, key), {value: True})
431425

426+
def test_sync_language_field(self):
427+
"""
428+
Test that the language field is synced correctly when sync_resource_details is True.
429+
"""
430+
self.assertFalse(self.channel.has_changes())
431+
self.assertFalse(self.derivative_channel.has_changes())
432+
433+
contentnode = (
434+
self.channel.main_tree.get_descendants()
435+
.exclude(kind_id=content_kinds.TOPIC)
436+
.first()
437+
)
438+
439+
target_child = self.derivative_channel.main_tree.get_descendants().get(
440+
source_node_id=contentnode.node_id
441+
)
442+
443+
# Set a different language on the original node
444+
spanish_language = Language.objects.get(id="es")
445+
contentnode.language = spanish_language
446+
contentnode.save()
447+
448+
sync_channel(
449+
self.derivative_channel,
450+
sync_titles_and_descriptions=False,
451+
sync_resource_details=True,
452+
sync_files=False,
453+
sync_assessment_items=False,
454+
)
455+
456+
self.assertTrue(self.channel.has_changes())
457+
self.assertTrue(self.derivative_channel.has_changes())
458+
459+
target_child.refresh_from_db()
460+
self.assertEqual(target_child.language, spanish_language)
461+
462+
def test_sync_provider_field(self):
463+
"""
464+
Test that the provider field is synced correctly when sync_resource_details is True.
465+
"""
466+
self.assertFalse(self.channel.has_changes())
467+
self.assertFalse(self.derivative_channel.has_changes())
468+
469+
contentnode = (
470+
self.channel.main_tree.get_descendants()
471+
.exclude(kind_id=content_kinds.TOPIC)
472+
.first()
473+
)
474+
475+
target_child = self.derivative_channel.main_tree.get_descendants().get(
476+
source_node_id=contentnode.node_id
477+
)
478+
479+
# Set a provider on the original node
480+
contentnode.provider = "Test Provider Organization"
481+
contentnode.save()
482+
483+
sync_channel(
484+
self.derivative_channel,
485+
sync_titles_and_descriptions=False,
486+
sync_resource_details=True,
487+
sync_files=False,
488+
sync_assessment_items=False,
489+
)
490+
491+
self.assertTrue(self.channel.has_changes())
492+
self.assertTrue(self.derivative_channel.has_changes())
493+
494+
target_child.refresh_from_db()
495+
self.assertEqual(target_child.provider, "Test Provider Organization")
496+
497+
def test_sync_aggregator_field(self):
498+
"""
499+
Test that the aggregator field is synced correctly when sync_resource_details is True.
500+
"""
501+
self.assertFalse(self.channel.has_changes())
502+
self.assertFalse(self.derivative_channel.has_changes())
503+
504+
contentnode = (
505+
self.channel.main_tree.get_descendants()
506+
.exclude(kind_id=content_kinds.TOPIC)
507+
.first()
508+
)
509+
510+
target_child = self.derivative_channel.main_tree.get_descendants().get(
511+
source_node_id=contentnode.node_id
512+
)
513+
514+
# Set an aggregator on the original node
515+
contentnode.aggregator = "Test Aggregator Organization"
516+
contentnode.save()
517+
518+
sync_channel(
519+
self.derivative_channel,
520+
sync_titles_and_descriptions=False,
521+
sync_resource_details=True,
522+
sync_files=False,
523+
sync_assessment_items=False,
524+
)
525+
526+
self.assertTrue(self.channel.has_changes())
527+
self.assertTrue(self.derivative_channel.has_changes())
528+
529+
target_child.refresh_from_db()
530+
self.assertEqual(target_child.aggregator, "Test Aggregator Organization")
531+
532+
def test_sync_role_visibility_field(self):
533+
"""
534+
Test that the role_visibility field is synced correctly when sync_resource_details is True.
535+
"""
536+
self.assertFalse(self.channel.has_changes())
537+
self.assertFalse(self.derivative_channel.has_changes())
538+
539+
contentnode = (
540+
self.channel.main_tree.get_descendants()
541+
.exclude(kind_id=content_kinds.TOPIC)
542+
.first()
543+
)
544+
545+
target_child = self.derivative_channel.main_tree.get_descendants().get(
546+
source_node_id=contentnode.node_id
547+
)
548+
549+
# Set role_visibility to COACH on the original node
550+
contentnode.role_visibility = roles.COACH
551+
contentnode.save()
552+
553+
sync_channel(
554+
self.derivative_channel,
555+
sync_titles_and_descriptions=False,
556+
sync_resource_details=True,
557+
sync_files=False,
558+
sync_assessment_items=False,
559+
)
560+
561+
self.assertTrue(self.channel.has_changes())
562+
self.assertTrue(self.derivative_channel.has_changes())
563+
564+
target_child.refresh_from_db()
565+
self.assertEqual(target_child.role_visibility, roles.COACH)
566+
567+
def test_sync_all_missing_fields(self):
568+
"""
569+
Test that all four previously missing fields (language, provider, aggregator,
570+
role_visibility) are synced together when sync_resource_details is True.
571+
"""
572+
self.assertFalse(self.channel.has_changes())
573+
self.assertFalse(self.derivative_channel.has_changes())
574+
575+
contentnode = (
576+
self.channel.main_tree.get_descendants()
577+
.exclude(kind_id=content_kinds.TOPIC)
578+
.first()
579+
)
580+
581+
target_child = self.derivative_channel.main_tree.get_descendants().get(
582+
source_node_id=contentnode.node_id
583+
)
584+
585+
# Set all four fields on the original node
586+
french_language = Language.objects.get(id="fr")
587+
contentnode.language = french_language
588+
contentnode.provider = "Comprehensive Test Provider"
589+
contentnode.aggregator = "Comprehensive Test Aggregator"
590+
contentnode.role_visibility = roles.COACH
591+
contentnode.save()
592+
593+
sync_channel(
594+
self.derivative_channel,
595+
sync_titles_and_descriptions=False,
596+
sync_resource_details=True,
597+
sync_files=False,
598+
sync_assessment_items=False,
599+
)
600+
601+
self.assertTrue(self.channel.has_changes())
602+
self.assertTrue(self.derivative_channel.has_changes())
603+
604+
target_child.refresh_from_db()
605+
self.assertEqual(target_child.language, french_language)
606+
self.assertEqual(target_child.provider, "Comprehensive Test Provider")
607+
self.assertEqual(target_child.aggregator, "Comprehensive Test Aggregator")
608+
self.assertEqual(target_child.role_visibility, roles.COACH)
609+
610+
def test_sync_missing_fields_not_synced_without_flag(self):
611+
"""
612+
Test that the four fields (language, provider, aggregator, role_visibility)
613+
are NOT synced when sync_resource_details is False.
614+
"""
615+
self.assertFalse(self.channel.has_changes())
616+
self.assertFalse(self.derivative_channel.has_changes())
617+
618+
contentnode = (
619+
self.channel.main_tree.get_descendants()
620+
.exclude(kind_id=content_kinds.TOPIC)
621+
.first()
622+
)
623+
624+
target_child = self.derivative_channel.main_tree.get_descendants().get(
625+
source_node_id=contentnode.node_id
626+
)
627+
628+
# Store original values
629+
original_language = target_child.language
630+
original_provider = target_child.provider
631+
original_aggregator = target_child.aggregator
632+
original_role_visibility = target_child.role_visibility
633+
634+
# Modify all four fields in the original node
635+
german_language = Language.objects.get(id="de")
636+
contentnode.language = german_language
637+
contentnode.provider = "Should Not Sync Provider"
638+
contentnode.aggregator = "Should Not Sync Aggregator"
639+
contentnode.role_visibility = roles.COACH
640+
contentnode.save()
641+
642+
# Sync WITHOUT sync_resource_details
643+
sync_channel(
644+
self.derivative_channel,
645+
sync_titles_and_descriptions=False,
646+
sync_resource_details=False,
647+
sync_files=False,
648+
sync_assessment_items=False,
649+
)
650+
651+
target_child.refresh_from_db()
652+
653+
# Verify fields remain unchanged
654+
self.assertEqual(target_child.language, original_language)
655+
self.assertEqual(target_child.provider, original_provider)
656+
self.assertEqual(target_child.aggregator, original_aggregator)
657+
self.assertEqual(target_child.role_visibility, original_role_visibility)
658+
432659

433660
class ContentIDTestCase(SyncTestMixin, StudioAPITestCase):
434661
def setUp(self):

contentcuration/contentcuration/utils/sync.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ def sync_node(
7171
"license_description",
7272
"copyright_holder",
7373
"author",
74+
"language",
75+
"provider",
76+
"aggregator",
77+
"role_visibility",
7478
"extra_fields",
7579
"categories",
7680
"learner_needs",

0 commit comments

Comments
 (0)