Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 4.0.0

* Breaking: ``LibraryCollectionLocator.collection_id`` is renamed to ``LibraryCollectionLocator.collection_code``.
There is a readonly ``collection_id`` property for backwards compatibility, but calls to the constructor
which use the ``collection_id=`` kwarg must be updated to ``collection_code=``.
Comment on lines +4 to +5
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why can't we make the constructor accept a backwards compatible kwarg, so this isn't a breaking change? Because it's difficult, or because we want to force a change sooner and avoid having an inconsistent state for a long time?


# 3.1.0

* The Django OpaqueKeyField subclasses like CourseKeyField now specify a default
Expand Down
2 changes: 1 addition & 1 deletion opaque_keys/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from stevedore.enabled import EnabledExtensionManager

__version__ = '3.1.0'
__version__ = '4.0.0'


class InvalidKeyError(Exception):
Expand Down
27 changes: 17 additions & 10 deletions opaque_keys/edx/locator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1625,30 +1625,30 @@ def html_id(self) -> str:
class LibraryCollectionLocator(CheckFieldMixin, CollectionKey):
"""
When serialized, these keys look like:
lib-collection:org:lib:collection-id
lib-collection:{org_code}:{library_code}:{collection_code}
"""
CANONICAL_NAMESPACE = 'lib-collection'
KEY_FIELDS = ('lib_key', 'collection_id')
KEY_FIELDS = ('lib_key', 'collection_code')
lib_key: LibraryLocatorV2
collection_id: str
collection_code: str

__slots__ = KEY_FIELDS
CHECKED_INIT = False

# Allow collection IDs to contian unicode characters
COLLECTION_ID_REGEXP = re.compile(r'^[\w\-.]+$', flags=re.UNICODE)
COLLECTION_CODE_REGEXP = re.compile(r'^[\w\-.]+$', flags=re.UNICODE)

def __init__(self, lib_key: LibraryLocatorV2, collection_id: str):
def __init__(self, lib_key: LibraryLocatorV2, collection_code: str):
"""
Construct a CollectionLocator
"""
if not isinstance(lib_key, LibraryLocatorV2):
raise TypeError("lib_key must be a LibraryLocatorV2")

self._check_key_string_field("collection_id", collection_id, regexp=self.COLLECTION_ID_REGEXP)
self._check_key_string_field("collection_code", collection_code, regexp=self.COLLECTION_CODE_REGEXP)
super().__init__(
lib_key=lib_key,
collection_id=collection_id,
collection_code=collection_code,
)

@property
Expand All @@ -1658,6 +1658,13 @@ def org(self) -> str | None: # pragma: no cover
"""
return self.lib_key.org

@property
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add the @deprecated decorator? It's technically a Python 3.13 feature but you can get it via from typing_extensions import deprecated in 3.12.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

good idea. i'll do that when I open the broader renaming PR

def collection_id(self) -> str:
"""
Backcompat alias for collection_code
"""
return self.collection_code

def _to_string(self) -> str:
"""
Serialize this key as a string
Expand All @@ -1670,9 +1677,9 @@ def _from_string(cls, serialized: str) -> Self:
Instantiate this key from a serialized string
"""
try:
(org, lib_slug, collection_id) = serialized.split(':')
lib_key = LibraryLocatorV2(org, lib_slug)
return cls(lib_key, collection_id)
(org, library_code, collection_code) = serialized.split(':')
lib_key = LibraryLocatorV2(org, library_code)
return cls(lib_key, collection_code)
except (ValueError, TypeError) as error:
raise InvalidKeyError(cls, serialized) from error

Expand Down
10 changes: 5 additions & 5 deletions opaque_keys/edx/tests/test_collection_locators.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ def test_coll_key_constructor(self):
lib = 'LibraryX'
code = 'test-problem-bank'
lib_key = LibraryLocatorV2(org=org, slug=lib)
coll_key = LibraryCollectionLocator(lib_key=lib_key, collection_id=code)
coll_key = LibraryCollectionLocator(lib_key=lib_key, collection_code=code)
lib_key = coll_key.lib_key
assert str(coll_key) == "lib-collection:TestX:LibraryX:test-problem-bank"
assert coll_key.org == org
assert coll_key.collection_id == code
assert coll_key.collection_code == code
assert lib_key.org == org
assert lib_key.slug == lib
assert isinstance(coll_key, CollectionKey)
Expand All @@ -44,9 +44,9 @@ def test_coll_key_constructor_bad_ids(self):
lib_key = LibraryLocatorV2(org="TestX", slug="lib1")

with self.assertRaises(ValueError):
LibraryCollectionLocator(lib_key=lib_key, collection_id='usage-!@#{$%^&*}')
LibraryCollectionLocator(lib_key=lib_key, collection_code='usage-!@#{$%^&*}')
with self.assertRaises(TypeError):
LibraryCollectionLocator(lib_key=None, collection_id='usage')
LibraryCollectionLocator(lib_key=None, collection_code='usage')

def test_coll_key_from_string(self):
org = 'TestX'
Expand All @@ -57,7 +57,7 @@ def test_coll_key_from_string(self):
assert coll_key == CollectionKey.from_string(str_key)
assert str(coll_key) == str_key
assert coll_key.org == org
assert coll_key.collection_id == code
assert coll_key.collection_code == code
lib_key = coll_key.lib_key
assert isinstance(lib_key, LibraryLocatorV2)
assert lib_key.org == org
Expand Down