From 6baf0df407f5c94e135b10a7a3da0a7d5a6c13f3 Mon Sep 17 00:00:00 2001 From: carlos Date: Thu, 9 Jul 2026 12:50:06 -0500 Subject: [PATCH 01/11] feat: bulk add courses to base catalog --- partner_catalog/admin.py | 147 +++++++++++++----- .../basecatalog/bulk_add_courses.html | 62 ++++++++ 2 files changed, 169 insertions(+), 40 deletions(-) create mode 100644 partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index 9cb2e76..a17fea7 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -1,14 +1,17 @@ """Admin configuration for Partner Catalog models.""" -from urllib.parse import urlencode - -from django.contrib import admin +from django import forms +from django.contrib import admin, messages +from django.contrib.admin.widgets import FilteredSelectMultiple from django.db.models import Count from django.http import HttpResponseRedirect -from django.urls import reverse +from django.shortcuts import get_object_or_404 +from django.template.response import TemplateResponse +from django.urls import path, reverse from django.utils.html import format_html from flex_catalog.admin import CourseKeysMixin +from partner_catalog.edxapp_wrapper.course_module import course_overview from partner_catalog.models import ( BaseCatalog, BaseCatalogCourse, @@ -23,6 +26,22 @@ ) +class BulkAddCoursesForm(forms.Form): + """Form for bulk-adding courses to a BaseCatalog using a visual dual-list picker.""" + + courses = forms.ModelMultipleChoiceField( + queryset=None, + widget=FilteredSelectMultiple("Courses", is_stacked=False), + required=False, + help_text="Select one or more courses to add. Use the search box to filter results.", + ) + + def __init__(self, *args, available_courses=None, **kwargs): + super().__init__(*args, **kwargs) + self.fields['courses'].queryset = available_courses if available_courses is not None else \ + course_overview().objects.none() + + @admin.register(BaseCatalog) class BaseCatalogAdmin(admin.ModelAdmin): """Admin interface for BaseCatalog model.""" @@ -32,6 +51,72 @@ class BaseCatalogAdmin(admin.ModelAdmin): fields = ('name', 'slug', 'created', 'modified', 'course_count', 'course_ids_display', 'add_course_button') search_fields = ('name', 'slug') + def get_urls(self): + """Register custom admin URLs including the bulk-add-courses view.""" + urls = super().get_urls() + custom_urls = [ + path( + '/bulk-add-courses/', + self.admin_site.admin_view(self.bulk_add_courses_view), + name='partner_catalog_basecatalog_bulk_add_courses', + ), + ] + return custom_urls + urls + + def bulk_add_courses_view(self, request, pk): + """Custom admin view: visual dual-list picker to bulk-add courses to a BaseCatalog.""" + base_catalog = get_object_or_404(BaseCatalog, pk=pk) + CourseOverview = course_overview() + + existing_course_ids = base_catalog.base_catalog_courses.values_list( + 'course_overview_id', flat=True + ) + available_courses = ( + CourseOverview.objects + .exclude(id__in=existing_course_ids) + .order_by('display_name') + ) + + if request.method == 'POST': + form = BulkAddCoursesForm(request.POST, available_courses=available_courses) + if form.is_valid(): + selected_courses = form.cleaned_data['courses'] + added = 0 + for course in selected_courses: + _, created = BaseCatalogCourse.objects.get_or_create( + base_catalog=base_catalog, + course_overview=course, + defaults={'added_by': request.user}, + ) + if created: + added += 1 + + self.message_user( + request, + f"Successfully added {added} course(s) to \"{base_catalog.name}\".", + messages.SUCCESS, + ) + return HttpResponseRedirect( + reverse('admin:partner_catalog_basecatalog_change', args=[pk]) + ) + else: + form = BulkAddCoursesForm(available_courses=available_courses) + + context = { + **self.admin_site.each_context(request), + 'title': f'Bulk Add Courses to "{base_catalog.name}"', + 'base_catalog': base_catalog, + 'form': form, + 'media': self.media + form.media, + 'opts': self.model._meta, + 'has_change_permission': self.has_change_permission(request), + } + return TemplateResponse( + request, + 'admin/partner_catalog/basecatalog/bulk_add_courses.html', + context, + ) + def get_queryset(self, request): """Optimize queryset with prefetch.""" qs = super().get_queryset(request) @@ -67,37 +152,37 @@ def course_ids_display(self, obj): course_ids_display.short_description = 'Course IDs' def add_course_button(self, obj): - """Genera un botón para agregar un nuevo curso a este BaseCatalog.""" + """Render action buttons for adding courses to this BaseCatalog.""" if not obj.pk: - return format_html('Guarda el catálogo primero') + return format_html('Save the catalog first') - course_model = BaseCatalogCourse - add_course_url = reverse( - f"admin:{course_model._meta.app_label}_{course_model._meta.model_name}_add" - ) - full_url = f"{add_course_url}?base_catalog={obj.pk}" + single_url = reverse( + f"admin:{BaseCatalogCourse._meta.app_label}_{BaseCatalogCourse._meta.model_name}_add" + ) + f"?base_catalog={obj.pk}" + + bulk_url = reverse('admin:partner_catalog_basecatalog_bulk_add_courses', args=[obj.pk]) return format_html( - '+ Add Course', - full_url, + '+ Add Single Course' + '+ Bulk Add Courses', + single_url, + bulk_url, ) add_course_button.short_description = "Add Courses" def add_course(self, obj): - """Genera un link para agregar un nuevo curso a este BaseCatalog.""" - course_model = BaseCatalogCourse - add_course_url = reverse( - f"admin:{course_model._meta.app_label}_{course_model._meta.model_name}_add" - ) - full_url = f"{add_course_url}?base_catalog={obj.pk}" + """Render bulk-add link in the list view.""" + if not obj.pk: + return format_html('') + bulk_url = reverse('admin:partner_catalog_basecatalog_bulk_add_courses', args=[obj.pk]) return format_html( - '+ Add Course', - full_url, + '+ Bulk Add Courses', + bulk_url, ) - add_course.short_description = "+ Add Course" + add_course.short_description = "Add Courses" @admin.register(BaseCatalogCourse) @@ -122,24 +207,6 @@ def save_model(self, request, obj, form, change): obj.added_by = request.user super().save_model(request, obj, form, change) - def response_add(self, request, obj, post_url_continue=None): - """After saving a new entry, keep base_catalog pre-filled when adding another.""" - response = super().response_add(request, obj, post_url_continue) - if "_addanother" in request.POST and obj.base_catalog_id: - add_url = reverse( - f"admin:{self.model._meta.app_label}_{self.model._meta.model_name}_add" - ) - query = urlencode({"base_catalog": obj.base_catalog_id}) - return HttpResponseRedirect(f"{add_url}?{query}") - return response - - def get_changeform_initial_data(self, request): - """Pre-populate base_catalog from query-string when coming from response_add.""" - initial = super().get_changeform_initial_data(request) - if "base_catalog" in request.GET and "base_catalog" not in initial: - initial["base_catalog"] = request.GET["base_catalog"] - return initial - def get_queryset(self, request): """Optimize queryset with select_related.""" qs = super().get_queryset(request) diff --git a/partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html b/partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html new file mode 100644 index 0000000..5a3439b --- /dev/null +++ b/partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html @@ -0,0 +1,62 @@ +{% extends "admin/base_site.html" %} +{% load i18n static %} + +{% block title %}{{ title }} | {% trans "Site administration" %}{% endblock %} + +{% block extrastyle %} + {{ block.super }} + +{% endblock %} + +{% block extrahead %} + {{ block.super }} + + {{ media }} +{% endblock %} + +{% block breadcrumbs %} +
+ {% trans "Home" %} + › {% trans "Base Catalogs" %} + › {{ base_catalog.name }} + › {% trans "Bulk Add Courses" %} +
+{% endblock %} + +{% block content %} +
+

