From ca650f94eaa7da04fe3d4057dfcaa2808d0abb63 Mon Sep 17 00:00:00 2001 From: carlos Date: Thu, 2 Jul 2026 12:30:18 -0500 Subject: [PATCH 1/3] fix(admin): keep base_catalog pre-filled after save and add another --- CHANGELOG.rst | 2 +- partner_catalog/admin.py | 21 ++++ tests/admin_test_urls.py | 8 ++ tests/test_admin_base_catalog_course.py | 128 ++++++++++++++++++++++++ 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 tests/admin_test_urls.py create mode 100644 tests/test_admin_base_catalog_course.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 22c44f3..2318745 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,4 +14,4 @@ Change Log Unreleased ********** -* +* fix(admin): keep base_catalog pre-filled after "Save and add another" in BaseCatalogCourseAdmin diff --git a/partner_catalog/admin.py b/partner_catalog/admin.py index bf8e636..9cb2e76 100644 --- a/partner_catalog/admin.py +++ b/partner_catalog/admin.py @@ -1,7 +1,10 @@ """Admin configuration for Partner Catalog models.""" +from urllib.parse import urlencode + from django.contrib import admin from django.db.models import Count +from django.http import HttpResponseRedirect from django.urls import reverse from django.utils.html import format_html @@ -119,6 +122,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) diff --git a/tests/admin_test_urls.py b/tests/admin_test_urls.py new file mode 100644 index 0000000..1c09e58 --- /dev/null +++ b/tests/admin_test_urls.py @@ -0,0 +1,8 @@ +"""Minimal URL conf for admin-related tests that need reverse('admin:...').""" + +from django.contrib import admin +from django.urls import include, path + +urlpatterns = [ + path("admin/", admin.site.urls), +] diff --git a/tests/test_admin_base_catalog_course.py b/tests/test_admin_base_catalog_course.py new file mode 100644 index 0000000..7417481 --- /dev/null +++ b/tests/test_admin_base_catalog_course.py @@ -0,0 +1,128 @@ +""" +Tests for BaseCatalogCourseAdmin sticky base_catalog behaviour. + +When the user clicks "Save and add another" after creating a BaseCatalogCourse, +the redirect URL should carry the same base_catalog ID so that the next add-form +comes pre-populated. +""" + +import pytest +from django.contrib.admin.sites import AdminSite +from django.contrib.messages.storage.fallback import FallbackStorage +from django.test import RequestFactory, override_settings + +from partner_catalog.admin import BaseCatalogCourseAdmin +from partner_catalog.edxapp_wrapper.course_module import course_overview +from partner_catalog.models import BaseCatalog, BaseCatalogCourse + +from tests.factories import make_user + +_ADMIN_URL_CONF = "tests.admin_test_urls" + +CourseOverview = course_overview() + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_course(): + return CourseOverview.objects.create() + + +def _make_base_catalog(name="Test Base Catalog"): + return BaseCatalog.objects.create(name=name, slug=name.lower().replace(" ", "-")) + + +def _post_request(user, data=None): + factory = RequestFactory() + request = factory.post("/fake-admin/", data=data or {}) + request.user = user + request.session = {} + request._messages = FallbackStorage(request) + return request + + +def _get_request(user, params=None): + factory = RequestFactory() + query = f"?{params}" if params else "" + request = factory.get(f"/fake-admin/{query}") + request.user = user + request.session = {} + request._messages = FallbackStorage(request) + return request + + +def _admin_view(): + return BaseCatalogCourseAdmin(BaseCatalogCourse, AdminSite()) + + +# --------------------------------------------------------------------------- +# response_add — redirect preserves base_catalog when "_addanother" posted +# --------------------------------------------------------------------------- + +@pytest.mark.django_db +@override_settings(ROOT_URLCONF=_ADMIN_URL_CONF) +def test_response_add_addanother_redirects_with_base_catalog(): + """Clicking 'Save and add another' redirects to the add URL with base_catalog param.""" + admin_user = make_user(is_staff=True, is_superuser=True) + catalog = _make_base_catalog() + course = _make_course() + entry = BaseCatalogCourse.objects.create( + base_catalog=catalog, + course_overview=course, + added_by=admin_user, + ) + + request = _post_request(admin_user, data={"_addanother": "1"}) + response = _admin_view().response_add(request, entry) + + assert response.status_code == 302 + assert f"base_catalog={catalog.pk}" in response["Location"] + + +@pytest.mark.django_db +@override_settings(ROOT_URLCONF=_ADMIN_URL_CONF) +def test_response_add_save_does_not_add_base_catalog_param(): + """A plain 'Save' (no _addanother) does not append base_catalog to the redirect.""" + admin_user = make_user(is_staff=True, is_superuser=True) + catalog = _make_base_catalog() + course = _make_course() + entry = BaseCatalogCourse.objects.create( + base_catalog=catalog, + course_overview=course, + added_by=admin_user, + ) + + request = _post_request(admin_user, data={}) + response = _admin_view().response_add(request, entry) + + assert response.status_code == 302 + assert "base_catalog" not in response["Location"] + + +# --------------------------------------------------------------------------- +# get_changeform_initial_data — pre-populates from GET param +# --------------------------------------------------------------------------- + +@pytest.mark.django_db +def test_get_changeform_initial_data_sets_base_catalog_from_get(): + """GET ?base_catalog= pre-populates the initial form data.""" + admin_user = make_user(is_staff=True, is_superuser=True) + catalog = _make_base_catalog() + + request = _get_request(admin_user, params=f"base_catalog={catalog.pk}") + initial = _admin_view().get_changeform_initial_data(request) + + assert str(initial.get("base_catalog")) == str(catalog.pk) + + +@pytest.mark.django_db +def test_get_changeform_initial_data_without_param_is_empty(): + """Without a GET param, base_catalog is absent from initial data.""" + admin_user = make_user(is_staff=True, is_superuser=True) + + request = _get_request(admin_user) + initial = _admin_view().get_changeform_initial_data(request) + + assert "base_catalog" not in initial From 2ed2cea55e1a9187da996d7a6f173bfe4fbd4cad Mon Sep 17 00:00:00 2001 From: carlos Date: Thu, 2 Jul 2026 16:45:15 -0500 Subject: [PATCH 2/3] fix(quality): remove unused import and add missing docstrings --- tests/admin_test_urls.py | 2 +- tests/test_admin_base_catalog_course.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/admin_test_urls.py b/tests/admin_test_urls.py index 1c09e58..be6e3d3 100644 --- a/tests/admin_test_urls.py +++ b/tests/admin_test_urls.py @@ -1,7 +1,7 @@ """Minimal URL conf for admin-related tests that need reverse('admin:...').""" from django.contrib import admin -from django.urls import include, path +from django.urls import path urlpatterns = [ path("admin/", admin.site.urls), diff --git a/tests/test_admin_base_catalog_course.py b/tests/test_admin_base_catalog_course.py index 7417481..a81b576 100644 --- a/tests/test_admin_base_catalog_course.py +++ b/tests/test_admin_base_catalog_course.py @@ -27,29 +27,33 @@ # --------------------------------------------------------------------------- def _make_course(): + """Create and return a CourseOverview instance.""" return CourseOverview.objects.create() def _make_base_catalog(name="Test Base Catalog"): + """Create and return a BaseCatalog with the given name.""" return BaseCatalog.objects.create(name=name, slug=name.lower().replace(" ", "-")) def _post_request(user, data=None): + """Build a POST request with messages middleware attached.""" factory = RequestFactory() request = factory.post("/fake-admin/", data=data or {}) request.user = user request.session = {} - request._messages = FallbackStorage(request) + request._messages = FallbackStorage(request) # pylint: disable=protected-access return request def _get_request(user, params=None): + """Build a GET request with messages middleware attached.""" factory = RequestFactory() query = f"?{params}" if params else "" request = factory.get(f"/fake-admin/{query}") request.user = user request.session = {} - request._messages = FallbackStorage(request) + request._messages = FallbackStorage(request) # pylint: disable=protected-access return request From 89d8f4ee549d4edf89648e2d790468be2bbbc536 Mon Sep 17 00:00:00 2001 From: carlos Date: Thu, 2 Jul 2026 16:47:54 -0500 Subject: [PATCH 3/3] fix(quality): fix isort import order --- tests/test_admin_base_catalog_course.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_admin_base_catalog_course.py b/tests/test_admin_base_catalog_course.py index a81b576..98754be 100644 --- a/tests/test_admin_base_catalog_course.py +++ b/tests/test_admin_base_catalog_course.py @@ -14,7 +14,6 @@ from partner_catalog.admin import BaseCatalogCourseAdmin from partner_catalog.edxapp_wrapper.course_module import course_overview from partner_catalog.models import BaseCatalog, BaseCatalogCourse - from tests.factories import make_user _ADMIN_URL_CONF = "tests.admin_test_urls"