Comments (16)
Shall we expose the password field? I think we should not, I think we should allow its creation/update (so context=edit) only.
Good catch, see new issue to track: #9
Access plans: Maybe we can add access plans ids and/or add them to the _links array as well.
Per #1 access plans will be their own resource at /plans
-- once that resource is defined then courses will have an _link
to the collection of relevant plans.
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.
What about Sections and Lessons: shouldn't we add them to _links?
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 courses/{id}/contents
Links will be added to the course
schema too when that's specced out.
Since we have the comment(ping)_status we can also add the review_status. Probably we can either add comments/ping/review links as related resources.
Nice catch, I completely forgot about reviews (as is often the case). See #10
I see you removed the date_gmt property for creation/update methods. I think we should add it back, otherwise wp_insert_post it can retrieve it from the date but only if the data contains informations about the gmt "offset"... otherwise data_gmt will be equal to date. Does this make sense to you? If so, let's reintroduce it :D
Oh yea, you're right, see #11
from lifterlms-rest.
@thomas
I see what you mean yes, thanks.
- I missed the read-only "mark" (though I guessed it :D), sorry, I was looking at the html not the json.
- No need to be enum then, it's "fixed", and it's the current "object" post_type.
from lifterlms-rest.
A sort of "capacity status" should be added, we have the course's
enable_capacity
bool.
Added as capacaity_enabled
, see 7b9712b#diff-f438ec0990118ddfa46daae2716d8ff2R67
from lifterlms-rest.
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.
Good to know I cannot trust it, will go with grep next time :D
from lifterlms-rest.
@eri-trabiccolo & @actual-saurabh can you please review and let me know if you find any holes / issues / whatever
from lifterlms-rest.
I start with some initial considerations on Courses:
- Shall we expose the password field? I think we should not, I think we should allow its creation/update (so context=edit) only.
- Access plans: Maybe we can add access plans ids and/or add them to the
_links
array as well. - What about Sections and Lessons: shouldn't we add them to
_links
? - Since we have the
comment(ping)_status
we can also add thereview_status
. Probably we can either add comments/ping/review links as related resources. - I see you removed the
date_gmt
property for creation/update methods. I think we should add it back, otherwisewp_insert_post
it can retrieve it from thedate
but only if the data contains informations about the gmt "offset"... otherwisedata_gmt
will be equal todate
. Does this make sense to you? If so, let's reintroduce it :D
from lifterlms-rest.
About the Course's access period object:
opens and closes message field in the specs contain shortcodes...
This makes me thing that we should split all the properties which can contain shortcodes (or blocks?!) into raw
and rendered
with the second only accessible in edit
context, exactly as we do with the title
,content
,excerpt
.
What do you think?
from lifterlms-rest.
I don't think I get the meaning of the "post_type" property for the Course resource. :D
Enum:"course" "llms_membership"
.
The course post_type can be only "course" no?!
from lifterlms-rest.
I don't think I get the meaning of the "post_type" property for the Course resource. :D
Enum:"course" "llms_membership"
.The course post_type can be only "course" no?!
In the /courses
resource it doesn't make sense, exactly, no. However when pulling courses as a link from other resources via link (like the /enrollments
"post" resource the post could be either a course or membership.
Unless we split every resource that can have either courses or memberships into separate resources (/course-enrollments
and /membership-enrollments
for example) having the post_type
property on course and memberships makes sense.
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 type
in the return of a /wp-json/wp/v2/posts
request, for an example
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 _links
to the post type information. However, this would require additional API calls or parsing to determine the post type instead of just reading the text field.
Edit:
Also note that the post_type
field is marked as read only for the purposes of our API resources. When creating/updating courses (and later memberships after we define that spec) you won't have to (and will not be able to) specify a post type. You'll only be able to read it during gets
from lifterlms-rest.
A sort of "capacity status" should be added, we have the course's enable_capacity
bool.
from lifterlms-rest.
@thomasplevy I have a "problem" :D
First I want to be sure how the Course "rendered" content should look like. Let me explain.
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...
Hence, the 'rendered' property would be:
apply_filters( 'the_content', $post->post_content );
And here's my problem:
If I use the code above, for some reason, the rendered content of a course built with the block editor has "pieces" like instructor, syllabus etc. duplicated. The duplication comes from the template hooks: e.g. add_action( 'lifterlms_single_course_after_summary', 'lifterlms_template_course_author', 40 );
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?
from lifterlms-rest.
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.
Since this action is run at wp
it's possible that it's not being run during rest_api_init
-- I'm guessing. Look into the action stack and see if that could be causing it.
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
from lifterlms-rest.
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
from lifterlms-rest.
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.
Do you still think this should be addressed in the blocks plugin?
Hope I've been understandable.
from lifterlms-rest.
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
A per my previous comment I've implemented it in this repo.
Basically when preparing the object (e.g. single course, or a course in a collection) for the response, I do the following:
- get a list of action/filters to be removed for the current object (a course, a membership and a lesson have a different list)
- remove them and keep track of what I removed
- prepare the object for response (e.g. get the rendered content along with the other properties)
- re-add the previously removed filters.
from lifterlms-rest.
@thomasplevy
I think we need a different course's sales_page_url
.
Looking at:
so the lifterlms core the sales_page_url
is not what the specs are describing right now.
The specs are calling sales_page_url
what for the corse is sales_page_content_url
https://github.com/gocodebox/lifterlms/blob/3.33.2/includes/models/model.llms.course.php#L276
I think we can change the specs so that sales_page_url
is a read-only param which equals to LLMS_Course->get_sales_page_url()
, while sales_page_type
, sales_page_page_id
, sales_page_content_url
(the former sales_page_url
) are available in both edit
and view
contexts.
Though maybe we can also consider to be more restrictive and make the latter 3 available in edit
context only. I mean, if you're a viewer should you really know how something has been built? I guess no, you shouldn't...
what do you think?
from lifterlms-rest.
Related Issues (20)
- access plan `availability_restrictions` property never returned HOT 3
- access plan `checkout_redirect_forced` property not included in REST resource HOT 1
- Cannot enable access plan `forced_redirect` property
- Creating or Updating access plans when there are 6 (max) plans associated with a course/membership results in errors
- Enrollment status update provides two triggers HOT 3
- Improve github pages (specs) cache busting
- Changing the Parent_ID for a lesson trigger using the API triggers an error HOT 2
- CodeQL Error
- Improve response fields filtering HOT 1
- multiple webhooks, same topic HOT 2
- Avoid fatals when searching for courses/memberships but the query post type parameter is forced to be something else
- Setup webhook automatic retries HOT 2
- Add webhook failure email (and admin?) notice(s)
- In the get enrollments for student API call I think there is an error where the pages available is greater than the actual pages for the paginated call. Happens when the page size is equal to the enrolments. HOT 2
- Try enrolling a user with ID 0 into a course: the current user will be enrolled into the course
- PHP Notice when listing API keys.
- Expose assignments and grades through REST API
- test
- Test
- test
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from lifterlms-rest.