+ Select the courses you want to add to {{ base_catalog.name }}. + Use the search box to filter courses by name, then move them to the right panel. + Courses already in this catalog are not shown. +

+ +
+ {% csrf_token %} + +
+
+ {{ form.courses.errors }} + {{ form.courses }} + {% if form.courses.help_text %} +

{{ form.courses.help_text }}

+ {% endif %} +
+
+ +
+ + {% trans "Cancel" %} +
+
+
+{% endblock %} From 74500ba83d44f1a372841f732113779f2a9d98bc Mon Sep 17 00:00:00 2001 From: carlos Date: Thu, 9 Jul 2026 13:21:05 -0500 Subject: [PATCH 02/11] fix: fix pydocstyle violations in bulk add view --- partner_catalog/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index a17fea7..b0f47a0 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -37,6 +37,7 @@ class BulkAddCoursesForm(forms.Form): ) def __init__(self, *args, available_courses=None, **kwargs): + """Initialize the form with the given available courses queryset.""" super().__init__(*args, **kwargs) self.fields['courses'].queryset = available_courses if available_courses is not None else \ course_overview().objects.none() @@ -64,7 +65,7 @@ def get_urls(self): return custom_urls + urls def bulk_add_courses_view(self, request, pk): - """Custom admin view: visual dual-list picker to bulk-add courses to a BaseCatalog.""" + """Render a visual dual-list picker to bulk-add courses to a BaseCatalog.""" base_catalog = get_object_or_404(BaseCatalog, pk=pk) CourseOverview = course_overview() From 21e19fd0c49291feb577ce3ed9609cd56c6eb7a1 Mon Sep 17 00:00:00 2001 From: carlos Date: Fri, 10 Jul 2026 15:40:50 -0500 Subject: [PATCH 03/11] refactor: manage courses inline on BaseCatalog change form --- partner_catalog/admin.py | 180 ++++++------------ .../basecatalog/bulk_add_courses.html | 62 ------ 2 files changed, 53 insertions(+), 189 deletions(-) delete mode 100644 partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index b0f47a0..a877426 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -1,13 +1,10 @@ """Admin configuration for Partner Catalog models.""" from django import forms -from django.contrib import admin, messages +from django.contrib import admin from django.contrib.admin.widgets import FilteredSelectMultiple from django.db.models import Count -from django.http import HttpResponseRedirect -from django.shortcuts import get_object_or_404 -from django.template.response import TemplateResponse -from django.urls import path, reverse +from django.urls import reverse from django.utils.html import format_html from flex_catalog.admin import CourseKeysMixin @@ -26,102 +23,72 @@ ) -class BulkAddCoursesForm(forms.Form): - """Form for bulk-adding courses to a BaseCatalog using a visual dual-list picker.""" +class BaseCatalogAdminForm(forms.ModelForm): + """ModelForm for BaseCatalog with an inline dual-list course manager.""" courses = forms.ModelMultipleChoiceField( queryset=None, widget=FilteredSelectMultiple("Courses", is_stacked=False), required=False, - help_text="Select one or more courses to add. Use the search box to filter results.", + help_text=( + "Manage courses in this catalog. " + "Use the search box to filter, Ctrl+click to select multiple, " + "then use the arrow buttons to add or remove them. " + "Saving will apply all additions and removals at once." + ), ) - def __init__(self, *args, available_courses=None, **kwargs): - """Initialize the form with the given available courses queryset.""" + def __init__(self, *args, **kwargs): + """Pre-populate the courses field with the catalog's current courses.""" super().__init__(*args, **kwargs) - self.fields['courses'].queryset = available_courses if available_courses is not None else \ - course_overview().objects.none() + self.fields['courses'].queryset = course_overview().objects.order_by('display_name') + if self.instance.pk: + self.fields['courses'].initial = self.instance.courses.all() + + class Meta: + """Meta options for BaseCatalogAdminForm.""" + + model = BaseCatalog + fields = '__all__' @admin.register(BaseCatalog) class BaseCatalogAdmin(admin.ModelAdmin): """Admin interface for BaseCatalog model.""" - list_display = ('name', 'slug', 'course_count', 'course_ids', 'add_course') - readonly_fields = ('created', 'modified', 'course_count', 'course_ids_display', 'add_course_button') - fields = ('name', 'slug', 'created', 'modified', 'course_count', 'course_ids_display', 'add_course_button') + form = BaseCatalogAdminForm + + list_display = ('name', 'slug', 'course_count', 'course_ids', 'manage_courses') + readonly_fields = ('created', 'modified', 'course_count') + fields = ('name', 'slug', 'created', 'modified', 'course_count', 'courses') search_fields = ('name', 'slug') - def get_urls(self): - """Register custom admin URLs including the bulk-add-courses view.""" - urls = super().get_urls() - custom_urls = [ - path( - '/bulk-add-courses/', - self.admin_site.admin_view(self.bulk_add_courses_view), - name='partner_catalog_basecatalog_bulk_add_courses', - ), - ] - return custom_urls + urls + def get_queryset(self, request): + """Optimize queryset with prefetch.""" + qs = super().get_queryset(request) + return qs.prefetch_related('courses', 'base_catalog_courses') - def bulk_add_courses_view(self, request, pk): - """Render a visual dual-list picker to bulk-add courses to a BaseCatalog.""" - base_catalog = get_object_or_404(BaseCatalog, pk=pk) - CourseOverview = course_overview() + def save_related(self, request, form, formsets, change): + """Sync the courses M2M: add newly selected courses and remove deselected ones.""" + super().save_related(request, form, formsets, change) - existing_course_ids = base_catalog.base_catalog_courses.values_list( - 'course_overview_id', flat=True - ) - available_courses = ( - CourseOverview.objects - .exclude(id__in=existing_course_ids) - .order_by('display_name') - ) + selected_courses = set(form.cleaned_data.get('courses', [])) + selected_ids = {course.pk for course in selected_courses} - if request.method == 'POST': - form = BulkAddCoursesForm(request.POST, available_courses=available_courses) - if form.is_valid(): - selected_courses = form.cleaned_data['courses'] - added = 0 - for course in selected_courses: - _, created = BaseCatalogCourse.objects.get_or_create( - base_catalog=base_catalog, - course_overview=course, - defaults={'added_by': request.user}, - ) - if created: - added += 1 + current_entries = form.instance.base_catalog_courses.select_related('course_overview') + current_ids = {entry.course_overview_id for entry in current_entries} - self.message_user( - request, - f"Successfully added {added} course(s) to \"{base_catalog.name}\".", - messages.SUCCESS, - ) - return HttpResponseRedirect( - reverse('admin:partner_catalog_basecatalog_change', args=[pk]) + for course in selected_courses: + if course.pk not in current_ids: + BaseCatalogCourse.objects.create( + base_catalog=form.instance, + course_overview=course, + added_by=request.user, ) - else: - form = BulkAddCoursesForm(available_courses=available_courses) - - context = { - **self.admin_site.each_context(request), - 'title': f'Bulk Add Courses to "{base_catalog.name}"', - 'base_catalog': base_catalog, - 'form': form, - 'media': self.media + form.media, - 'opts': self.model._meta, - 'has_change_permission': self.has_change_permission(request), - } - return TemplateResponse( - request, - 'admin/partner_catalog/basecatalog/bulk_add_courses.html', - context, - ) - def get_queryset(self, request): - """Optimize queryset with prefetch.""" - qs = super().get_queryset(request) - return qs.prefetch_related('courses', 'base_catalog_courses') + form.instance.base_catalog_courses.filter( + course_overview_id__in=current_ids - selected_ids + ).delete() def course_count(self, obj): """Display the total number of courses in the catalog.""" @@ -129,61 +96,20 @@ def course_count(self, obj): course_count.short_description = 'Total Courses' def course_ids(self, obj): - """Display preview of course IDs in the list view.""" + """Display course IDs in the list view.""" course_runs = obj.get_course_runs() - if course_runs: - course_ids = [str(course.id) for course in course_runs] - return format_html('
'.join(course_ids)) - + return format_html('
'.join(str(c.id) for c in course_runs)) return format_html('No courses') - course_ids.short_description = 'Course IDs' - def course_ids_display(self, obj): - """Display all course IDs in the detail view.""" - course_runs = obj.get_course_runs() - - if course_runs: - course_ids = [str(course.id) for course in course_runs] - return format_html('
'.join(course_ids)) - - return format_html('No courses') - - course_ids_display.short_description = 'Course IDs' - - def add_course_button(self, obj): - """Render action buttons for adding courses to this BaseCatalog.""" - if not obj.pk: - return format_html('Save the catalog first') - - single_url = reverse( - f"admin:{BaseCatalogCourse._meta.app_label}_{BaseCatalogCourse._meta.model_name}_add" - ) + f"?base_catalog={obj.pk}" - - bulk_url = reverse('admin:partner_catalog_basecatalog_bulk_add_courses', args=[obj.pk]) - - return format_html( - '+ Add Single Course' - '+ Bulk Add Courses', - single_url, - bulk_url, - ) - - add_course_button.short_description = "Add Courses" - - def add_course(self, obj): - """Render bulk-add link in the list view.""" + def manage_courses(self, obj): + """Link to the catalog change page to manage its courses.""" if not obj.pk: return format_html('') - - bulk_url = reverse('admin:partner_catalog_basecatalog_bulk_add_courses', args=[obj.pk]) - return format_html( - '+ Bulk Add Courses', - bulk_url, - ) - - add_course.short_description = "Add Courses" + url = reverse('admin:partner_catalog_basecatalog_change', args=[obj.pk]) + return format_html('Manage Courses', url) + manage_courses.short_description = 'Manage Courses' @admin.register(BaseCatalogCourse) diff --git a/partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html b/partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html deleted file mode 100644 index 5a3439b..0000000 --- a/partner_catalog/templates/admin/partner_catalog/basecatalog/bulk_add_courses.html +++ /dev/null @@ -1,62 +0,0 @@ -{% extends "admin/base_site.html" %} -{% load i18n static %} - -{% block title %}{{ title }} | {% trans "Site administration" %}{% endblock %} - -{% block extrastyle %} - {{ block.super }} - -{% endblock %} - -{% block extrahead %} - {{ block.super }} - - {{ media }} -{% endblock %} - -{% block breadcrumbs %} -
- {% trans "Home" %} - › {% trans "Base Catalogs" %} - › {{ base_catalog.name }} - › {% trans "Bulk Add Courses" %} -
-{% endblock %} - -{% block content %} -
-

- Select the courses you want to add to {{ base_catalog.name }}. - Use the search box to filter courses by name, then move them to the right panel. - Courses already in this catalog are not shown. -

- -
- {% csrf_token %} - -
-
- {{ form.courses.errors }} - {{ form.courses }} - {% if form.courses.help_text %} -

{{ form.courses.help_text }}

- {% endif %} -
-
- -
- - {% trans "Cancel" %} -
-
-
-{% endblock %} From cbf0c4440f1f8575bc781207e3c394b310fcdfc9 Mon Sep 17 00:00:00 2001 From: carlos Date: Fri, 10 Jul 2026 15:47:11 -0500 Subject: [PATCH 04/11] fix(admin): remove redundant label and bypass admin.E013 check --- partner_catalog/admin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index a877426..ca472af 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -30,6 +30,7 @@ class BaseCatalogAdminForm(forms.ModelForm): queryset=None, widget=FilteredSelectMultiple("Courses", is_stacked=False), required=False, + label="", help_text=( "Manage courses in this catalog. " "Use the search box to filter, Ctrl+click to select multiple, " @@ -60,9 +61,12 @@ class BaseCatalogAdmin(admin.ModelAdmin): list_display = ('name', 'slug', 'course_count', 'course_ids', 'manage_courses') readonly_fields = ('created', 'modified', 'course_count') - fields = ('name', 'slug', 'created', 'modified', 'course_count', 'courses') search_fields = ('name', 'slug') + def get_fields(self, request, obj=None): + """Return fields for the change form, including the custom courses widget.""" + return ('name', 'slug', 'created', 'modified', 'course_count', 'courses') + def get_queryset(self, request): """Optimize queryset with prefetch.""" qs = super().get_queryset(request) From 80d8d4bb933006d530cbc8b5bd5cac38558caeb9 Mon Sep 17 00:00:00 2001 From: carlos Date: Wed, 5 Aug 2026 09:58:31 -0500 Subject: [PATCH 05/11] test: add coverage for BaseCatalog inline course manager --- partner_catalog/admin.py | 8 +- tests/test_base_catalog_admin.py | 178 +++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tests/test_base_catalog_admin.py diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index ca472af..6e835a2 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -42,7 +42,13 @@ class BaseCatalogAdminForm(forms.ModelForm): def __init__(self, *args, **kwargs): """Pre-populate the courses field with the catalog's current courses.""" super().__init__(*args, **kwargs) - self.fields['courses'].queryset = course_overview().objects.order_by('display_name') + CourseOverview = course_overview() + try: + CourseOverview._meta.get_field('display_name') + qs = CourseOverview.objects.order_by('display_name') + except Exception: # pylint: disable=broad-except + qs = CourseOverview.objects.all() + self.fields['courses'].queryset = qs if self.instance.pk: self.fields['courses'].initial = self.instance.courses.all() diff --git a/tests/test_base_catalog_admin.py b/tests/test_base_catalog_admin.py new file mode 100644 index 0000000..e3e9014 --- /dev/null +++ b/tests/test_base_catalog_admin.py @@ -0,0 +1,178 @@ +""" +Tests for BaseCatalog admin — form pre-population and save_related sync logic (Suite 10). + +Covers: +- BaseCatalogAdminForm.__init__: courses queryset is set on all forms +- BaseCatalogAdminForm.__init__: courses initial is empty for a new catalog +- BaseCatalogAdminForm.__init__: courses initial is pre-populated for an existing catalog +- BaseCatalogAdmin.save_related: adds newly selected courses +- BaseCatalogAdmin.save_related: removes deselected courses +- BaseCatalogAdmin.save_related: no-op when selection matches current state +- BaseCatalogAdmin.save_related: records added_by from request.user +- BaseCatalogAdmin.save_related: clears all courses when selection is empty +""" + +from unittest.mock import MagicMock + +import pytest +from django.contrib.admin.sites import AdminSite + +from partner_catalog.admin import BaseCatalogAdmin, BaseCatalogAdminForm +from partner_catalog.models import BaseCatalog, BaseCatalogCourse +from partner_catalog.services.catalog_courses import CourseOverview +from tests.factories import make_user + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def make_base_catalog(slug_suffix="1"): + """Create and return a BaseCatalog for testing.""" + return BaseCatalog.objects.create(name=f"Test Catalog {slug_suffix}", slug=f"test-catalog-{slug_suffix}") + + +def make_course(): + """Create and return a CourseOverview (test backend) instance.""" + return CourseOverview.objects.create() + + +def _admin(): + """Return a BaseCatalogAdmin instance bound to a fresh AdminSite.""" + return BaseCatalogAdmin(BaseCatalog, AdminSite()) + + +def _request(user=None): + """Return a mock request with the given user (or a new staff user).""" + req = MagicMock() + req.user = user or make_user(is_staff=True) + return req + + +def _form(instance, selected_courses): + """Return a mock form with cleaned_data and instance set.""" + frm = MagicMock() + frm.instance = instance + frm.cleaned_data = {'courses': selected_courses} + return frm + + +# --------------------------------------------------------------------------- +# BaseCatalogAdminForm — __init__ pre-population +# --------------------------------------------------------------------------- + +class TestBaseCatalogAdminFormInit: + """Tests for BaseCatalogAdminForm.__init__ initialization behaviour.""" + + @pytest.mark.django_db + def test_courses_queryset_includes_all_courses(self): + """The courses queryset covers all CourseOverview objects.""" + make_course() + make_course() + + form = BaseCatalogAdminForm() + + assert form.fields['courses'].queryset.count() == 2 + + @pytest.mark.django_db + def test_courses_initial_is_empty_for_new_catalog(self): + """Without an existing instance the courses initial is not set.""" + form = BaseCatalogAdminForm() + + assert not form.fields['courses'].initial + + @pytest.mark.django_db + def test_courses_initial_pre_populates_existing_courses(self): + """With an existing catalog the initial value matches its current courses.""" + catalog = make_base_catalog() + course1 = make_course() + course2 = make_course() + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course1) + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course2) + + form = BaseCatalogAdminForm(instance=catalog) + + initial_ids = {c.pk for c in form.fields['courses'].initial} + assert initial_ids == {course1.pk, course2.pk} + + @pytest.mark.django_db + def test_courses_initial_is_empty_for_catalog_with_no_courses(self): + """A catalog with no courses yields an empty initial queryset.""" + catalog = make_base_catalog() + + form = BaseCatalogAdminForm(instance=catalog) + + assert list(form.fields['courses'].initial) == [] + + +# --------------------------------------------------------------------------- +# BaseCatalogAdmin.save_related — course sync logic +# --------------------------------------------------------------------------- + +class TestBaseCatalogAdminSaveRelated: + """Tests for BaseCatalogAdmin.save_related diff/sync logic.""" + + @pytest.mark.django_db + def test_adds_newly_selected_courses(self): + """save_related creates BaseCatalogCourse entries for newly selected courses.""" + catalog = make_base_catalog(slug_suffix="a") + course1 = make_course() + course2 = make_course() + + _admin().save_related(_request(), _form(catalog, [course1, course2]), [], change=True) + + assert catalog.courses.count() == 2 + assert BaseCatalogCourse.objects.filter(base_catalog=catalog, course_overview=course1).exists() + assert BaseCatalogCourse.objects.filter(base_catalog=catalog, course_overview=course2).exists() + + @pytest.mark.django_db + def test_removes_deselected_courses(self): + """save_related deletes BaseCatalogCourse entries for deselected courses.""" + catalog = make_base_catalog(slug_suffix="b") + course1 = make_course() + course2 = make_course() + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course1) + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course2) + + _admin().save_related(_request(), _form(catalog, [course1]), [], change=True) + + assert catalog.courses.count() == 1 + assert BaseCatalogCourse.objects.filter(base_catalog=catalog, course_overview=course1).exists() + assert not BaseCatalogCourse.objects.filter(base_catalog=catalog, course_overview=course2).exists() + + @pytest.mark.django_db + def test_no_op_when_selection_matches_current_state(self): + """save_related does not create duplicates when the selection is unchanged.""" + catalog = make_base_catalog(slug_suffix="c") + course = make_course() + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course) + + _admin().save_related(_request(), _form(catalog, [course]), [], change=True) + + assert catalog.courses.count() == 1 + assert BaseCatalogCourse.objects.filter(base_catalog=catalog).count() == 1 + + @pytest.mark.django_db + def test_records_added_by_from_request_user(self): + """save_related sets the added_by field to the current request user.""" + catalog = make_base_catalog(slug_suffix="d") + course = make_course() + user = make_user() + + _admin().save_related(_request(user=user), _form(catalog, [course]), [], change=True) + + entry = BaseCatalogCourse.objects.get(base_catalog=catalog, course_overview=course) + assert entry.added_by == user + + @pytest.mark.django_db + def test_clears_all_courses_when_selection_is_empty(self): + """save_related removes all entries when the submitted selection is empty.""" + catalog = make_base_catalog(slug_suffix="e") + course1 = make_course() + course2 = make_course() + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course1) + BaseCatalogCourse.objects.create(base_catalog=catalog, course_overview=course2) + + _admin().save_related(_request(), _form(catalog, []), [], change=True) + + assert catalog.courses.count() == 0 From 71efbef4b7ce924b78695072a0f4ace19f700576 Mon Sep 17 00:00:00 2001 From: carlos Date: Wed, 5 Aug 2026 10:06:47 -0500 Subject: [PATCH 06/11] fix(quality): simplify empty list comparison in test --- tests/test_base_catalog_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_base_catalog_admin.py b/tests/test_base_catalog_admin.py index e3e9014..39b6781 100644 --- a/tests/test_base_catalog_admin.py +++ b/tests/test_base_catalog_admin.py @@ -102,7 +102,7 @@ def test_courses_initial_is_empty_for_catalog_with_no_courses(self): form = BaseCatalogAdminForm(instance=catalog) - assert list(form.fields['courses'].initial) == [] + assert not list(form.fields['courses'].initial) # --------------------------------------------------------------------------- From 66f8bbbdc54e84bc662322c3da55de50cda5642e Mon Sep 17 00:00:00 2001 From: carlos Date: Wed, 5 Aug 2026 10:12:22 -0500 Subject: [PATCH 07/11] fix: fix isort blank line in test_base_catalog_admin --- tests/test_base_catalog_admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_base_catalog_admin.py b/tests/test_base_catalog_admin.py index 39b6781..a997381 100644 --- a/tests/test_base_catalog_admin.py +++ b/tests/test_base_catalog_admin.py @@ -22,7 +22,6 @@ from partner_catalog.services.catalog_courses import CourseOverview from tests.factories import make_user - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- From 267cbf679fbd8635daf26b5b207217e91120a0ef Mon Sep 17 00:00:00 2001 From: carlos Date: Wed, 5 Aug 2026 10:15:37 -0500 Subject: [PATCH 08/11] fix: fix blank lines in test_base_catalog_admin --- tests/test_base_catalog_admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_base_catalog_admin.py b/tests/test_base_catalog_admin.py index a997381..39b6781 100644 --- a/tests/test_base_catalog_admin.py +++ b/tests/test_base_catalog_admin.py @@ -22,6 +22,7 @@ from partner_catalog.services.catalog_courses import CourseOverview from tests.factories import make_user + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- From 75e8a2f887532e5b5615724c9016762ca6aeb136 Mon Sep 17 00:00:00 2001 From: carlos Date: Wed, 5 Aug 2026 10:23:35 -0500 Subject: [PATCH 09/11] fix: fix blank lines and implicit booleanness in test_base_catalog_admin --- tests/test_base_catalog_admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_base_catalog_admin.py b/tests/test_base_catalog_admin.py index 39b6781..f6dd50d 100644 --- a/tests/test_base_catalog_admin.py +++ b/tests/test_base_catalog_admin.py @@ -22,11 +22,11 @@ from partner_catalog.services.catalog_courses import CourseOverview from tests.factories import make_user - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- + def make_base_catalog(slug_suffix="1"): """Create and return a BaseCatalog for testing.""" return BaseCatalog.objects.create(name=f"Test Catalog {slug_suffix}", slug=f"test-catalog-{slug_suffix}") @@ -175,4 +175,4 @@ def test_clears_all_courses_when_selection_is_empty(self): _admin().save_related(_request(), _form(catalog, []), [], change=True) - assert catalog.courses.count() == 0 + assert not catalog.courses.count() From b83ca8b4ff018d6ef968dc4d710cb7fab8ec1094 Mon Sep 17 00:00:00 2001 From: carlos Date: Mon, 10 Aug 2026 15:40:27 -0500 Subject: [PATCH 10/11] fix(admin): catch FieldDoesNotExist and add unique constraint on BaseCatalogCourse --- partner_catalog/admin.py | 3 ++- ...atalogcourse_unique_base_catalog_course.py | 21 +++++++++++++++++++ partner_catalog/models.py | 6 ++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 partner_catalog/migrations/0010_basecatalogcourse_unique_base_catalog_course.py diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index 6e835a2..4166c48 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -3,6 +3,7 @@ from django import forms from django.contrib import admin from django.contrib.admin.widgets import FilteredSelectMultiple +from django.core.exceptions import FieldDoesNotExist from django.db.models import Count from django.urls import reverse from django.utils.html import format_html @@ -46,7 +47,7 @@ def __init__(self, *args, **kwargs): try: CourseOverview._meta.get_field('display_name') qs = CourseOverview.objects.order_by('display_name') - except Exception: # pylint: disable=broad-except + except FieldDoesNotExist: qs = CourseOverview.objects.all() self.fields['courses'].queryset = qs if self.instance.pk: diff --git a/partner_catalog/migrations/0010_basecatalogcourse_unique_base_catalog_course.py b/partner_catalog/migrations/0010_basecatalogcourse_unique_base_catalog_course.py new file mode 100644 index 0000000..738fab3 --- /dev/null +++ b/partner_catalog/migrations/0010_basecatalogcourse_unique_base_catalog_course.py @@ -0,0 +1,21 @@ +"""Migration to add a uniqueness constraint on (base_catalog, course_overview) for BaseCatalogCourse.""" + +from django.db import migrations, models + + +class Migration(migrations.Migration): + """Add UniqueConstraint to prevent duplicate BaseCatalogCourse entries.""" + + dependencies = [ + ("partner_catalog", "0009_alter_catalogcourseenrollment_course_overview"), + ] + + operations = [ + migrations.AddConstraint( + model_name="basecatalogcourse", + constraint=models.UniqueConstraint( + fields=["base_catalog", "course_overview"], + name="unique_base_catalog_course", + ), + ), + ] diff --git a/partner_catalog/models.py b/partner_catalog/models.py index 381f148..bd9d226 100644 --- a/partner_catalog/models.py +++ b/partner_catalog/models.py @@ -95,6 +95,12 @@ class Meta: verbose_name = "Base Catalog Course" verbose_name_plural = "Base Catalog Courses" ordering = ["-added_at"] + constraints = [ + models.UniqueConstraint( + fields=["base_catalog", "course_overview"], + name="unique_base_catalog_course", + ) + ] def __str__(self): """Return string representation.""" From 6fe5b238fd61a084c07439e6fe8ec5041c9480b9 Mon Sep 17 00:00:00 2001 From: carlos Date: Wed, 12 Aug 2026 12:30:14 -0500 Subject: [PATCH 11/11] fix: restore response_add and get_changeform_initial_data lost during rebase --- partner_catalog/admin.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index 4166c48..b428837 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -1,10 +1,13 @@ """Admin configuration for Partner Catalog models.""" +from urllib.parse import urlencode + from django import forms from django.contrib import admin from django.contrib.admin.widgets import FilteredSelectMultiple from django.core.exceptions import FieldDoesNotExist from django.db.models import Count +from django.http import HttpResponseRedirect from django.urls import reverse from django.utils.html import format_html @@ -145,6 +148,24 @@ def save_model(self, request, obj, form, change): obj.added_by = request.user super().save_model(request, obj, form, change) + def response_add(self, request, obj, post_url_continue=None): + """After saving a new entry, keep base_catalog pre-filled when adding another.""" + response = super().response_add(request, obj, post_url_continue) + if "_addanother" in request.POST and obj.base_catalog_id: + add_url = reverse( + f"admin:{self.model._meta.app_label}_{self.model._meta.model_name}_add" + ) + query = urlencode({"base_catalog": obj.base_catalog_id}) + return HttpResponseRedirect(f"{add_url}?{query}") + return response + + def get_changeform_initial_data(self, request): + """Pre-populate base_catalog from query-string when coming from response_add.""" + initial = super().get_changeform_initial_data(request) + if "base_catalog" in request.GET and "base_catalog" not in initial: + initial["base_catalog"] = request.GET["base_catalog"] + return initial + def get_queryset(self, request): """Optimize queryset with select_related.""" qs = super().get_queryset(request)