Merge pull request #5125 from bjester/recommendation-language

Add defensiveness against missing language for recommendations

Author: bjester

contentcuration/contentcuration/frontend/channelEdit/router.js

@@ -38,6 +38,21 @@ const router = new VueRouter({
       path: '/import/:destNodeId/browse/:channelId?/:nodeId?',
       component: SearchOrBrowseWindow,
       props: true,
+      beforeEnter: (to, from, next) => {
+        const promises = [
+          // search recommendations require ancestors to be loaded
+          store.dispatch('contentNode/loadAncestors', { id: to.params.destNodeId }),
+        ];
+
+        if (!store.getters['currentChannel/currentChannel']) {
+          // ensure the current channel is loaded, in case of hard refresh on this route.
+          // alternatively, the page could be reactive to this getter's value, although that doesn't
+          // seem to work properly
+          promises.push(store.dispatch('currentChannel/loadChannel'));
+        }
+
+        return Promise.all(promises).then(() => next());
+      },
     },
     {
       name: RouteNames.IMPORT_FROM_CHANNELS_SEARCH,

contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue

@@ -22,7 +22,7 @@
 
         <!-- Main panel >= 800px -->
         <KGridItem
-          :layout12="{ span: isAIFeatureEnabled && layoutFitsTwoColumns ? 8 : 12 }"
+          :layout12="{ span: shouldShowRecommendations && layoutFitsTwoColumns ? 8 : 12 }"
           :layout8="{ span: 8 }"
           :layout4="{ span: 4 }"
         >
@@ -95,7 +95,7 @@
 
         <!-- Recommended resources panel >= 400px -->
         <KGridItem
-          v-if="isAIFeatureEnabled"
+          v-if="shouldShowRecommendations"
           :layout12="{ span: layoutFitsTwoColumns ? 4 : 12 }"
           :layout8="{ span: 8 }"
           :layout4="{ span: 4 }"
@@ -183,16 +183,21 @@
   import { mapActions, mapGetters, mapMutations, mapState } from 'vuex';
   import { computed } from 'vue';
   import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow';
+  import { SCHEMA } from 'kolibri-constants/EmbedTopicsRequest';
   import { RouteNames } from '../../constants';
-  import RecommendedResourceCard from '../../../RecommendedResourceCard/components/RecommendedResourceCard';
   import ChannelList from './ChannelList';
   import ContentTreeList from './ContentTreeList';
   import SearchResultsList from './SearchResultsList';
   import SavedSearchesModal from './SavedSearchesModal';
   import ImportFromChannelsModal from './ImportFromChannelsModal';
+  import logging from 'shared/logging';
+  import RecommendedResourceCard from 'shared/views/RecommendedResourceCard';
   import { withChangeTracker } from 'shared/data/changes';
   import { formatUUID4 } from 'shared/data/resources';
   import { searchRecommendationsStrings } from 'shared/strings/searchRecommendationsStrings';
+  import { compile } from 'shared/utils/jsonSchema';
+
+  const validateEmbedTopicRequest = compile(SCHEMA);
 
   export default {
     name: 'SearchOrBrowseWindow',
@@ -249,7 +254,6 @@
         copyNode: null,
         languageFromChannelList: null,
         showSavedSearches: false,
-        importDestinationAncestors: [],
         showAboutRecommendations: false,
         recommendations: [],
         otherRecommendations: [],
@@ -266,6 +270,7 @@
       };
     },
     computed: {
+      ...mapGetters('contentNode', ['getContentNodeAncestors']),
       ...mapGetters('currentChannel', ['currentChannel']),
       ...mapGetters('importFromChannels', ['savedSearchesExist']),
       ...mapGetters(['isAIFeatureEnabled']),
@@ -291,6 +296,29 @@
           this.searchTerm.trim() !== this.$route.params.searchTerm
         );
       },
+      shouldShowRecommendations() {
+        if (!this.isAIFeatureEnabled) {
+          return false;
+        }
+
+        if (!validateEmbedTopicRequest(this.embedTopicRequest)) {
+          // log to sentry-- this is unexpected, since we use the channel's language as a fallback
+          // and channels are required to have a language
+          logging.error(
+            new Error(
+              'Recommendation request is invalid: ' +
+                JSON.stringify(
+                  validateEmbedTopicRequest.errors.map(err => {
+                    return `${err.instancePath}: ${err.message}`;
+                  }),
+                ),
+            ),
+          );
+          return false;
+        }
+
+        return true;
+      },
       loadMoreRecommendationsText() {
         let link = null;
         let description = null;
@@ -329,11 +357,11 @@
       },
       browseWindowStyle() {
         return {
-          maxWidth: this.isAIFeatureEnabled ? '1200px' : '800px',
+          maxWidth: this.shouldShowRecommendations ? '1200px' : '800px',
         };
       },
       topicId() {
-        return this.importDestinationFolder?.id;
+        return this.$route.params.destNodeId;
       },
       recommendationsSectionTitle() {
         return this.resourcesMightBeRelevantTitle$({
@@ -361,13 +389,14 @@
               description: this.importDestinationFolder.description,
               language: this.recommendationsLanguage,
               ancestors: this.topicAncestors,
+              channel_id: formatUUID4(this.importDestinationFolder.channel_id),
             },
           ],
-          metadata: {
-            channel_id: formatUUID4(this.importDestinationFolder.channel_id),
-          },
         };
       },
+      importDestinationAncestors() {
+        return this.getContentNodeAncestors(this.topicId, true);
+      },
       importDestinationFolder() {
         return this.importDestinationAncestors.slice(-1)[0];
       },
@@ -405,14 +434,11 @@
     },
     mounted() {
       this.searchTerm = this.$route.params.searchTerm || '';
-      this.loadAncestors({ id: this.$route.params.destNodeId }).then(ancestors => {
-        this.importDestinationAncestors = ancestors;
-        this.loadRecommendations(this.recommendationsBelowThreshold);
-      });
+      this.loadRecommendations(this.recommendationsBelowThreshold);
     },
     methods: {
       ...mapActions('clipboard', ['copy']),
-      ...mapActions('contentNode', ['loadAncestors', 'loadPublicContentNode']),
+      ...mapActions('contentNode', ['loadPublicContentNode']),
       ...mapActions('importFromChannels', ['fetchRecommendations']),
       ...mapMutations('importFromChannels', {
         selectNodes: 'SELECT_NODES',
@@ -504,7 +530,7 @@
         }
       },
       async loadRecommendations(belowThreshold) {
-        if (this.isAIFeatureEnabled) {
+        if (this.shouldShowRecommendations) {
           this.recommendationsLoading = true;
           this.recommendationsLoadingError = false;
           try {

…d/components/RecommendedResourceCard.vue …shared/views/RecommendedResourceCard.vuecontentcuration/contentcuration/frontend/RecommendedResourceCard/components/RecommendedResourceCard.vue renamed to contentcuration/contentcuration/frontend/shared/views/RecommendedResourceCard.vue contentcuration/contentcuration/frontend/RecommendedResourceCard/components/RecommendedResourceCard.vue renamed to contentcuration/contentcuration/frontend/shared/views/RecommendedResourceCard.vue

@@ -1,3 +1,5 @@
+import uuid
+
 from automation.utils.appnexus import errors
 from django.urls import reverse
 from le_utils.constants import content_kinds
@@ -9,19 +11,20 @@
 from contentcuration.tests.base import StudioAPITestCase
 
 
-class CRUDTestCase(StudioAPITestCase):
+class RecommendationsCRUDTestCase(StudioAPITestCase):
     @property
     def topics(self):
         return {
             "topics": [
                 {
-                    "id": "00000000000000000000000000000001",
+                    "id": str(uuid.uuid4()),
+                    "channel_id": str(uuid.uuid4()),
                     "title": "Target topic",
                     "description": "Target description",
                     "language": "en",
                     "ancestors": [
                         {
-                            "id": "00000000000000000000000000000001",
+                            "id": str(uuid.uuid4()),
                             "title": "Parent topic",
                             "description": "Parent description",
                             "language": "en",
@@ -55,7 +58,7 @@ def recommendations_list(self):
         ]
 
     def setUp(self):
-        super(CRUDTestCase, self).setUp()
+        super(RecommendationsCRUDTestCase, self).setUp()
 
     @patch(
         "contentcuration.utils.automation_manager.AutomationManager.load_recommendations"

contentcuration/contentcuration/tests/viewsets/test_recommendations.py

@@ -5,7 +5,7 @@
 from automation.utils.appnexus import errors
 from django.http import HttpResponseServerError
 from django.http import JsonResponse
-from le_utils.validators import embed_topics_request
+from le_utils.constants import embed_topics_request
 from rest_framework.permissions import IsAuthenticated
 from rest_framework.views import APIView
 
@@ -15,6 +15,10 @@
 logger = logging.getLogger(__name__)
 
 
+def validate_recommendations_request(data):
+    jsonschema.validate(instance=data, schema=embed_topics_request.SCHEMA)
+
+
 class RecommendationView(APIView):
 
     permission_classes = [
@@ -29,7 +33,7 @@ def post(self, request):
             # Remove and store override_threshold as it isn't defined in the schema
             override_threshold = request_data.pop("override_threshold", False)
 
-            embed_topics_request.validate(request_data)
+            validate_recommendations_request(request_data)
         except jsonschema.ValidationError as e:
             logger.error("Schema validation error: %s", str(e))
             return JsonResponse(

contentcuration/contentcuration/viewsets/recommendation.py

@@ -71,7 +71,7 @@
     "jquery": "^2.2.4",
     "jspdf": "https://github.com/parallax/jsPDF.git#b7a1d8239c596292ce86dafa77f05987bcfa2e6e",
     "jszip": "^3.10.1",
-    "kolibri-constants": "^0.2.0",
+    "kolibri-constants": "^0.2.12",
     "kolibri-design-system": "5.2.0",
     "lodash": "^4.17.21",
     "material-icons": "0.3.1",

package.json

@@ -5,7 +5,7 @@ djangorestframework==3.15.1
 psycopg2-binary==2.9.10
 django-js-reverse==0.10.2
 django-registration==3.4
-le-utils==0.2.10
+le-utils>=0.2.12
 gunicorn==23.0.0
 django-postmark==0.1.6
 jsonfield==3.1.0

pnpm-lock.yaml

@@ -154,7 +154,7 @@ jsonschema-specifications==2024.10.1
     # via jsonschema
 kombu==5.5.2
     # via celery
-le-utils==0.2.10
+le-utils==0.2.12
     # via -r requirements.in
 packaging==25.0
     # via