feat!: Make Timestamp, Author params consistent

[kdmccormick]

Description

This PR aims to unify handling of Timestamp and Author parameters across all API functions. It ensures that the default behavior is "figure out a reasonable Timestamp and Author" rather than "assume that they are None". Timestamps are forbidden for being None, and Authors can only be None if explicitly specified as such using the special AUTHOR_NONE sentinel.

BREAKING CHANGE: Various Timestamp params (named <verb>ed, <verb>ed_at) and Author params (named <verb>ed_by) have stricter rules. For example, a concrete User or explicit AUTHOR_NONE must always be specified outside of a bulk_draft_changes_forcontext. See resolve_timestamp and resolve_author functions for details. I will make this breaking change message more specific before merging.

Background

See:

• Improve usability of created/created_by/set_at/set_by in the publishing API ? #549

Concerns

I haven't mapped it out exactly, but I believe this will add additional queries to several functions, where we need to get the active DraftChangeLogContext (for a given learning_package_id) in order to resolve TIMESTAMP_AUTO and AUTHOR_AUTO. Additionally, functions which only take an entity_id (or version_id, etc.) will need to perform an extra extra query in order to get the operation's learning_package_id.

Status

I'm just looking for feedback on the approach currently.

Done:

• Implemented POC, just for src/openedx_content/applets/publishing.
• mypy passes in src/openedx_content/applets/publishing.

Todo:

• Unit tests
• Implement it in the rest of the applets.
• Manual tests
• Write up the BREAKING CHANGEs in more detail
• openedx-platform PR

AI

This code is all written by hand, but I used Claude Opus 4.7 to help me plan it. In particular, it suggested that:

Log-only draft APIs raise inside a context. set_draft_version, soft_delete_draft, and reset_drafts_to_published raise ValueError if called inside a bulk_draft_changes_for block with a non-AUTO_* author or timestamp, because the underlying DraftChangeLog records those fields at the log level (not per record). Outside a context the explicit values are honored.

which led me to add the can_override_context parameters.

[kdmccormick]

@ormsbee @bradenmacdonald Let me know what you think about this approach, and whether you feel confident enough in it that I should rush it in for Verawood or wait until post-1.0.

[bradenmacdonald]

In the future, we could add a UserID type here, and/or move these types to openedx-core: https://github.com/openedx/openedx-platform/blob/master/openedx/core/types/user.py

I suspect that without modifying Django itself, aliasing UserID = int is the best we can do.

[bradenmacdonald]

We could use djngo-crum or something to try to auto-detect the user, but I think it's better to just explicitly pass it in.

[bradenmacdonald]

Do we need this cast? If changed_at is a Timestamp and it's not TIMESTAMP_AUTO then it must be a datetime already right?

[kdmccormick]

Yeah, but I couldn't get mypy to believe it.

[bradenmacdonald]

I believe this will add additional queries to several functions, where we need to get the active DraftChangeLogContext (for a given learning_package_id) in order to resolve TIMESTAMP_AUTO and AUTHOR_AUTO

Getting the active DraftChangeLogContext doesn't use any queries, AFAIK.

Additionally, functions which only take an entity_id (or version_id, etc.) will need to perform an extra extra query in order to get the operation's learning_package_id.

Yes, that would be a an extra query.

[kdmccormick]

ensure to add test data where datetime is None

[ormsbee]

Maybe DATETIME_AUTO? In Python, "timestamp" specifically means a Unix float timestamp, i.e. seconds since the epoch. There's even a timestamp() method on datetime objects.