feat: [FC-7879] add new fields/indices to Content-/UserDate to support assignments #343

Serj-N · 2025-10-13T16:45:36Z

Note

This is a newly reopened version of an earlier PR, which should most likely be closed due to contribution rights.

Description

This PR Extends the ContentDate and UserDate models in edx-when to support enhanced assignment metadata and future scheduling use cases.

ContentDate model changes:

Added assignment_title, course_name, and subsection_name fields to improve tracking and display of assignment metadata.
Added indices to support efficient filtering and lookup by assignment and subsection per course.

UserDate model changes:

Added an indexed field first_component_block_id for referencing the starting block of a user’s assignment attempt.
Added is_content_gated to establish whether the user has access to the corresponding block. For convenience, exposed as learner_has_access property.

openedx-webhooks · 2025-10-13T16:45:41Z

Thanks for the pull request, @Serj-N!

This repository is currently maintained by @openedx/openedx-unmaintained.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

cmltaWt0 · 2025-10-16T16:46:43Z

edx_when/models.py

    field = models.CharField(max_length=255, default='')
    active = models.BooleanField(default=True)
    block_type = models.CharField(max_length=255, null=True)
+    assignment_title = models.CharField(max_length=255, blank=True, default='', db_index=True)


Could you add an explanation or performance implication of adding index (not the general explanation but some real example of the field usage)? Just to be aware of the use case.
It will be useful to see the filtering to assess the necessity to trade an additional effort to do the indexing.

Currently, the fields assignment_title and first_component_block_id are not used for filtering, they are only serialized in AllCourseDatesSerializer.

I'm also curious about the indices that have been added on these new fields in both models. What's the value add? Is there a particular performance improvement that we need on reads for which the indexes will be a substantial help (where they have not been considered necessary on the other columns in the table)? The fields are only used for serialization and not filtering, so why do they need to be indexed?

Because all the index attributes do cause a performance cost on write, I am definitely curious about the advantage gain they are bringing here. I'm comfortable putting indices on the columns if they are necessary, I just want to know that the reason to been considered and judged valuable.

Agree with you. Removed unused indexes 746c3dd

cmltaWt0 · 2025-10-16T16:46:55Z

edx_when/models.py

    block_type = models.CharField(max_length=255, null=True)
+    assignment_title = models.CharField(max_length=255, blank=True, default='', db_index=True)
+    course_name = models.CharField(max_length=255, blank=True, default='')
+    subsection_name = models.CharField(max_length=255, blank=True, default='', db_index=True)


The same as above.

Currently, there are no use cases for this particular field, perhaps it was added with some future functionality in mind.

