Skip to content

feat: bulk add courses to base catalog - #67

Merged
ManuelStarDo merged 11 commits into
mainfrom
crls/feat/bulk-add-courses-to-base-catalog
Aug 12, 2026
Merged

feat: bulk add courses to base catalog#67
ManuelStarDo merged 11 commits into
mainfrom
crls/feat/bulk-add-courses-to-base-catalog

Conversation

@ccantillo

@ccantillo ccantillo commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Manage courses inline on BaseCatalog change form

Summary

Administrators could only add one course at a time to the Base Catalog, which
was impractical when dealing with dozens or hundreds of courses. This PR embeds
a dual-panel course manager directly on the BaseCatalog change form, mirroring
the behaviour of Django's built-in auth/group admin. Courses can be added and
removed in a single save operation, with the current state pre-loaded on every
page open.


What changed

partner_catalog/admin.py

  • Added BaseCatalogAdminForm — a ModelForm with a custom courses field
    using Django's FilteredSelectMultiple widget. On load, __init__ sets
    courses.initial to the catalog's currently assigned courses so the right
    panel is pre-populated.
  • Added get_fields() to BaseCatalogAdmin — returns the field list at
    runtime (including courses) to bypass Django's static admin.E013 check,
    which forbids listing M2M-with-through-model fields in the class-level
    fields attribute.
  • Added save_related() to BaseCatalogAdmin — diffs the submitted selection
    against current BaseCatalogCourse entries and:
    • Creates BaseCatalogCourse rows for newly selected courses
      (preserving added_by / added_at metadata).
    • Deletes rows for deselected courses.
    • No-ops for courses whose state is unchanged.
  • Updated list view manage_courses column to link to the catalog change page.

tests/test_base_catalog_admin.py (new)

Nine pytest tests covering:

  • BaseCatalogAdminForm.__init__ — queryset population, empty initial for a
    new catalog, pre-populated initial for an existing catalog with courses.
  • BaseCatalogAdmin.save_related — add, remove, no-op, added_by tracking,
    and clear-all scenarios.

How the widget works

The FilteredSelectMultiple widget renders as a dual-panel picker directly on
the BaseCatalog change form. No separate page or URL is needed.

Action How
Select a single course Click it in the left panel
Select multiple courses Ctrl + click (or Cmd + click on Mac) each course
Move selected to right Click the Choose selected Courses arrow button
Move all visible to right Click Choose all Courses
Deselect / remove Select in right panel → Remove selected Courses
Remove all chosen Click Remove all Courses
Filter courses by name Type in the Filter box above either panel

Note: Choose all Courses acts on the currently visible (filtered)
list. Type a search term first, then click it to bulk-select only the
matching results.

On Save, all additions and removals are applied atomically.


Files changed

Repo File Change
openedx-corporate partner_catalog/admin.py BaseCatalogAdminForm, get_fields, save_related, updated list column
openedx-corporate tests/test_base_catalog_admin.py New — 9 tests for form init and save_related sync logic

Testing

  1. Go to /admin/partner_catalog/basecatalog/.
  2. Open an existing Base Catalog — its current courses appear pre-loaded in
    the right panel.
  3. Move courses between panels using the filter, Ctrl+click, and arrows.
  4. Click Save — added and removed courses are reflected immediately in the
    Total Courses count and the widget on the next load.
  5. Re-open the catalog and confirm the right panel matches the last saved
    selection exactly.

@ccantillo
ccantillo requested a review from rguerra-fccn July 9, 2026 18:03
@ccantillo
ccantillo marked this pull request as ready for review July 9, 2026 18:36
@igobranco
igobranco requested review from a team and igobranco July 10, 2026 10:15
@igobranco

igobranco commented Jul 10, 2026

Copy link
Copy Markdown
Member

@ccantillo in future please add screenshots!

I have tested locally and it seems to be working fine.

Nevertheless, this is not good enough! The functionality requested needs to be improved. We can't just "Add Bulk Courses" but also we need to improve it so it is a "Manage Bulk Courses".
The both select multiples need to work has User Groups on Django. Like the use case: /admin/auth/group/1/change/

Screenshots of the corporate module:

image image image

Screenshot of Manage User Group:

image

On refresh it still loads the existing groups.

@igobranco igobranco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transform it to be like Manage Bulk Courses.
Currently I didn't find a way to remove courses.

@ccantillo

Copy link
Copy Markdown
Contributor Author

