feat: Allow IDs or Models to be passed to all publishing APIs

[kdmccormick]

Part of: #562

To reduce noise in the implementations, I'm proposing these general patterns:

• Keep the param positional so that callers don't care if it's model, model_id, or model_or_id.
• When the function just needs the ID, name the param model_id and immediately invoke model_id = get_model_id(model_id).
• When the function needs the model instance, name the param model and immediately invoke model = get_<model>(model).

If this looks good, I'll apply it to the other applets as well.

BREAKING CHANGE: Some API functions which previously accepted int now accept <Model>.ID. Runtime behavior has not changed, but callers may need to use additional annotations or typing.cast(...) in order to get their typechecker to pass.

[kdmccormick]

Yes, I know this is funny :P But it makes the rest of the code simpler.

[kdmccormick]

list is invariant (its items must be the exact type, not subclasses thereof), whereas Sequence is covariant, thus more likely to behave with mypy the way consumers expect.

[kdmccormick]

@bradenmacdonald Could you review? If you like it, I'll do the other applets this way too.

[bradenmacdonald]

@kdmccormick I like it!

Could you please add a test for the get_model_id() helper that uses assert_type to verify that it's returning a typed ID like LearningPackageID correctly? It is working perfectly already; I just want to ensure that it stays working and doesn't downgrade to returning typing.All in some future refactor.

[bradenmacdonald]

@kdmccormick I'm curious why get_model_id() wasn't typechecking properly? I tried out your branch before and it seemed to be working fine.

[kdmccormick]

@bradenmacdonald It passes mypy. The issue is that mypy seems to effectively consisder get_model_id to return Any rather than <Any subclass of int>.

For example.

# given:
my_model_or_id: Model | int

# these pass, as we'd expect
my_id: LearningPackage.ID = get_model_id(my_model_or_id)
my_id: int = get_model = get_model_id(my_model_or_id)

# but this passes too :(
my_id: str = get_model_id(my_model_or_id)

It seems that mypy doesn't respect bound=int on the return TypeVar.

You're welcome to take a whack at it--I'm going to put this down to focus on the timestamp & author stuff.

[bradenmacdonald]

@kdmccormick

The problem you're encountering is that you're defining models in test_django_app/models.py but Django-stubs is only auto-detecting models in our main project:

openedx-core/mypy.ini

Lines 12 to 13 in c3d876d

	[mypy.plugins.django-stubs]
	django_settings_module = "projects.dev"

So it's not properly typing all the relation and ID accessors on those particular models.

I think it's safe to change mypy to use test_settings as its Django settings, and then you'll find that your latest two revert commits cb7742e and fd4924d aren't necessary.

# given:
my_model_or_id: Model | int
It seems that mypy doesn't respect bound=int on the return TypeVar.

I think you mean given my_model_or_id: Model | Model.ID right?

It seems to be working fine for me on 8dc3240

# tests/openedx_django_lib/test_typing.py

def more_tests(my_model_or_id: LearningPackage | LearningPackage.ID) -> None:
    # these pass, as we'd expect
    my_typed_id: LearningPackage.ID = get_model_id(my_model_or_id)
    my_int_id: int = get_model_id(my_model_or_id)

    # but this passes too :(
    my_str_id: str = get_model_id(my_model_or_id)

def more_tests2(my_model_or_id: MyTypedModel | MyTypedModel.ID) -> None:
    # these pass, as we'd expect
    my_typed_id: MyTypedModel.ID = get_model_id(my_model_or_id)
    my_int_id: int = get_model_id(my_model_or_id)

    # but this passes too :(
    my_str_id: str = get_model_id(my_model_or_id)

mypy:

tests/openedx_django_lib/test_typing.py:47: error: Incompatible types in assignment (expression has type "LearningPackageID", variable has type "str")  [assignment]
tests/openedx_django_lib/test_typing.py:55: error: Incompatible types in assignment (expression has type "MyTypedModelID", variable has type "str")  [assignment]