mitodl / mit-open

BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

/learningpaths and /learning_resources?resource_type=learning_path return different results #67

Open ChristopherChudzicki opened 1 year ago

ChristopherChudzicki commented 1 year ago

As a logged-in user with permission to edit learning paths:

I would expect the results to be the same.

For context: It seems most convenient on the frontend to just use the unified API /learning_resource API, which is how I came across this.

mbertrand commented 1 year ago

Since learning paths are CRUDable, we need at least one API endpoint to return both published and unpublished resources of this type (but only to people who have permissions to edit them), which is what the learningpaths endpoint is aimed at. The learning_resources endpoint could be modified to do the same but only for LearningPaths and only for authorized users, if that is desirable.

mbertrand commented 1 year ago

PS the LearningPathViewSet also has custom create and destroy functions for CRUD functionality that the read-only LearningResourceViewSet lacks.

ChristopherChudzicki commented 1 year ago

Having worked more on the learningpaths frontend, I do think it would be more convenient to consistently use the /learning_resources API for fetching details, listings, etc, rather than swapping endpoints.

It also seems unexpected to me that /learningpaths/:id might 200 be but /learning_resources/:id might 404 because of the published issue. I would expect /learningpaths/:id api to return a a subset of /learning_resources.

I made #71 for this behavior change.

PS the LearningPathViewSet also has custom create and destroy functions for CRUD functionality that the read-only LearningResourceViewSet lacks.

Yes, those are working well 👍

ChristopherChudzicki commented 1 year ago

In looking at #71, @mbertrand raised a good point about /learning_resources API and published/unpublished:

there may be times when it is only appropriate to show published paths (for instance, a carousel of top 10 newest learning paths or top 10 newest resources in general). One way to avoid this is to add published to the filterset_fields above, and filter by ?published=true whenever necessary...