Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly set admin_imported flag at import and deletion. #11254

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class ContentContentnode(Base):
learner_needs_bitmask_0 = Column(BigInteger)
learning_activities_bitmask_0 = Column(BigInteger)
ancestors = Column(Text)
admin_imported = Column(Boolean)
parent_id = Column(ForeignKey("content_contentnode.id"), index=True)

lang = relationship("ContentLanguage")
Expand Down
24 changes: 18 additions & 6 deletions kolibri/core/content/management/commands/deletecontent.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
logger = logging.getLogger(__name__)


def delete_metadata(channel, node_ids, exclude_node_ids, force_delete):
def delete_metadata(
channel, node_ids, exclude_node_ids, force_delete, ignore_admin_flags
):
# Only delete all metadata if we are not doing selective deletion
delete_all_metadata = not (node_ids or exclude_node_ids)

Expand All @@ -31,7 +33,9 @@ def delete_metadata(channel, node_ids, exclude_node_ids, force_delete):
)

# If we have been passed node ids do not do a full deletion pass
set_content_invisible(channel.id, node_ids, exclude_node_ids)
set_content_invisible(
channel.id, node_ids, exclude_node_ids, not ignore_admin_flags
)
# If everything has been made invisible, delete all the metadata
delete_all_metadata = delete_all_metadata or not channel.root.available

Expand Down Expand Up @@ -123,11 +127,20 @@ def add_arguments(self, parser):
help="Ensure removal of files",
)

parser.add_argument(
"--ignore_admin_flags",
action="store_false",
dest="ignore_admin_flags",
default=True,
help="Don't modify admin_imported values when deleting content",
)

def handle_async(self, *args, **options):
channel_id = options["channel_id"]
node_ids = options["node_ids"]
exclude_node_ids = options["exclude_node_ids"]
force_delete = options["force_delete"]
ignore_admin_flags = options["ignore_admin_flags"]

try:
channel = ChannelMetadata.objects.get(pk=channel_id)
Expand All @@ -136,10 +149,9 @@ def handle_async(self, *args, **options):
"Channel matching id {id} does not exist".format(id=channel_id)
)