adding a new database column – a new indexed database column, no less – for no planned future use seems like a bad idea to me. If there is particular future functionality that is on a roadmap, that's different, but if it's just speculative, does it need to be added? I assume if nothing is populating the column, then the indexing is cost free (I'm not sure, actually, does the DB bother to rebuild the index when you add more '' strings to an empty column?), but even so.

Removed indexes from these fields 746c3dd

cmltaWt0 · 2025-10-16T16:49:19Z

edx_when/models.py

+    is_content_gated = models.BooleanField(default=False)
+
+    class Meta:
+        """Django Metadata."""


Please update the docstring according the project's code style standard.

I have not found a single case of class Meta with a proper dosctring in the project, and in fact, I don't think it is needed here. Removed.

Ok, pylint complained, so I added back more elaborate versions of the docstrings.

cmltaWt0 · 2025-10-16T16:50:23Z

edx_when/models.py

    actor = models.ForeignKey(
        get_user_model(), null=True, default=None, blank=True, related_name="actor", on_delete=models.CASCADE
    )
+    first_component_block_id = UsageKeyField(null=True, blank=True, max_length=255, db_index=True)


The same as above about performance - could you please add some comments in the PR about the frequency of filtering by this field.

Currently, the field is only used for serialization, not filtering.

if it's not used for filtering then this field shouldn't be indexed either, should it? I'm not sure because the comment in the code specifically says the index is there to speed up lookups.

Removed this index - https://github.com/openedx/edx-when/compare/746c3dd009f4f08766df9caab5b2893c61e12e7d..288008661a5002dde80157849b43c40ea7e4a9c7

cmltaWt0 · 2025-10-16T16:53:28Z

General comment - code coverage should be improved.

Serj-N · 2025-11-18T16:39:24Z

@e0d @cmltaWt0 Please note that the coverage has been addressed in this PR.

qasimgulzar · 2025-11-24T12:03:02Z

edx_when/models.py

+            models.Index(fields=('assignment_title', 'course_id'), name='edx_when_assignment_course_idx'),
+            models.Index(fields=('subsection_name', 'course_id'), name='edx_when_subsection_course_idx'),


🔍 Composite Indexing: The Left-Prefix Principle

This is a crucial point for database performance! To ensure we get the most benefit from our multi-column indexes, we must respect the left-prefix principle (or left-most column rule).

An index on (A, B, C) can be used efficiently for filtering on:

A

(A, B)

(A, B, C)

What it Cannot Do

The index will not be used efficiently (or at all, depending on the database and query) if the query skips the leading column(s):

Filtering on B or C alone.

Filtering on (B, C).

Filtering on (A, C). (It can use the A part, but the query optimizer will still have to scan the entire range of B values for that specific A.)

Actionable Takeaway

When defining a composite index, the columns should be ordered based on the query patterns: place the column(s) most frequently used in the WHERE clause first.

Suggested change

models.Index(fields=('assignment_title', 'course_id'), name='edx_when_assignment_course_idx'),

models.Index(fields=('subsection_name', 'course_id'), name='edx_when_subsection_course_idx'),

models.Index(fields=('course_id', 'assignment_title',), name='edx_when_assignment_course_idx'),

models.Index(fields=( 'course_id', 'subsection_name'), name='edx_when_subsection_course_idx'),

Thanks. These fields won't be used for filtering, I removed them (746c3dd). If there will be need to use them - index can be added later

e0d · 2025-12-17T14:37:52Z

@qasimgulzar @deborahgu Are you happy with the latest round of change?

@cmltaWt0 you should also resolve your requested changes if you are satisfied.

cmltaWt0 · 2025-12-17T14:46:53Z

@qasimgulzar @deborahgu Are you happy with the latest round of change?

@cmltaWt0 you should also resolve your requested changes if you are satisfied.

Sure! Thanks for ping.

deborahgu

looks good to me.

qasimgulzar

Looks good to me

e0d · 2026-01-06T16:07:15Z

@deborahgu would you mind merging, the author doesn't have access to do so.

deborahgu · 2026-01-06T16:39:30Z

I also don't have access to merge, @e0d.

UsamaSadiq

A version update will be needed to release this change on PyPI. Will that be done in a follow up PR?

e0d · 2026-01-06T18:07:51Z

@UsamaSadiq that's a good call. I think we should do it as part of this PR. @Serj-N can you update the version? We'll need to update requirements in edx-platform and elsewhere as appropriate.

Serj-N · 2026-01-12T08:55:22Z

@e0d @UsamaSadiq Sure, but could you please specify what version it should be? I see the latest version is 3.1.0. Should the new version be 3.2.0?

UsamaSadiq · 2026-01-12T09:41:24Z

@e0d @UsamaSadiq Sure, but could you please specify what version it should be? I see the latest version is 3.1.0. Should the new version be 3.2.0?

@Serj-N Yes, this introduced migration will also be applied in edx-platform so it'll be best to bump a minor version to highlight the change it is introducing. You can bump the package version to 3.2.0 in both CHANGELOG.rst and edx_when/__init__.py.

…models-to-support-assignments

Serj-N · 2026-01-12T15:24:49Z

@e0d @UsamaSadiq Done, version bumped to 3.2.0.

feat: add new fields and indices to ContentDate, UserDate

320b939

Serj-N self-assigned this Oct 13, 2025

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 13, 2025

openedx-webhooks added this to Contributions Oct 13, 2025

github-project-automation bot moved this to Needs Triage in Contributions Oct 13, 2025

fix: reorder imports, ignore pylint for user.username

9ccdf89

Serj-N requested review from cmltaWt0 and e0d October 13, 2025 17:00

Serj-N mentioned this pull request Oct 13, 2025

feat: add new field for Content-/UserDate models to support assignments #324

Closed

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 15, 2025

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Oct 15, 2025

cmltaWt0 suggested changes Oct 16, 2025

View reviewed changes

Serj-N added 3 commits October 22, 2025 18:18

style: remove Django Metadata docstring

85cad9d

style: readd expanded docstrings for class Meta

390ab5c

test: add test for learner_has_access

ea309fb

Serj-N force-pushed the feat/extend-date-models-to-support-assignments branch 3 times, most recently from fb73c11 to c2c5f16 Compare November 4, 2025 09:58

test: use pragma: no cover for str/repr, fix imports

a107530

Serj-N force-pushed the feat/extend-date-models-to-support-assignments branch from c2c5f16 to a107530 Compare November 4, 2025 10:03

deborahgu self-requested a review November 19, 2025 16:24

qasimgulzar suggested changes Nov 24, 2025

View reviewed changes

feat: remove unused indexes

2880086

kyrylo-kh force-pushed the feat/extend-date-models-to-support-assignments branch from 746c3dd to 2880086 Compare December 11, 2025 09:03

mphilbrick211 moved this from Ready for Review to In Eng Review in Contributions Dec 12, 2025

cmltaWt0 approved these changes Dec 17, 2025

View reviewed changes

deborahgu approved these changes Dec 17, 2025

View reviewed changes

qasimgulzar approved these changes Dec 25, 2025

View reviewed changes

UsamaSadiq approved these changes Jan 6, 2026

View reviewed changes

Serj-N added 2 commits January 12, 2026 16:04

Merge remote-tracking branch 'upstream/master' into feat/extend-date-…

f96e766

…models-to-support-assignments

chore: bump version to 3.2.0

fbbcbc7

Serj-N mentioned this pull request Jan 12, 2026

chore: [FC-7879] bump edx-when version to 3.2.0 openedx/edx-platform#37867

Closed

		models.Index(fields=('assignment_title', 'course_id'), name='edx_when_assignment_course_idx'),
		models.Index(fields=('subsection_name', 'course_id'), name='edx_when_subsection_course_idx'),

feat: [FC-7879] add new fields/indices to Content-/UserDate to support assignments #343

Are you sure you want to change the base?

feat: [FC-7879] add new fields/indices to Content-/UserDate to support assignments #343

Conversation

Serj-N commented Oct 13, 2025

Description

Uh oh!

openedx-webhooks commented Oct 13, 2025

Uh oh!

cmltaWt0 Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Serj-N Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmltaWt0 commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Serj-N commented Nov 18, 2025

Uh oh!

Choose a reason for hiding this comment

🔍 Composite Indexing: The Left-Prefix Principle

What it Cannot Do

Actionable Takeaway

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e0d commented Dec 17, 2025

Uh oh!

cmltaWt0 commented Dec 17, 2025

Uh oh!

deborahgu left a comment

Choose a reason for hiding this comment

Uh oh!

qasimgulzar left a comment

Choose a reason for hiding this comment

Uh oh!

e0d commented Jan 6, 2026

Uh oh!

deborahgu commented Jan 6, 2026

Uh oh!

UsamaSadiq left a comment

Choose a reason for hiding this comment

Uh oh!

e0d commented Jan 6, 2026

Uh oh!

Serj-N commented Jan 12, 2026

Uh oh!

UsamaSadiq commented Jan 12, 2026

Uh oh!

Serj-N commented Jan 12, 2026

Uh oh!

cmltaWt0 Oct 16, 2025 •

edited

Loading

Serj-N Oct 22, 2025 •

edited

Loading

cmltaWt0 commented Oct 16, 2025 •

edited

Loading