edx-platform icon indicating copy to clipboard operation
edx-platform copied to clipboard

feat: Add Library Collections REST endpoints [FC-0062]

Open yusuf-musleh opened this issue 1 year ago • 5 comments

Description

This PR adds REST endpoints (list/create/update/delete) for Library Collections, in addition to emitting events when a library is created/updated. The deleted endpoint and emitting its event will be implemented in a later ticket, however the implementation skeleton is there.

Supporting information

Related Tickets:

  • https://github.com/openedx/modular-learning/issues/226

Depends on / blocked by:

  • https://github.com/openedx/openedx-events/pull/386
  • https://github.com/openedx/openedx-learning/pull/223

Testing instructions

  1. Go over new tests, make sure they cover most cases and make sure they are passing
  2. For additional manual testing:
    1. Checkout this branch in your local dev environment
    2. Make sure you have https://github.com/openedx/openedx-events/pull/386 + https://github.com/openedx/openedx-learning/pull/223 installed and running locally
    3. Make sure you already have a V2 library or created one if not, e.g lib:SampleTaxonomyOrg1:AL1
    4. Add some blocks to that library.
    5. In an external HTTP client (like curl/postman or similar) make the following requests (make sure you have the correct headers for authentication, you can copy them from the network tab in your browser), replace the library key in the URL with your actual library key:
      1. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/ - Should return a list of all collections that belong to the library, initially it will be empty
      2. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/ - Should return a specific collection (with key="COL1") that belongs to the library, initially this should return 404, not found
      3. POST http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/
        1. Payload: {"key": "COL1", "title":"Collection 1", "description": "Desc for Collection 1"}
        2. This should return the newly created collection
      4. PATCH http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/
        1. Payload: {"title": "Collection 1 updated title"}
        2. This should return the updated collection
      5. POST http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/
        1. Payload: {"key": "COL2", "title":"Collection 2", "description": "Desc for Collection 2"}
        2. Should return another newly created collection
      6. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/ - This should now return the 2 collections created above
      7. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/ - Should return the first collection we created
      8. DELETE http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/ - Should not delete and return a 405
      9. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/blocks/
        1. Should return the list of blocks added to this library
        2. Note their usage key values for the steps below.
      10. PATCH http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/contents/
        1. Payload: {"usage_keys": [ <list of usage keys from the library blocks> ]
        2. Should return {"count": <number of components in list>}
        3. Check the logs and confirm that the event was emitted for each component, triggering an "upsert" into Meilisearch.
      11. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/
        1. The modified timestamp should be updated.
      12. DELETE http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/contents/
        1. Payload: {"usage_keys": [ <list of usage keys from the library blocks> ]
        2. Should return {"count": <number of components in list>}
        3. Check the logs and confirm that the event was emitted for each component, triggering an "upsert" into Meilisearch.
      13. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/
        1. The modified timestamp should be updated.
      14. GET http://studio.local.edly.io:8001/api/libraries/v2/lib:SampleTaxonomyOrg1:AL1/collections/COL1/contents/ - will raise a 405, as an object's collections will be fetched from Meilisearch.

Private-Ref: FAL-3783

yusuf-musleh avatar Aug 19 '24 04:08 yusuf-musleh

Thanks for the pull request, @yusuf-musleh!

What's next?

Please work through the following steps to get your changes ready for engineering review:

:radio_button: 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.

:radio_button: 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

:radio_button: 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.

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

Who will review my changes?

This repository is currently maintained by @openedx/wg-maintenance-edx-platform. 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

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

openedx-webhooks avatar Aug 19 '24 04:08 openedx-webhooks

@rpenido @pomegranited Thanks for the reviews/comments! I've addressed them all here: 5bf9422

yusuf-musleh avatar Aug 29 '24 06:08 yusuf-musleh

Does this account for the new requirement that collections will need slugs, as discussed on the opaque-keys PR, or is that coming in a separate PR?

bradenmacdonald avatar Aug 30 '24 18:08 bradenmacdonald

@bradenmacdonald

Does this account for the new requirement that collections will need slugs, as discussed on the opaque-keys PR, or is that coming in a separate PR?

No, that requirements change isn't handled here, but we can if needed.

This PR uses the collection.id to identify the collection in the REST API. Should it use this new slug instead? Or maybe the full LibraryCollectionLocator ?

pomegranited avatar Sep 02 '24 04:09 pomegranited

@pomegranited I created https://github.com/open-craft/edx-platform/pull/683 with some small changes. Those changes are used in https://github.com/openedx/frontend-app-course-authoring/pull/1259, I've seen that it's easier to add them in this PR. Let me know what you think

ChrisChV avatar Sep 10 '24 17:09 ChrisChV

Hi @ChrisChV! I found a small issue in the id for the document in the meilisearch index. I updated it here: 1738447695c56069e13a9474a338e38f29d09c81

Our key field is not unique (we can have the same key for multiple libraries). As this id is for meilisearch only and not exported or used elsewhere, I think it is safe to use the Collection id instead of the key.

rpenido avatar Sep 11 '24 16:09 rpenido

@rpenido Looks good and works well :+1:

ChrisChV avatar Sep 11 '24 17:09 ChrisChV

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot avatar Sep 11 '24 19:09 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot avatar Sep 11 '24 20:09 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot avatar Sep 11 '24 20:09 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot avatar Sep 11 '24 20:09 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot avatar Sep 11 '24 20:09 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot avatar Sep 11 '24 20:09 edx-pipeline-bot

@rpenido

As this id is for meilisearch only and not exported or used elsewhere, I think it is safe to use the Collection id instead of the key.

Good catch on the id -- but we still need the Collection.key to be present in that data, because that's required for accessing the REST APIs.

Will follow up with on this with FAL-3787 CC @ChrisChV

pomegranited avatar Sep 12 '24 04:09 pomegranited