Transform it to be like Manage Bulk Courses. Currently I didn't find a way to remove courses.

Hi @igobranco, thanks for the feedback!

Reworked the implementation to match the auth/group pattern. The "Bulk Add Courses" separate page is gone — the FilteredSelectMultiple widget now lives directly on the BaseCatalog change form, pre-populated with the catalog's current courses on load.

A single Save now handles both additions and removals: it diffs the selected set against the current BaseCatalogCourse entries, creates the new ones (preserving added_by/added_at metadata), and deletes the removed ones.

image image

@igobranco
igobranco self-requested a review July 13, 2026 09:00

@igobranco igobranco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✔️

Image Image

@ManuelStarDo
ManuelStarDo self-requested a review August 3, 2026 17:02

@ManuelStarDo ManuelStarDo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

  • No test coverage was added for save_related's diff/sync logic (add + remove in one save) or for the BaseCatalogAdminForm.__init__ pre-population behavior. Existing test suite (tests/test_catalog_course_service.py) only covers the unrelated CatalogCourseService/PartnerCatalog add/remove flow, not this new BaseCatalog inline widget path. Given this directly manipulates DB state (create/delete) from raw cleaned_data, admin-level tests exercising add, remove, and no-op saves are warranted.

  • The PR description/title still describe the old "Bulk Add Courses" separate-page approach (custom template, get_urls(), bulk_add_courses_view()) — none of which exists in the final diff. This is a documentation mismatch that should be fixed before merging so reviewers and future readers aren't misled.

@ccantillo
Please address these 2 issues before merging

@ccantillo

Copy link
Copy Markdown
Contributor Author

Review

  • No test coverage was added for save_related's diff/sync logic (add + remove in one save) or for the BaseCatalogAdminForm.__init__ pre-population behavior. Existing test suite (tests/test_catalog_course_service.py) only covers the unrelated CatalogCourseService/PartnerCatalog add/remove flow, not this new BaseCatalog inline widget path. Given this directly manipulates DB state (create/delete) from raw cleaned_data, admin-level tests exercising add, remove, and no-op saves are warranted.
  • The PR description/title still describe the old "Bulk Add Courses" separate-page approach (custom template, get_urls(), bulk_add_courses_view()) — none of which exists in the final diff. This is a documentation mismatch that should be fixed before merging so reviewers and future readers aren't misled.

@ccantillo Please address these 2 issues before merging

Hi @ManuelStarDo. Done. tests are in and the PR description is updated to match what was actually built. Let me know if anything else needs adjusting.

@ManuelStarDo
ManuelStarDo self-requested a review August 5, 2026 15:40

@ManuelStarDo ManuelStarDo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccantillo
I tested this locally and the feature is working.

Please just address these 2 changes and we can merge the PR.

Comment thread partner_catalog/admin.py
Comment thread partner_catalog/admin.py
@ccantillo

Copy link
Copy Markdown
Contributor Author

@ccantillo I tested this locally and the feature is working.

Please just address these 2 changes and we can merge the PR.

Hi @ManuelStarDo. Both changes are done:

Replaced except Exception with except FieldDoesNotExist — now catches only what's intended.
Added a UniqueConstraint on (base_catalog, course_overview) with its migration — duplicate rows are now prevented at the DB level.

@ManuelStarDo
ManuelStarDo force-pushed the crls/feat/bulk-add-courses-to-base-catalog branch from b1775a9 to b83ca8b Compare August 12, 2026 10:25
@ManuelStarDo

Copy link
Copy Markdown
Contributor

@ccantillo

I did a rebase to fix the CI problems regarding the Integration Tests in Tutor / Tests (Tutor ==19.0.0) (pull_request) versioning mismatch

I don't know if I messed up the rebase, but the PR is now failing at a specific Python CI test.
Please compare the final file you have locally of admin.py against the PRs final version to see if I missed something.

If not, then please fix the code/test so it passes CI

Apart from that, i tested everything locally and it was working. With the CI/rebase issues addressed, we are good for merging

@ccantillo

Copy link
Copy Markdown
Contributor Author

Hi @ManuelStarDo Fixed and pushed. should be good now

@ManuelStarDo
ManuelStarDo merged commit 8bda788 into main Aug 12, 2026
4 checks passed
@ManuelStarDo
ManuelStarDo deleted the crls/feat/bulk-add-courses-to-base-catalog branch August 12, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants