-
Notifications
You must be signed in to change notification settings - Fork 7
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
Resource Feedback for Courses, Students, Enrollments #6
Comments
@eri-trabiccolo & @actual-saurabh can you please review and let me know if you find any holes / issues / whatever |
I start with some initial considerations on Courses:
|
Good catch, see new issue to track: #9
Per #1 access plans will be their own resource at In the current state of LifterLMS plans belong to a course or membership but in the future we'd like to have global plans that can be shared. Moving them to their own resource in the rest api (instead of nested) will not require us to add global plans today but in the future when we do it'll be an easy transition on the api side.
Yes. As above, I'd like sections/lessons/quizzes/etc (collectively going to refer to them as "content" to be their own resource. See #1 again, the resource will be Links will be added to the
Nice catch, I completely forgot about reviews (as is often the case). See #10
Oh yea, you're right, see #11 |
About the Course's access period object: |
I don't think I get the meaning of the "post_type" property for the Course resource. :D The course post_type can be only "course" no?! |
In the Unless we split every resource that can have either courses or memberships into separate resources ( Additionally, following patterns from the WP Core, the post schema for any post type returns its post type as a parameter even when pulling it from the post type resource. See I'm open to solving this a different way but I think in this context it makes sense. The alternative I can come up with would be to add Edit: Also note that the |
@thomas
|
A sort of "capacity status" should be added, we have the course's |
Added as |
@thomasplevy I have a "problem" :D If we built the Course content with the block editor, stuff like the instructor and the syllabus are part of the rendered content. While if the course has been built with the "classic editor" instructor, syllabus etc. are rendered with filter applied to the content hook (well a filter that loads before and after templates that will trigger action hooks etc. etc.) Should the Course's rendered content always display such "contents"? I guess so... And here's my problem: I think these kind of actions are removed in the core when a course has blocks but I cannot find the code so that I can trigger it when building the rest response. Any hints? |
Since this action is run at If it is, I think the rest api plugin should duplicate this effort for now and also remove these actions like the blocks plugin does. I think. If this doesn't solve it open this as a new issue so we can track it more precisely |
Thanks, I think my editor is buggy, I searched for any "remove_action"/"remove_all_" throughout the lifterlms-blocks repo and couldn't find them. |
On second thought i think it'd be better to update this in the blocks plugin so that we don't need to duplicate all the existing logic there to determine if a post is migrated. There could just be an additional action in the constructor to run the same remove action method in the blocks plugin |
Mmm I think the problem here would be when retrieving the collection, the actions "removal" should be done for each course: I should remove those actions if the current post is migrated, but don't remove those if it is not, which means potentially re-adding them for not migrated posts. Which also means that I should check an action was actually hooked, otherwise I'll end up adding actions someone removed. |
A per my previous comment I've implemented it in this repo.
|
@thomasplevy so the lifterlms core the I think we can change the specs so that Though maybe we can also consider to be more restrictive and make the latter 3 available in what do you think? |
Open discussion thread for discussion of these resources as documented in the current state of the spec.
Let's pay special attention to:
_links
provided for each resource to help consumers gather additional information about related resources.https://gocodebox.github.io/lifterlms-rest/
The text was updated successfully, but these errors were encountered: