Merge pull request learningequality#6046 from rtibbles/hotfixesintounstable

Hotfixes into unstable

Author: rtibbles

.github/workflows/pythontest.yml

@@ -66,6 +66,8 @@ jobs:
           - 6379:6379
     steps:
     - uses: actions/checkout@v7
+      with:
+        fetch-depth: 0
     - name: Set up minio
       run: |
         docker run -d -p 9000:9000 --name minio \
@@ -83,6 +85,17 @@ jobs:
       run: |
         # Use uv to install dependencies directly from requirements files
         uv pip sync requirements.txt requirements-dev.txt
+    - name: Lint new migrations for unsafe operations
+      if: github.event_name == 'pull_request'
+      env:
+        BASE_REF: ${{ github.base_ref }}
+        DJANGO_SETTINGS_MODULE: contentcuration.not_production_settings
+      run: |
+        set -euo pipefail
+        git fetch --no-tags origin "$BASE_REF"
+        base="$(git merge-base "origin/$BASE_REF" HEAD)"
+        test -n "$base"
+        python contentcuration/manage.py lintmigrations --git-commit-id "$base" --no-cache --warnings-as-errors
     - name: Test pytest
       run: |
         sh -c './contentcuration/manage.py makemigrations --check'

Makefile

@@ -39,7 +39,8 @@ migrate:
 # 4) Remove the management command from this `deploy-migrate` recipe
 # 5) Repeat!
 deploy-migrate:
-	echo "Nothing to do here!"
+	# studio#5974: remove at cutover.
+	python contentcuration/manage.py backfill_column --model contentcuration.File --source-field file_size --target-field file_size_bigint
 
 contentnodegc:
 	python contentcuration/manage.py garbage_collect

contentcuration/contentcuration/db/dual_write.py

@@ -0,0 +1,31 @@
+import hashlib
+
+import pgtrigger
+
+
+def mirror_field(source, target):
+    """Mirror Django field `source` into `target` via a BEFORE INSERT/UPDATE
+    trigger (expand/contract dual-write)."""
+
+    def decorator(model):
+        source_col = model._meta.get_field(source).column
+        target_col = model._meta.get_field(target).column
+        name = "mirror_{}_to_{}".format(source_col, target_col)
+        if len(name) > 43:  # stay safely under pgtrigger's trigger-name limit
+            digest = hashlib.sha1(
+                "{}_{}".format(source_col, target_col).encode()
+            ).hexdigest()[:8]
+            name = "mirror_{}".format(digest)
+        # Change-guard (IS DISTINCT FROM): keeps a read cutover from clobbering
+        # writes to the repointed column with the stale source value.
+        trigger = pgtrigger.Trigger(
+            name=name,
+            when=pgtrigger.Before,
+            operation=pgtrigger.Insert | pgtrigger.Update,
+            func="IF NEW.{s} IS DISTINCT FROM OLD.{s} THEN NEW.{t} = NEW.{s}; END IF; RETURN NEW;".format(
+                s=source_col, t=target_col
+            ),
+        )
+        return pgtrigger.register(trigger)(model)
+
+    return decorator

contentcuration/contentcuration/frontend/administration/router.js