(
total_resource_number,
delete_all_metadata,
) = delete_metadata(channel, node_ids, exclude_node_ids, force_delete)
(total_resource_number, delete_all_metadata,) = delete_metadata(
channel, node_ids, exclude_node_ids, force_delete, ignore_admin_flags
)
unused_files = LocalFile.objects.get_unused_files()
# Get the number of files that are being deleted
unused_files_count = unused_files.count()
Expand Down
4 changes: 2 additions & 2 deletions kolibri/core/content/management/commands/generate_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from kolibri.core.content.apps import KolibriContentConfig
from kolibri.core.content.constants.schema_versions import CONTENT_SCHEMA_VERSION
from kolibri.core.content.constants.schema_versions import CURRENT_SCHEMA_VERSION
from kolibri.core.content.utils.channel_import import models_to_exclude
from kolibri.core.content.utils.channel_import import no_schema_models
from kolibri.core.content.utils.sqlalchemybridge import __SQLALCHEMY_CLASSES_MODULE_NAME
from kolibri.core.content.utils.sqlalchemybridge import __SQLALCHEMY_CLASSES_PATH
from kolibri.core.content.utils.sqlalchemybridge import (
Expand Down Expand Up @@ -100,7 +100,7 @@ def handle(self, *args, **options):
table_names = [
model._meta.db_table
for name, model in app_config.models.items()
if name != "channelmetadatacache" and model not in models_to_exclude
if name != "channelmetadatacache" and model not in no_schema_models
]
metadata.reflect(bind=engine, only=table_names)
Base = prepare_base(metadata, name=version)
Expand Down
33 changes: 33 additions & 0 deletions kolibri/core/content/management/commands/importchannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,13 @@ def progress_callback(bytes):
# if upgrading, import the channel
if not no_upgrade:
try:
# In each case we need to evaluate the queryset now,
# in order to get node ids as they currently are before
# the import. If we did not coerce each of these querysets
# to a list now, they would be lazily evaluated after the
# import, and would reflect the state of the database
# after the import.

# evaluate list so we have the current node ids
node_ids = list(
ContentNode.objects.filter(
Expand All @@ -214,13 +221,39 @@ def progress_callback(bytes):
.exclude(kind=content_kinds.TOPIC)
.values_list("id", flat=True)
)
# evaluate list so we have the current node ids
admin_imported_ids = list(
ContentNode.objects.filter(
channel_id=channel_id, available=True, admin_imported=True
)
.exclude(kind=content_kinds.TOPIC)
.values_list("id", flat=True)
)
# evaluate list so we have the current node ids
not_admin_imported_ids = list(
ContentNode.objects.filter(
channel_id=channel_id, available=True, admin_imported=False
)
.exclude(kind=content_kinds.TOPIC)
.values_list("id", flat=True)
)
rtibbles marked this conversation as resolved.
Show resolved Hide resolved
import_ran = import_channel_by_id(
channel_id, self.is_cancelled, contentfolder
)
if import_ran:
if node_ids:
# annotate default channel db based on previously annotated leaf nodes
update_content_metadata(channel_id, node_ids=node_ids)
if admin_imported_ids:
# Reset admin_imported flag for nodes that were imported by admin
ContentNode.objects.filter_by_uuids(
admin_imported_ids
).update(admin_imported=True)
if not_admin_imported_ids:
# Reset admin_imported flag for nodes that were not imported by admin
ContentNode.objects.filter_by_uuids(
not_admin_imported_ids
).update(admin_imported=False)
else:
# ensure the channel is available to the frontend
ContentCacheKey.update_cache_key()
Expand Down
94 changes: 93 additions & 1 deletion kolibri/core/content/test/test_annotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,28 @@ def test_all_nodes_available_non_include_exclude_unaffected(self):
node.refresh_from_db()
self.assertTrue(node.available)

def test_all_leaves_clear_admin_imported(self):
ContentNode.objects.all().update(available=True)
set_leaf_nodes_invisible(test_channel_id, clear_admin_imported=True)
self.assertFalse(
any(
ContentNode.objects.exclude(kind=content_kinds.TOPIC).values_list(
"admin_imported", flat=True
)
)
)

def test_all_leaves_no_clear_admin_imported(self):
ContentNode.objects.all().update(available=True, admin_imported=True)
set_leaf_nodes_invisible(test_channel_id)
self.assertTrue(
all(
ContentNode.objects.exclude(kind=content_kinds.TOPIC).values_list(
"admin_imported", flat=True
)
)
)

def tearDown(self):
call_command("flush", interactive=False)
super(SetContentNodesInvisibleTestCase, self).tearDown()
Expand Down Expand Up @@ -375,7 +397,7 @@ def test_all_local_files_available_exclude_duplicate_topic(self):
ContentNode.objects.filter(title="c2c1", available=True).count(), 1
)

def test_all_local_files_available_include_orignial_exclude_duplicate_topic(self):
def test_all_local_files_available_include_original_exclude_duplicate_topic(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
parent = ContentNode.objects.get(title="c3")
Expand Down Expand Up @@ -419,6 +441,76 @@ def test_all_local_files_available_non_include_exclude_unaffected(self):
node.refresh_from_db()
self.assertTrue(node.available)

def test_all_local_files_admin_imported_true(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
node = ContentNode.objects.get(title="copy", kind=content_kinds.VIDEO)
set_leaf_node_availability_from_local_file_availability(
test_channel_id, admin_imported=True
)
node.refresh_from_db()
self.assertTrue(node.admin_imported)

def test_all_local_files_admin_imported_true_overwrites(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
node = ContentNode.objects.get(title="copy", kind=content_kinds.VIDEO)
node.admin_imported = False
node.save()
set_leaf_node_availability_from_local_file_availability(
test_channel_id, admin_imported=True
)
node.refresh_from_db()
self.assertTrue(node.admin_imported)

def test_all_local_files_admin_imported_false(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
node = ContentNode.objects.get(title="copy", kind=content_kinds.VIDEO)
set_leaf_node_availability_from_local_file_availability(
test_channel_id, admin_imported=False
)
node.refresh_from_db()
self.assertFalse(node.admin_imported)

def test_all_local_files_admin_imported_false_no_overwrite(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
node = ContentNode.objects.get(title="copy", kind=content_kinds.VIDEO)
node.admin_imported = True
node.save()
set_leaf_node_availability_from_local_file_availability(
test_channel_id, admin_imported=False
)
node.refresh_from_db()
self.assertTrue(node.admin_imported)

def test_all_local_files_admin_imported_none_no_effect(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
node = ContentNode.objects.get(title="copy", kind=content_kinds.VIDEO)
set_leaf_node_availability_from_local_file_availability(test_channel_id)
node.refresh_from_db()
self.assertIsNone(node.admin_imported)

def test_all_local_files_admin_imported_non_include_exclude_unaffected(self):
ContentNode.objects.all().update(available=False)
LocalFile.objects.all().update(available=True)
exclude = ContentNode.objects.get(title="c3")
include = ContentNode.objects.get(title="c2")
node = ContentNode.objects.get(title="copy", kind=content_kinds.VIDEO)
self.assertFalse(node.available)
node.admin_imported = False
node.save()
set_leaf_node_availability_from_local_file_availability(
test_channel_id,
node_ids=[include.id],
exclude_node_ids=[exclude.id],
admin_imported=True,
)
node.refresh_from_db()
self.assertFalse(node.admin_imported)

def tearDown(self):
call_command("flush", interactive=False)
super(AnnotationFromLocalFileAvailability, self).tearDown()
Expand Down
12 changes: 11 additions & 1 deletion kolibri/core/content/test/test_import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,7 @@ def test_remote_import_httperror_404(
node_ids={self.c2c1_node_id},
exclude_node_ids=None,
public=False,
admin_imported=True,
)

@patch("kolibri.core.content.utils.resource_import.transfer.FileDownload")
Expand All @@ -1032,7 +1033,12 @@ def test_remote_import_httperror_500(
manager = RemoteChannelResourceImportManager(self.the_channel_id)
manager.run()
self.annotation_mock.set_content_visibility.assert_called_with(
self.the_channel_id, [], node_ids=None, exclude_node_ids=None, public=False
self.the_channel_id,
[],
node_ids=None,
exclude_node_ids=None,
public=False,
admin_imported=True,
)

@patch("kolibri.core.content.utils.resource_import.get_free_space")
Expand Down Expand Up @@ -1140,6 +1146,7 @@ def test_remote_import_no_space_after_first_download(
exclude_node_ids=None,
node_ids=None,
public=False,
admin_imported=True,
)

@patch("kolibri.utils.file_transfer.sleep")
Expand Down Expand Up @@ -1422,6 +1429,7 @@ def test_remote_import_source_corrupted(
exclude_node_ids=None,
node_ids={self.c1_node_id},
public=False,
admin_imported=True,
)

@patch(
Expand Down Expand Up @@ -1480,6 +1488,7 @@ def test_remote_import_full_import(
exclude_node_ids=None,
node_ids=None,
public=False,
admin_imported=True,
)

def test_local_import_with_detected_manifest_file(
Expand Down Expand Up @@ -1988,6 +1997,7 @@ def test_remote_import_fail_on_error_missing(
node_ids={self.c2c1_node_id},
exclude_node_ids=None,
public=False,
admin_imported=True,
)

@patch("kolibri.core.content.utils.resource_import.logger.warning")
Expand Down
2 changes: 1 addition & 1 deletion kolibri/core/content/test/utils/test_content_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ def test_basic(self):
"deletecontent",
self.node.channel_id,
node_ids=[self.request.contentnode_id],
force_delete=True,
ignore_admin_flags=True,
)
self.assertEqual(self.qs.count(), 0)
self.assertEqual(ContentDownloadRequest.objects.count(), 0)
Expand Down
Loading