Skip to content

feat: add course csv export endpoint for publisher reports - #69

Draft
rjv31 wants to merge 1 commit into
masterfrom
feat/course-report-csv-export
Draft

feat: add course csv export endpoint for publisher reports#69
rjv31 wants to merge 1 commit into
masterfrom
feat/course-report-csv-export

Conversation

@rjv31

@rjv31 rjv31 commented Aug 3, 2026

Copy link
Copy Markdown

No description provided.

@rjv31
rjv31 force-pushed the feat/course-report-csv-export branch from 8d2d964 to 2d98145 Compare August 4, 2026 06:44
@rjv31
rjv31 requested a lite review from Copilot August 4, 2026 07:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a new CSV export endpoint on the Courses API to support publisher reporting use cases, along with test coverage validating filtering/permissions and CSV formatting.

Changes:

  • Added a csv collection action to CourseViewSet that returns a CSV download of filtered courses.
  • Implemented CSV column mapping and publisher URL/status display transformations for exported rows.
  • Added a dedicated test suite validating headers, filtering parity, partner scoping, and auth/permission behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
course_discovery/apps/api/v1/views/courses.py Adds the csv export action and CSV row/header construction logic for course exports.
course_discovery/apps/api/v1/tests/test_views/test_courses.py Adds tests to verify CSV export behavior, filtering, formatting, and permissions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread course_discovery/apps/api/v1/views/courses.py Outdated
Comment thread course_discovery/apps/api/v1/views/courses.py
Comment thread course_discovery/apps/api/v1/views/courses.py Outdated
@rjv31
rjv31 force-pushed the feat/course-report-csv-export branch from 2d98145 to eacabe3 Compare August 12, 2026 11:26
@rjv31
rjv31 requested a lite review from Copilot August 12, 2026 11:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

course_discovery/apps/api/v1/views/courses.py:218

  • StreamingHttpResponse is constructed from a fully materialized csv_lines list, so the response isn’t actually streamed and memory usage scales with the full export size. Yielding rows from a generator keeps memory bounded while preserving the same output.
        csv_lines = [self._csv_line(self.EXPORT_CSV_HEADERS)]
        for serialized_course in csv_serialized:
            course_metadata = course_metadata_by_uuid.get(serialized_course.get('uuid'), {})
            csv_lines.append(
                self._csv_line(
                    self._csv_row(
                        serialized_course,
                        course_metadata.get('publisher_url', ''),
                        course_metadata.get('organization_key', ''),
                        course_metadata.get('project_coordinator', ''),
                    )
                )
            )

        response = StreamingHttpResponse(csv_lines, content_type='text/csv; charset=utf-8')
        response['Content-Disposition'] = 'attachment; filename="publisher_courses.csv"'
        return response

course_discovery/apps/api/v1/views/courses.py:119

  • CSV formula-injection escaping only checks the first character, so values with leading whitespace (e.g., " \t=SUM(…)" or " -1+2") won’t be escaped and can still be interpreted as formulas in spreadsheet tools. Consider checking the first non-whitespace character instead while preserving the original value.
        def escape_csv_cell(value):
            if isinstance(value, str) and value and value[0] in ('=', '+', '-', '@'):
                return "'" + value
            return value

@rjv31
rjv31 force-pushed the feat/course-report-csv-export branch 2 times, most recently from 4f8f40a to 7300114 Compare August 12, 2026 15:15
@rjv31
rjv31 requested a lite review from Copilot August 12, 2026 15:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

course_discovery/apps/api/v1/views/courses.py:195

  • filtered_courses = list(filtered_queryset) and self.get_serializer(filtered_courses, many=True).data fully materialize both the queryset (including all prefetched relations from get_queryset) and the entire serialized payload in memory before streaming. For large exports this defeats the main scalability benefit of StreamingHttpResponse and can cause high memory usage/time-to-first-byte.

Consider introducing a dedicated, lightweight export queryset/serializer (only fields needed for CSV) and streaming rows without building the full serialized list in memory (e.g., by iterating courses in chunks and writing rows per chunk).

        filtered_courses = list(filtered_queryset)

        csv_serialized = self.get_serializer(filtered_courses, many=True).data

@rjv31
rjv31 force-pushed the feat/course-report-csv-export branch from 7300114 to da2239a Compare August 13, 2026 07:46
@rjv31
rjv31 requested a lite review from Copilot August 13, 2026 07:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

course_discovery/apps/api/v1/views/courses.py:192

  • _iterate_queryset_in_chunks uses queryset slicing with an ever-increasing OFFSET. For large exports this degrades significantly (OFFSET scans get slower as the offset grows), which undermines the intent of streaming the response. A more scalable approach is to stream primary keys via .iterator() and refetch each chunk using pk__in, preserving the queryset's existing filters/orderings while avoiding large OFFSETs.
    @staticmethod
    def _iterate_queryset_in_chunks(queryset, chunk_size=500):
        """Efficiently iterate through a queryset in bounded chunks."""
        offset = 0
        while True:
            chunk = list(queryset[offset:offset + chunk_size])
            if not chunk:
                break
            yield chunk
            offset += chunk_size

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.

2 participants