Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ChristopherChudzicki opened this issue Sep 5, 2023 · 4 comments

Comments

@ChristopherChudzicki
Copy link
Contributor

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
Copy link
Member

mbertrand commented Sep 7, 2023

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
Copy link
Member

mbertrand commented Sep 7, 2023

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

@ChristopherChudzicki
Copy link
Contributor Author

ChristopherChudzicki commented Sep 8, 2023

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
Copy link
Contributor Author

ChristopherChudzicki commented Sep 11, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants