feat: adds Collection.key #223

pomegranited · 2024-09-03T07:40:10Z

Description

Adds a new required field to the Collection model: key (string), which uniquely identifies a Collection within a Learning Package.
Existing Collection keys are initialized with a random string.

Supporting information

Part of : openedx/modular-learning#226

Private-ref: FAL-3783

Testing instructions

See openedx/edx-platform#35321 for full testing instructions.

Testing migrations:

In an openedx-learning virtual environment, run:

Create an openedx-learning virtual environment
Install openedx-learning @ main, and run python3 manage.py migrate

Create a collection python3 manage.py shell:

import openedx_learning.api.authoring as api
learning_package = api.create_learning_package("LP1", "Learning Package 1")
for x in range(0, 11):
    api.create_collection(learning_package.id, title=f"Collection {x}", created_by=None)
    # <Collection> (1:Collection 1)
    # <Collection> (2:Collection 2)
    ...
    # <Collection> (10:Collection 10)

Install openedx-learning @ open-craft/jill/collection-key
Run python3 manage.py migrate

Check that a key was generated for your collection python3 manage.py shell:

import openedx_learning.api.authoring as api
learning_package = api.get_learning_package_by_key("LP1")
collections = api.get_collections(learning_package.id)
for collection in collections.all():
    print(collection.key)
    # gy19KxxV9xrjEMmR4TRNeE2p8sEbQfetVjZPzKPUbHURAgAgJY
    # RNeE2p8sEbQfetVjZPzKPUbgy19KxxV9xrjEMmR4TRNeE2p8sEb
    ...

openedx-webhooks · 2024-09-03T07:40:14Z

Thanks for the pull request, @pomegranited!

What's next?

Please work through the following steps to get your changes 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.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @axim-engineering. Tag them in a comment and let them know that your changes are ready for review.

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.

which uniquely identifies a Collection within a Learning Package. Existing Collection keys are initialized with a random string.

to identify Collections. We do this because the `key` will be used in the Collection's opaque key (not the ID).

rpenido

Hi @pomegranited! This is looking good!

Only a few nits

openedx_learning/apps/authoring/collections/models.py

rpenido · 2024-09-04T19:48:42Z

openedx_learning/apps/authoring/collections/models.py

@@ -165,7 +186,7 @@ def __str__(self) -> str:
 """
 User-facing string representation of a Collection.
 """
- return f"<{self.__class__.__name__}> ({self.id}:{self.title})"
+ return f"<{self.__class__.__name__}> ({self.key}:{self.title})"


This would not be unique. Do you think we should add the library key here?

Sure, see bdcb52b

openedx_learning/apps/authoring/collections/migrations/0004_collection_key.py

rpenido · 2024-09-06T00:40:56Z

openedx_learning/apps/authoring/collections/api.py

- collection_id: int,
+ learning_package_id: int,
+ key: str,
+ *,
 title: str | None = None,
 description: str | None = None,


Sorry for not catching this before. 😞
I think we should be able to update the key here.

Suggested change

description: str | None = None,

description: str | None = None,

new_key: str,

Hmm.. if we allow that, it'll change the Collection's opaque key, which will disconnect it from any applied tags.. so I think it should be immutable.

You're right! My bad!

rpenido

LGTM 👍
Thank you for your work, @pomegranited!

I commented about adding a new parameter to allow the user to update the key. Other than that, everything is good on my end.

I tested this using the instructions from feat: Add Library Collections REST endpoints [FC-0062] edx-platform#35321
I read through the code
~~I checked for accessibility issues~~
Includes documentation

pomegranited · 2024-09-06T04:45:20Z

Hi @ormsbee , this is a minor PR, but I'd appreciate your review if you have time?

rpenido

Thank you for the changes, @pomegranited!

LGTM 👍

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 3, 2024

pomegranited force-pushed the jill/collection-key branch from 2a5bfac to f4a6350 Compare September 4, 2024 01:42

pomegranited added 3 commits September 4, 2024 12:42

feat: adds Collection.key

b6d7797

which uniquely identifies a Collection within a Learning Package. Existing Collection keys are initialized with a random string.

refactor: updates collections api to use learning_package_id + key

8b17282

to identify Collections. We do this because the `key` will be used in the Collection's opaque key (not the ID).

chore: bumps version to 0.11.4

56b5fb6

pomegranited force-pushed the jill/collection-key branch from a300346 to 56b5fb6 Compare September 4, 2024 03:13

pomegranited mentioned this pull request Sep 4, 2024

feat: Add Library Collections REST endpoints [FC-0062] openedx/edx-platform#35321

Merged

pomegranited marked this pull request as ready for review September 4, 2024 03:24

pomegranited requested a review from rpenido September 4, 2024 03:24

rpenido suggested changes Sep 4, 2024

View reviewed changes

pomegranited added 2 commits September 5, 2024 09:21

fix: ensure unique key in data migration

8b75af1

fix: add lp.id to Collection repr/str

bdcb52b

pomegranited mentioned this pull request Sep 5, 2024

Store content object collections in search index [FC-0062] open-craft/edx-platform#680

Closed

rpenido reviewed Sep 6, 2024

View reviewed changes

pomegranited requested a review from ormsbee September 6, 2024 04:50

rpenido approved these changes Sep 10, 2024

View reviewed changes

pomegranited merged commit 6fa83ba into openedx:main Sep 10, 2024
9 checks passed

pomegranited deleted the jill/collection-key branch September 10, 2024 23:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds Collection.key #223

feat: adds Collection.key #223

pomegranited commented Sep 3, 2024 •

edited

Loading

openedx-webhooks commented Sep 3, 2024 •

edited

Loading

rpenido left a comment

rpenido Sep 4, 2024

pomegranited Sep 4, 2024 •

edited

Loading

rpenido Sep 6, 2024 •

edited

Loading

pomegranited Sep 6, 2024

rpenido Sep 6, 2024

rpenido left a comment

pomegranited commented Sep 6, 2024

rpenido left a comment

	description: str \| None = None,
	description: str \| None = None,
	new_key: str,

feat: adds Collection.key #223

feat: adds Collection.key #223

Conversation

pomegranited commented Sep 3, 2024 • edited Loading

Description

Supporting information

Testing instructions

openedx-webhooks commented Sep 3, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

rpenido left a comment

Choose a reason for hiding this comment

rpenido Sep 4, 2024

Choose a reason for hiding this comment

pomegranited Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

rpenido Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

pomegranited Sep 6, 2024

Choose a reason for hiding this comment

rpenido Sep 6, 2024

Choose a reason for hiding this comment

rpenido left a comment

Choose a reason for hiding this comment

pomegranited commented Sep 6, 2024

rpenido left a comment

Choose a reason for hiding this comment

pomegranited commented Sep 3, 2024 •

edited

Loading

openedx-webhooks commented Sep 3, 2024 •

edited

Loading

pomegranited Sep 4, 2024 •

edited

Loading

rpenido Sep 6, 2024 •

edited

Loading