@@ -34,11 +34,7 @@ const router = new VueRouter({
       name: RouteNames.COMMUNITY_LIBRARY_SUBMISSION,
       path: '/community-library/:channelId/:submissionId',
       component: SubmissionDetailsModal,
-      props: route => ({
-        channelId: route.params.channelId,
-        submissionId: route.params.submissionId,
-        adminReview: true,
-      }),
+      props: true,
     },
     // Catch-all redirect to channels tab
     {

contentcuration/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditor.vue

@@ -200,7 +200,10 @@
         return editor.value.storage.markdown.getMarkdown();
       };
 
-      let isUpdatingFromOutside = false; // A flag to prevent infinite update loops
+      // Cache of the latest markdown value emitted by this component. Used to
+      // detect when an incoming prop.value is just our own emitted value echoed
+      // back, so we can skip unnecessary re-rendering of the editor content.
+      let lastEmittedMarkdown = null;
 
       watch(
         () => props.mode,
@@ -220,6 +223,13 @@
       watch(
         () => props.value,
         newValue => {
+          // If the incoming value matches what we last emitted, the editor
+          // already reflects this content, so skip re-rendering to avoid
+          // unnecessary work and resetting the editor state.
+          if (newValue === lastEmittedMarkdown) {
+            return;
+          }
+
           const processedContent =
             props.format === 'html' ? newValue : preprocessMarkdown(newValue);
 
@@ -231,29 +241,31 @@
           }
 
           if (getContent() !== newValue) {
-            isUpdatingFromOutside = true;
             editor.value.commands.setContent(processedContent, false);
-            nextTick(() => {
-              isUpdatingFromOutside = false;
-            });
           }
         },
         { immediate: true },
       );
 
-      // sync changes from the editor to the parent component
-      watch(
-        () => editor.value?.state,
-        () => {
-          if (!editor.value || !isReady.value || isUpdatingFromOutside) return;
+      // sync changes from the editor to the parent component, only on blur
+      const emitContentUpdate = () => {
+        if (!editor.value || !isReady.value) {
+          return;
+        }
 
-          const content = getContent();
-          if (content !== props.value) {
-            emit('update', content);
-          }
-        },
-        { deep: true },
-      );
+        const content = getContent();
+        if (content !== props.value) {
+          lastEmittedMarkdown = content;
+          emit('update', content);
+        }
+      };
+
+      // Emit the content update only when the editor loses focus (blur).
+      watch(isFocused, (focused, wasFocused) => {
+        if (wasFocused && !focused) {
+          emitContentUpdate();
+        }
+      });
 
       const handleContainerKeydown = event => {
         if (event.key === 'Enter') {

contentcuration/contentcuration/frontend/shared/views/communityLibrary/SubmissionDetailsModal/__tests__/index.spec.js

@@ -0,0 +1,81 @@
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import Vuex from 'vuex';
+import VueRouter from 'vue-router';
+import SubmissionDetailsModal from '../index.vue';
+import {
+  AdminCommunityLibrarySubmission,
+  ChannelVersion,
+  CommunityLibrarySubmission,
+} from 'shared/data/resources';
+
+jest.mock('shared/data/resources', () => ({
+  AdminCommunityLibrarySubmission: { fetchModel: jest.fn() },
+  ChannelVersion: { fetchCollection: jest.fn() },
+  CommunityLibrarySubmission: {
+    fetchModel: jest.fn(),
+    fetchCollection: jest.fn(() => Promise.resolve({ results: [] })),
+  },
+}));
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+localVue.use(VueRouter);
+
+const stubChannel = {
+  id: 'ch1',
+  name: 'Test',
+  thumbnail_url: null,
+  thumbnail_encoding: null,
+  description: '',
+};
+const stubSubmission = {
+  id: 'sub1',
+  channel_id: 'ch1',
+  channel_version: 1,
+  status: 'PENDING',
+  version_token: null,
+};
+const stubChannelVersion = { id: 'cv1' };
+
+function makeStore(isAdmin) {
+  return new Vuex.Store({
+    getters: { isAdmin: () => isAdmin },
+    modules: {
+      channel: {
+        namespaced: true,
+        actions: { loadChannel: jest.fn(() => Promise.resolve(stubChannel)) },
+      },
+      errors: { namespaced: true, actions: { handleAxiosError: jest.fn() } },
+    },
+  });
+}
+
+describe('SubmissionDetailsModal', () => {
+  beforeEach(() => {
+    AdminCommunityLibrarySubmission.fetchModel.mockResolvedValue(stubSubmission);
+    CommunityLibrarySubmission.fetchModel.mockResolvedValue(stubSubmission);
+    ChannelVersion.fetchCollection.mockResolvedValue([stubChannelVersion]);
+  });
+
+  afterEach(() => jest.clearAllMocks());
+
+  it('uses AdminCommunityLibrarySubmission when user is admin', () => {
+    shallowMount(SubmissionDetailsModal, {
+      localVue,
+      store: makeStore(true),
+      router: new VueRouter(),
+      propsData: { channelId: 'ch1', submissionId: 'sub1' },
+    });
+    expect(AdminCommunityLibrarySubmission.fetchModel).toHaveBeenCalledWith('sub1');
+  });
+
+  it('uses CommunityLibrarySubmission when user is not admin', () => {
+    shallowMount(SubmissionDetailsModal, {
+      localVue,
+      store: makeStore(false),
+      router: new VueRouter(),
+      propsData: { channelId: 'ch1', submissionId: 'sub1' },
+    });
+    expect(CommunityLibrarySubmission.fetchModel).toHaveBeenCalledWith('sub1');
+  });
+});

contentcuration/contentcuration/frontend/shared/views/communityLibrary/SubmissionDetailsModal/index.vue

@@ -74,12 +74,12 @@
         </div>
         <div class="actions">
           <KButton
-            v-if="adminReview && submission.status === CommunityLibraryStatus.PENDING"
+            v-if="isAdmin && submission.status === CommunityLibraryStatus.PENDING"
             :text="reviewAction$()"
             @click="showReviewSidePanel = true"
           />
           <ChannelActionsDropdown
-            v-if="adminReview"
+            v-if="isAdmin"
             primary
             :channelId="channelId"
           />
@@ -105,7 +105,7 @@
         :channelId="channelId"
       />
       <ReviewSubmissionSidePanel
-        v-if="adminReview && showReviewSidePanel"
+        v-if="isAdmin && showReviewSidePanel"
         :submissionId="submission.id"
         :channel="channel"
         @close="showReviewSidePanel = false"
@@ -143,10 +143,6 @@
   import logging from 'shared/logging';
 
   const props = defineProps({
-    adminReview: {
-      type: Boolean,
-      default: false,
-    },
     channelId: {
       type: String,
       required: true,
@@ -163,6 +159,7 @@
   const route = useRoute();
   const router = useRouter();
   const store = useStore();
+  const isAdmin = computed(() => store.getters.isAdmin);
   const { windowBreakpoint } = useKResponsiveWindow();
 
   const isModalOpen = computed({
@@ -206,7 +203,7 @@
   } = useFetch({
     asyncFetchFunc: async () => {
       try {
-        const Resource = props.adminReview
+        const Resource = isAdmin.value
           ? AdminCommunityLibrarySubmission
           : CommunityLibrarySubmission;
         const submission = await Resource.fetchModel(props.submissionId);

contentcuration/contentcuration/management/commands/backfill_column.py

@@ -0,0 +1,100 @@
+from django.apps import apps
+from django.core.exceptions import FieldDoesNotExist
+from django.core.management.base import BaseCommand
+from django.core.management.base import CommandError
+from django.db import transaction
+from django.db.models import F
+
+
+class Command(BaseCommand):
+    help = (
+        "Idempotent, resumable online backfill of one column into another, in batches."
+    )
+
+    def add_arguments(self, parser):
+        parser.add_argument("--model", required=True, help="app_label.ModelName")
+        parser.add_argument("--source-field", required=True)
+        parser.add_argument("--target-field", required=True)
+        parser.add_argument("--batch-size", type=int, default=10000)
+        parser.add_argument("--start-id", default=None, help="resume from this pk")
+        parser.add_argument(
+            "--progress-check",
+            action="store_true",
+            help="report unbackfilled rows, exit nonzero if any",
+        )
+
+    def _resolve_model_fields(self, model_label, source, target):
+        try:
+            model = apps.get_model(model_label)
+        except (LookupError, ValueError) as e:
+            raise CommandError("Bad --model {!r}: {}".format(model_label, e))
+        try:
+            model._meta.get_field(source)
+            model._meta.get_field(target)
+        except FieldDoesNotExist as e:
+            raise CommandError(str(e))
+        return model
+
+    def _batch_end_pk(self, queryset, pk_name, start_pk, batch_size):
+        """Last pk of the batch of `batch_size` rows starting at `start_pk`.
+
+        Returns None when fewer than `batch_size` rows remain at/after
+        `start_pk` — the final, short batch. Keyset paging by pk, so it works
+        for any pk type (int or UUID).
+        """
+        return (
+            queryset.filter(pk__gte=start_pk)
+            .order_by(pk_name)
+            .values_list("pk", flat=True)[batch_size - 1 : batch_size]
+            .first()
+        )
+
+    def handle(self, *args, **options):
+        if options["batch_size"] < 1:
+            raise CommandError("--batch-size must be >= 1")
+        source = options["source_field"]
+        target = options["target_field"]
+        model = self._resolve_model_fields(options["model"], source, target)
+
+        pk_name = model._meta.pk.name
+        batch_size = options["batch_size"]
+        only_unfilled = {target + "__isnull": True, source + "__isnull": False}
+        unfilled = model.objects.filter(**only_unfilled)
+        unfilled_pks = unfilled.order_by(pk_name).values_list("pk", flat=True)
+
+        if options["progress_check"]:
+            # exists(), not count() — the target table can have millions of rows.
+            if unfilled.exists():
+                raise CommandError("backfill incomplete: rows still pending")
+            self.stdout.write("Backfill complete: no rows pending.")
+            return
+
+        # Start at the first unfilled pk (>= --start-id if given); re-runs and
+        # resumes skip straight past an already-filled prefix.
+        batch_start = unfilled_pks
+        if options["start_id"] is not None:
+            batch_start = batch_start.filter(pk__gte=options["start_id"])
+        batch_start = batch_start.first()
+
+        total = 0
+        while batch_start is not None:
+            batch_end = self._batch_end_pk(
+                model.objects, pk_name, batch_start, batch_size
+            )
+            if batch_end is None:
+                window = {"pk__gte": batch_start}
+            else:
+                window = {"pk__gte": batch_start, "pk__lte": batch_end}
+            with transaction.atomic():
+                total += model.objects.filter(**window, **only_unfilled).update(
+                    **{target: F(source)}
+                )
+            self.stdout.write(
+                "backfilled through pk={} (updated {} so far)".format(
+                    batch_start if batch_end is None else batch_end, total
+                )
+            )
+            if batch_end is None:
+                break
+            batch_start = unfilled_pks.filter(pk__gt=batch_end).first()
+        self.stdout.write("Done. {} rows updated.".format(total))