gocodebox / lifterlms-rest

LifterLMS REST API Feature Plugin
6 stars 7 forks source link

Handle registered meta and registered rest fields #243

Closed eri-trabiccolo closed 1 year ago

eri-trabiccolo commented 3 years ago

Description

Fixes #157 also fixes #73

How has this been tested?

manually

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

eri-trabiccolo commented 3 years ago

@thomasplevy thanks. Have to check the issue you reported. I might have made some changes after the latest working test :) About _llms_length... well it should be skipped automatically because of length being an item schema property, if it doesn't happen, that's another issue to look into. Thank you man! ❤️

thomasplevy commented 3 years ago

About _llms_length... well it should be skipped automatically because of length being an item schema property, if it doesn't happen, that's another issue to look into. Thank you man! heart

I saw this when performing an OPTIONS /courses to review the schema when trying to add fields. I tried to exclude it too (via the disallowed_meta prop) and it was still showing up on the OPTIONS request. I didn't actually try to set it and it wasn't being returned in the meta array.

eri-trabiccolo commented 3 years ago

@thomasplevy you were right about the _llms_length I wasn't exlcuding it from the schema, only from the responses. fixed in d55c312

eri-trabiccolo commented 3 years ago

You were right about the registered fields (~via register_rest_field - haven't re-tested the ones via register_meta though, I'll do asap~ ) too, there was an error in how I handled the schema properties skipping. I fixed it in ~c16d03d~75dcb26 I used this example to test the rest field: https://developer.wordpress.org/reference/functions/register_rest_field/#comment-3434 just changing post with course and $object->ID with $object->get('ID')

Fixed meta update in 141cca0

eri-trabiccolo commented 2 years ago

This massive PR is ready to be reviewed. Includes some code reorganization/refactoring (not exhaustive though) to reduce some redundancy. What it basically does is:

thomasplevy commented 2 years ago

@eri-trabiccolo sorry I haven't gotten a look at this yet. We'll get to it thank you <3

eri-trabiccolo commented 1 year ago

@seothemes can I get your functional review on this? Test everything works fine. Let me know when you can start and we'll chat a bit on the aims of this PR if it's not clear (it's a pretty big one :D)

eri-trabiccolo commented 1 year ago

Thanks! Gonna rebase and merge! <3