diff --git a/.gitignore b/.gitignore index 9f9debd85c..3edee3bc5e 100644 --- a/.gitignore +++ b/.gitignore @@ -25,10 +25,14 @@ var/ # Ignore editor / IDE related data .vscode/ +.gemini/ # IntelliJ IDE, except project config .idea/ /*.iml +.junie/ +.aiassistant/ +.aiignore # ignore future updates to run configuration .run/devserver.run.xml diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue index 373fe19ee7..4ca1aace0a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue @@ -400,7 +400,6 @@ ...mapGetters('contentNode', ['getContentNodeAncestors']), ...mapGetters('currentChannel', ['currentChannel']), ...mapGetters('importFromChannels', ['savedSearchesExist']), - ...mapGetters(['isAIFeatureEnabled']), ...mapState('importFromChannels', ['selected']), isBrowsing() { return this.$route.name === RouteNames.IMPORT_FROM_CHANNELS_BROWSE; @@ -432,10 +431,6 @@ }; }, shouldShowRecommendations() { - if (!this.isAIFeatureEnabled) { - return false; - } - if (this.embedTopicRequest === null) { return false; } diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/__tests__/SearchOrBrowseWindow.spec.js b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/__tests__/SearchOrBrowseWindow.spec.js index e254bb21ab..03a11abe34 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/__tests__/SearchOrBrowseWindow.spec.js +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/__tests__/SearchOrBrowseWindow.spec.js @@ -77,7 +77,6 @@ describe('SearchOrBrowseWindow', () => { getters = { 'currentChannel/currentChannel': () => ({ language: 'en' }), 'importFromChannels/savedSearchesExist': () => true, - isAIFeatureEnabled: () => true, 'contentNode/getContentNodeAncestors': () => () => [{ id: 'node-1', title: 'Test folder' }], }; @@ -132,9 +131,7 @@ describe('SearchOrBrowseWindow', () => { actions: { showSnackbar: actions.showSnackbar, }, - getters: { - isAIFeatureEnabled: getters.isAIFeatureEnabled, - }, + getters: {}, }); const routes = [ diff --git a/contentcuration/contentcuration/frontend/shared/vuex/session/index.js b/contentcuration/contentcuration/frontend/shared/vuex/session/index.js index 6112870535..b19421e64a 100644 --- a/contentcuration/contentcuration/frontend/shared/vuex/session/index.js +++ b/contentcuration/contentcuration/frontend/shared/vuex/session/index.js @@ -6,7 +6,6 @@ import { Session, User } from 'shared/data/resources'; import { forceServerSync } from 'shared/data/serverSync'; import translator from 'shared/translator'; import { applyMods } from 'shared/data/applyRemoteChanges'; -import { FeatureFlagKeys } from 'shared/constants'; function langCode(language) { // Turns a Django language name (en-gb) into an ISO language code (en-GB) @@ -95,12 +94,6 @@ export default { return getters.isAdmin || Boolean(getters.featureFlags[flag]); }; }, - isAIFeatureEnabled(state, getters) { - if (getters.loggedIn) { - return getters.hasFeatureEnabled(FeatureFlagKeys.ai_feature); - } - return false; - }, }, actions: { saveSession(context, currentUser) { diff --git a/contentcuration/contentcuration/frontend/shared/vuex/session/index.spec.js b/contentcuration/contentcuration/frontend/shared/vuex/session/index.spec.js index ef7e7308f3..b7272c34d1 100644 --- a/contentcuration/contentcuration/frontend/shared/vuex/session/index.spec.js +++ b/contentcuration/contentcuration/frontend/shared/vuex/session/index.spec.js @@ -1,5 +1,4 @@ import vuexSessionModule from './index.js'; -import { FeatureFlagKeys } from 'shared/constants'; describe('session module feature flag related getters', () => { let state; @@ -12,7 +11,6 @@ describe('session module feature flag related getters', () => { }, }, }; - state.currentUser.feature_flags[FeatureFlagKeys.ai_feature] = true; }); describe('featureFlags', () => { @@ -54,31 +52,4 @@ describe('session module feature flag related getters', () => { expect(getters.hasFeatureEnabled(state, getters)('false_flag')).toBe(false); }); }); - - describe('isAIFeatureEnabled', () => { - let getters; - beforeEach(() => { - getters = { - loggedIn: true, - hasFeatureEnabled: vuexSessionModule.getters.hasFeatureEnabled(state, { - featureFlags: vuexSessionModule.getters.featureFlags(state), - isAdmin: false, - }), - isAIFeatureEnabled: vuexSessionModule.getters.isAIFeatureEnabled, - }; - }); - it('should return false if not logged in', () => { - getters.loggedIn = false; - expect(getters.isAIFeatureEnabled(state, getters)).toBe(false); - }); - - it('should return true if logged in and ai feature flag is true', () => { - expect(getters.isAIFeatureEnabled(state, getters)).toBe(true); - }); - - it('should return false if logged in and ai feature flag is false', () => { - state.currentUser.feature_flags[FeatureFlagKeys.ai_feature] = false; - expect(getters.isAIFeatureEnabled(state, getters)).toBe(false); - }); - }); }); diff --git a/contentcuration/contentcuration/management/commands/fix_missing_import_sources.py b/contentcuration/contentcuration/management/commands/fix_missing_import_sources.py new file mode 100644 index 0000000000..6f40cb569a --- /dev/null +++ b/contentcuration/contentcuration/management/commands/fix_missing_import_sources.py @@ -0,0 +1,164 @@ +import csv +import logging +import time + +from django.core.management.base import BaseCommand +from django.db.models import Exists +from django.db.models import FilteredRelation +from django.db.models import OuterRef +from django.db.models import Q +from django.db.models.expressions import F +from django_cte import With + +from contentcuration.models import Channel +from contentcuration.models import ContentNode + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Audits nodes that have imported content from public channels and whether the imported content + has a missing source node. + + TODO: this does not yet FIX them + """ + + def handle(self, *args, **options): + start = time.time() + + public_cte = self.get_public_cte() + + # preliminary filter on channels to those private and non-deleted, which have content + # lft=1 is always true for root nodes, so rght>2 means it actually has children + private_channels_cte = With( + Channel.objects.filter( + public=False, + deleted=False, + ) + .annotate( + non_empty_main_tree=FilteredRelation( + "main_tree", condition=Q(main_tree__rght__gt=2) + ), + ) + .annotate( + tree_id=F("non_empty_main_tree__tree_id"), + ) + .values("id", "name", "tree_id"), + name="dest_channel_cte", + ) + + # reduce the list of private channels to those that have an imported node + # from a public channel + destination_channels = ( + private_channels_cte.queryset() + .with_cte(public_cte) + .with_cte(private_channels_cte) + .filter( + Exists( + public_cte.join( + ContentNode.objects.filter( + tree_id=OuterRef("tree_id"), + ), + original_channel_id=public_cte.col.id, + ) + ) + ) + .values("id", "name", "tree_id") + .order_by("id") + ) + + logger.info("=== Iterating over private destination channels. ===") + channel_count = 0 + total_node_count = 0 + + with open("fix_missing_import_sources.csv", "w", newline="") as csv_file: + csv_writer = csv.DictWriter( + csv_file, + fieldnames=[ + "channel_id", + "channel_name", + "contentnode_id", + "contentnode_title", + "public_channel_id", + "public_channel_name", + "public_channel_deleted", + ], + ) + csv_writer.writeheader() + + for channel in destination_channels.iterator(): + node_count = self.handle_channel(csv_writer, channel) + + if node_count > 0: + total_node_count += node_count + channel_count += 1 + + logger.info("=== Done iterating over private destination channels. ===") + logger.info(f"Found {total_node_count} nodes across {channel_count} channels.") + logger.info(f"Finished in {time.time() - start}") + + def get_public_cte(self) -> With: + # This CTE gets all public channels with their main tree info + return With( + Channel.objects.filter(public=True) + .annotate( + tree_id=F("main_tree__tree_id"), + ) + .values("id", "name", "deleted", "tree_id"), + name="public_cte", + ) + + def handle_channel(self, csv_writer: csv.DictWriter, channel: dict) -> int: + public_cte = self.get_public_cte() + channel_id = channel["id"] + channel_name = channel["name"] + tree_id = channel["tree_id"] + + missing_source_nodes = ( + public_cte.join( + ContentNode.objects.filter(tree_id=tree_id), + original_channel_id=public_cte.col.id, + ) + .with_cte(public_cte) + .annotate( + public_channel_id=public_cte.col.id, + public_channel_name=public_cte.col.name, + public_channel_deleted=public_cte.col.deleted, + ) + .filter( + Q(public_channel_deleted=True) + | ~Exists( + ContentNode.objects.filter( + tree_id=public_cte.col.tree_id, + node_id=OuterRef("original_source_node_id"), + ) + ) + ) + .values( + "public_channel_id", + "public_channel_name", + "public_channel_deleted", + contentnode_id=F("id"), + contentnode_title=F("title"), + ) + ) + + # Count and log results + node_count = missing_source_nodes.count() + + # TODO: this will be replaced with logic to correct the missing source nodes + if node_count > 0: + logger.info( + f"{channel_id}:{channel_name}\t{node_count} node(s) with missing source nodes." + ) + row_dict = { + "channel_id": channel_id, + "channel_name": channel_name, + } + for node_dict in missing_source_nodes.iterator(): + row_dict.update(node_dict) + csv_writer.writerow(row_dict) + + return node_count diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index a3f15770cd..272b9ff054 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -47,6 +47,7 @@ from django.utils import timezone from django.utils.translation import gettext as _ from django_cte import CTEManager +from django_cte import CTEQuerySet from django_cte import With from le_utils import proquint from le_utils.constants import content_kinds @@ -837,7 +838,7 @@ def exists(self, *filters): return Exists(self.queryset().filter(*filters).values("user_id")) -class ChannelModelQuerySet(models.QuerySet): +class ChannelModelQuerySet(CTEQuerySet): def create(self, **kwargs): """ Create a new object with the given kwargs, saving it to the database @@ -863,6 +864,12 @@ def update_or_create(self, defaults=None, **kwargs): return super().update_or_create(defaults, **kwargs) +class ChannelModelManager(models.Manager.from_queryset(ChannelModelQuerySet)): + """Custom Channel models manager with CTE support""" + + pass + + class Channel(models.Model): """ Permissions come from association with organizations """ @@ -994,7 +1001,7 @@ class Channel(models.Model): ] ) - objects = ChannelModelQuerySet.as_manager() + objects = ChannelModelManager() @classmethod def get_editable(cls, user, channel_id): diff --git a/contentcuration/contentcuration/static/feature_flags.json b/contentcuration/contentcuration/static/feature_flags.json index 93653fae13..545f7158a1 100644 --- a/contentcuration/contentcuration/static/feature_flags.json +++ b/contentcuration/contentcuration/static/feature_flags.json @@ -9,11 +9,6 @@ "description": "This no-op feature flag is excluded from non-dev environments", "$env": "development" }, - "ai_feature":{ - "type": "boolean", - "title":"Test AI feature", - "description": "Allow user access to AI features" - }, "survey":{ "type": "boolean", "title":"Test Survey feature", diff --git a/contentcuration/contentcuration/tests/management/__init__.py b/contentcuration/contentcuration/tests/management/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/contentcuration/contentcuration/tests/management/commands/__init__.py b/contentcuration/contentcuration/tests/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/contentcuration/contentcuration/tests/management/commands/test_fix_missing_import_sources.py b/contentcuration/contentcuration/tests/management/commands/test_fix_missing_import_sources.py new file mode 100644 index 0000000000..e624313ff8 --- /dev/null +++ b/contentcuration/contentcuration/tests/management/commands/test_fix_missing_import_sources.py @@ -0,0 +1,100 @@ +from unittest.mock import mock_open +from unittest.mock import patch + +from django.core.management import call_command + +from contentcuration.tests import testdata +from contentcuration.tests.base import StudioTestCase + + +class CommandTestCase(StudioTestCase): + """Test suite for the fix_missing_import_sources management command""" + + def setUp(self): + open_patcher = patch( + "contentcuration.management.commands.fix_missing_import_sources.open", + mock_open(), + ) + self.mock_open = open_patcher.start() + self.mock_file = self.mock_open.return_value + self.mock_file.__enter__.return_value = self.mock_file + self.addCleanup(open_patcher.stop) + + csv_writer_patcher = patch( + "contentcuration.management.commands.fix_missing_import_sources.csv.DictWriter" + ) + self.mock_csv_writer = csv_writer_patcher.start() + self.mock_csv_writer_instance = self.mock_csv_writer.return_value + self.addCleanup(csv_writer_patcher.stop) + + self.public_channel = testdata.channel("Public Channel") + self.public_channel.public = True + self.public_channel.save() + + self.private_channel = testdata.channel("Private Channel") + + # see tree.json for this file + self.original_node = ( + self.public_channel.main_tree.get_descendants() + .filter(node_id="00000000000000000000000000000003") + .first() + ) + self.copied_node = self.original_node.copy_to( + target=self.private_channel.main_tree + ) + + def test_handle__opens_csv_file(self): + call_command("fix_missing_import_sources") + + self.mock_open.assert_called_once_with( + "fix_missing_import_sources.csv", "w", newline="" + ) + + self.mock_csv_writer.assert_called_once_with( + self.mock_file, + fieldnames=[ + "channel_id", + "channel_name", + "contentnode_id", + "contentnode_title", + "public_channel_id", + "public_channel_name", + "public_channel_deleted", + ], + ) + + self.mock_csv_writer_instance.writeheader.assert_called_once() + self.mock_csv_writer_instance.writerow.assert_not_called() + + def test_handle__finds_missing(self): + self.original_node.delete() + call_command("fix_missing_import_sources") + + self.mock_csv_writer_instance.writerow.assert_called_once_with( + { + "channel_id": self.private_channel.id, + "channel_name": self.private_channel.name, + "contentnode_id": self.copied_node.id, + "contentnode_title": self.copied_node.title, + "public_channel_id": self.public_channel.id, + "public_channel_name": self.public_channel.name, + "public_channel_deleted": False, + } + ) + + def test_handle__finds_for_deleted_channel(self): + self.public_channel.deleted = True + self.public_channel.save(actor_id=testdata.user().id) + call_command("fix_missing_import_sources") + + self.mock_csv_writer_instance.writerow.assert_called_once_with( + { + "channel_id": self.private_channel.id, + "channel_name": self.private_channel.name, + "contentnode_id": self.copied_node.id, + "contentnode_title": self.copied_node.title, + "public_channel_id": self.public_channel.id, + "public_channel_name": self.public_channel.name, + "public_channel_deleted": True, + } + ) diff --git a/contentcuration/contentcuration/tests/viewsets/test_recommendations.py b/contentcuration/contentcuration/tests/viewsets/test_recommendations.py index dcc865244f..a16aa06b57 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_recommendations.py +++ b/contentcuration/contentcuration/tests/viewsets/test_recommendations.py @@ -150,6 +150,21 @@ def test_recommendation_generic_error(self, mock_load_recommendations): self.assertEqual(response.content.decode(), error_message) mock_load_recommendations.assert_called_once() + @patch( + "contentcuration.utils.automation_manager.AutomationManager.load_recommendations" + ) + def test_recommend_success_nonadmin_user(self, mock_load_recommendations): + user = testdata.user() + self.client.force_authenticate(user=user) + mock_load_recommendations.return_value = self.recommendations_list + + response = self.client.post( + reverse("recommendations"), data=self.topics, format="json" + ) + + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual(response.json(), self.recommendations_list) + class RecommendationsEventViewSetTestCase(StudioAPITestCase): @property diff --git a/contentcuration/contentcuration/viewsets/recommendation.py b/contentcuration/contentcuration/viewsets/recommendation.py index 7d1a3549ab..9f410e2cf7 100644 --- a/contentcuration/contentcuration/viewsets/recommendation.py +++ b/contentcuration/contentcuration/viewsets/recommendation.py @@ -10,7 +10,6 @@ from rest_framework.views import APIView from contentcuration.utils.automation_manager import AutomationManager -from contentcuration.viewsets.user import IsAIFeatureEnabledForUser logger = logging.getLogger(__name__) @@ -23,7 +22,6 @@ class RecommendationView(APIView): permission_classes = [ IsAuthenticated, - IsAIFeatureEnabledForUser, ] manager = AutomationManager() diff --git a/contentcuration/contentcuration/viewsets/user.py b/contentcuration/contentcuration/viewsets/user.py index c779d1be47..a515364532 100644 --- a/contentcuration/contentcuration/viewsets/user.py +++ b/contentcuration/contentcuration/viewsets/user.py @@ -56,27 +56,6 @@ def has_permission(self, request, view): return False -class IsAIFeatureEnabledForUser(BasePermission): - """ - Permission to check if the AI feature is enabled for a user. - """ - - def _can_user_access_feature(self, request): - try: - if request.user.is_admin: - return True - else: - return request.user.check_feature_flag("ai_feature") - except AttributeError: - return False - - def has_permission(self, request, view): - return self._can_user_access_feature(request) - - def has_object_permission(self, request, view, obj): - return self._can_user_access_feature(request) - - class UserListPagination(ValuesViewsetPageNumberPagination): page_size = None page_size_query_param = "page_size"