fix: info added about courses which are not published (#3974)

* fix: info added about courses which are not published

* test: external course sync test fixed

* fix: text updated

Author: marslanabdulrauf

cms/api.py

@@ -296,8 +296,13 @@ def save_page_revision(page, updated_revision):
     Args:
         page(Page): A page object.
         updated_revision(Page): Updated Page object using the `latest_revision_as_object`
+
+    Returns:
+        bool: True if the revision was published, False if it was kept as a draft
+            because the page had unpublished changes.
     """
     is_draft = page.has_unpublished_changes
     revision = updated_revision.save_revision(user=None, log_action=True)
     if not is_draft:
         revision.publish()
+    return not is_draft

courses/sync_external_courses/external_course_sync_api.py

@@ -450,10 +450,13 @@ def update_external_course_runs(external_courses, keymap):  # noqa: C901, PLR091
             log.info(
                 f"Creating or Updating course page, title: {external_course.course_title}, course_code: {external_course.course_run_code}"  # noqa: G004
             )
-            course_page, course_page_created, course_page_updated = (
-                create_or_update_external_course_page(
-                    course_index_page, course, external_course, keymap
-                )
+            (
+                course_page,
+                course_page_created,
+                course_page_updated,
+                course_page_published,
+            ) = create_or_update_external_course_page(
+                course_index_page, course, external_course, keymap
             )
 
             if course_page_created:
@@ -465,7 +468,7 @@ def update_external_course_runs(external_courses, keymap):  # noqa: C901, PLR091
                 log.info(
                     f"Created external course page for course title: {external_course.course_title}"  # noqa: G004
                 )
-            elif course_page_updated:
+            elif course_page_updated and course_page_published:
                 stats_collector.add_stat(
                     "course_pages_updated",
                     external_course.course_code,
@@ -474,6 +477,15 @@ def update_external_course_runs(external_courses, keymap):  # noqa: C901, PLR091
                 log.info(
                     f"Updated external course page for course title: {external_course.course_title}"  # noqa: G004
                 )
+            elif course_page_updated:
+                stats_collector.add_stat(
+                    "course_pages_kept_as_draft",
+                    external_course.course_code,
+                    external_course.course_title,
+                )
+                log.info(
+                    "Updated external course page kept as draft (page has unpublished changes)"
+                )
 
             if external_course.category:
                 topic = CourseTopic.objects.filter(
@@ -532,6 +544,9 @@ def update_external_course_runs(external_courses, keymap):  # noqa: C901, PLR091
     # so, we are removing the courses created from the updated courses list.
     stats_collector.remove_duplicates("existing_courses", "courses_created")
     stats_collector.remove_duplicates("course_pages_updated", "course_pages_created")
+    stats_collector.remove_duplicates(
+        "course_pages_kept_as_draft", "course_pages_created"
+    )
 
     return stats_collector
 
@@ -606,7 +621,11 @@ def create_or_update_external_course_page(  # noqa: C901
         external_course(ExternalCourse): A ExternalCourse object.
 
     Returns:
-        tuple(ExternalCoursePage, is_created, is_updated): ExternalCoursePage object, is_created, is_updated
+        tuple(ExternalCoursePage, is_created, is_updated, is_published): ExternalCoursePage
+            object, is_created, is_updated, and is_published. `is_published` indicates whether
+            an update was published (True) or kept as a draft (False) because the page had
+            unpublished changes. It is only meaningful when `is_updated` is True; for created
+            pages and unchanged pages it defaults to True.
     """
     course_page = (
         ExternalCoursePage.objects.select_for_update().filter(course=course).first()
@@ -630,6 +649,7 @@ def create_or_update_external_course_page(  # noqa: C901
             )
 
     is_created = is_updated = False
+    is_published = True
     if not course_page:
         course_page = ExternalCoursePage(
             course=course,
@@ -686,9 +706,9 @@ def create_or_update_external_course_page(  # noqa: C901
             is_updated = True
 
         if is_updated:
-            save_page_revision(course_page, latest_revision)
+            is_published = save_page_revision(course_page, latest_revision)
 
-    return course_page, is_created, is_updated
+    return course_page, is_created, is_updated, is_published
 
 
 def create_or_update_external_course_run(course, external_course):

courses/sync_external_courses/external_course_sync_api_test.py

@@ -249,7 +249,7 @@ def test_create_or_update_external_course_page(  # noqa: PLR0913, C901
     if not has_language:
         external_course_data.pop("language")
 
-    external_course_page, course_page_created, course_page_updated = (
+    external_course_page, course_page_created, course_page_updated, _ = (
         create_or_update_external_course_page(
             course_index_page,
             course,
@@ -908,9 +908,13 @@ def test_save_page_revision(is_draft_page, has_unpublished_changes):
 
     latest_revision = external_course_page.get_latest_revision_as_object()
     latest_revision.external_marketing_url = "https://test-external-course-sync-api.io/Internet-of-things-iot-design-and-applications"
-    save_page_revision(external_course_page, latest_revision)
+    # The revision is published only when the page had no unpublished changes
+    # before saving; otherwise it is kept as a draft.
+    expected_draft = external_course_page.has_unpublished_changes
+    published = save_page_revision(external_course_page, latest_revision)
 
     assert external_course_page.live == (not is_draft_page)
+    assert published is (not expected_draft)
 
     if has_unpublished_changes:
         assert external_course_page.has_unpublished_changes
@@ -1214,13 +1218,16 @@ def test_create_or_update_external_course_page_field_override(
 
     keymap = get_keymap(external_course_data["course_run_code"])
 
-    external_course_page, course_page_created, course_page_updated = (
-        create_or_update_external_course_page(
-            course_index_page,
-            course,
-            ExternalCourse(external_course_data, keymap=keymap),
-            keymap=keymap,
-        )
+    (
+        external_course_page,
+        course_page_created,
+        course_page_updated,
+        course_page_published,
+    ) = create_or_update_external_course_page(
+        course_index_page,
+        course,
+        ExternalCourse(external_course_data, keymap=keymap),
+        keymap=keymap,
     )
 
     latest_revision = external_course_page.get_latest_revision_as_object()
@@ -1231,6 +1238,8 @@ def test_create_or_update_external_course_page_field_override(
     )
     assert course_page_updated is True
     assert course_page_created is False
+    # The page has no unpublished changes here, so the update is published.
+    assert course_page_published is True
 
     # Check if fields were overridden based on whether they had existing values
     if expected_fields_overridden:
@@ -1255,3 +1264,55 @@ def test_create_or_update_external_course_page_field_override(
         assert latest_revision.description == existing_description
         assert latest_revision.background_image == existing_image
         assert latest_revision.thumbnail_image == existing_image
+
+
+@pytest.mark.parametrize(
+    "external_course_data",
+    [{"platform": EMERITUS_PLATFORM_NAME}],
+    indirect=True,
+)
+@pytest.mark.django_db
+def test_create_or_update_external_course_page_kept_as_draft(external_course_data):
+    """
+    Test that when a course page already has unpublished (manual) changes, an API
+    update is saved as a draft and NOT published, and `is_published` is returned as False.
+    """
+    home_page = HomePageFactory.create(title="Home Page", subhead="<p>subhead</p>")
+    course_index_page = CourseIndexPageFactory.create(parent=home_page, title="Courses")
+    course = CourseFactory.create(is_external=True, page=None)
+    ImageFactory.create(title=external_course_data["image_name"])
+
+    external_course_page = ExternalCoursePageFactory.create(
+        parent=course_index_page,
+        course=course,
+        title=external_course_data["program_name"],
+        external_marketing_url="https://old-url.com",
+    )
+    # Publish a baseline revision, then leave an unpublished (draft) manual change on top.
+    external_course_page.save_revision().publish()
+    external_course_page.refresh_from_db()
+    external_course_page.title = "Manually edited title"
+    external_course_page.save_revision()  # draft, not published
+    external_course_page.refresh_from_db()
+    assert external_course_page.has_unpublished_changes
+
+    keymap = get_keymap(external_course_data["course_run_code"])
+
+    _, course_page_created, course_page_updated, course_page_published = (
+        create_or_update_external_course_page(
+            course_index_page,
+            course,
+            ExternalCourse(external_course_data, keymap=keymap),
+            keymap=keymap,
+        )
+    )
+
+    assert course_page_created is False
+    assert course_page_updated is True
+    # The page had unpublished changes, so the API update is kept as a draft.
+    assert course_page_published is False
+
+    external_course_page.refresh_from_db()
+    # The live (published) version still has the old marketing URL.
+    assert external_course_page.external_marketing_url == "https://old-url.com"
+    assert external_course_page.has_unpublished_changes

courses/sync_external_courses/utils.py

@@ -131,6 +131,12 @@ def __init__(self):
             "course_pages_updated": StatItemsCollection(
                 "course_pages_updated",
                 "External Course Codes",
+                display_name="Course Pages Updated and Published",
+            ),
+            "course_pages_kept_as_draft": StatItemsCollection(
+                "course_pages_kept_as_draft",
+                "External Course Codes",
+                display_name="Course Pages Updated but set as Draft",
             ),
             "products_created": StatItemsCollection(
                 "products_created",

mail/templates/external_data_sync/body.html

@@ -175,7 +175,7 @@
             </li>
             <li>
               <span style="color: #008000"
-                >Course Pages Updated ({{ stats.course_pages_updated|length }}):</span
+                >Course Pages Updated and Published ({{ stats.course_pages_updated|length }}):</span
               >
               {% if stats.course_pages_updated %}
               <ul>
@@ -186,7 +186,23 @@
                 </li>
                 {% endfor %}
               </ul>
-              {% else %}No course pages updated during this sync.{% endif %}
+              {% else %}No course pages updated and published during this sync.{% endif %}
+            </li>
+            <li>
+              <span style="color: #ff0000"
+                >Course Pages Updated but set as Draft ({{ stats.course_pages_kept_as_draft|length }}):</span
+              >
+              {% if stats.course_pages_kept_as_draft %}
+              <ul>
+                {% for page in stats.course_pages_kept_as_draft %}
+                <li>
+                  {{ page.code }}
+                  ({{ page.title }}) - API changes were saved as a draft and NOT
+                  published because the page has unpublished manual changes.
+                </li>
+                {% endfor %}
+              </ul>
+              {% else %}No course pages kept as draft during this sync.{% endif %}
             </li>
           </ul>