Fixes database queries related to loading content libraries #4
Conversation
juho-jaakkola
force-pushed
the
library_queries
branch
from
2ace974 to
9f126ac
Compare
| 'libraryMajorVersion' => $data->major_version, | ||
| 'libraryMinorVersion' => $data->minor_version, | ||
| 'libraryEmbedTypes' => $data->embed_types, | ||
| 'libraryFullscreen' => $data->fullscreen, |
There was a problem hiding this comment.
Custom array key names are added here manually just so that they can be immediately renamed again into another array by H5PCore::loadContent()in https://github.com/h5p/h5p-php-library/blob/master/h5p.classes.php#L1735
I'm noticing a bit of a code smell... :)
I think in the long-term the integration library should be refactored to always pass objects as function arguments. That way properties could always be accessed with methods instead of arbitrary array keys. Additionally input type hinting could be used to further clarify what each method is expecting.
I opened a separate issue for further planning and discussion: h5p/h5p-php-library#11
There was a problem hiding this comment.
Yes, this is far from ideal.
I suggest we get it working before we start any refactoring.
|
This looks very good to me. I'll merge it in and do some more testing. Good job! |
Fixes database queries related to loading content libraries
Backport from h5p master
Add support for result.success & completion
Work in progress. Do not merge without proper testing and review.
I'm still not completely sure what exactly the integration should be doing, so some of the changes may include mistakes.