otacke / h5pxapikatchu

WordPress plugin to store xAPI statements emitted by H5P
MIT License
10 stars 6 forks source link

WP multisite mode missing #22

Open osa-freiburg opened 1 year ago

osa-freiburg commented 1 year ago

Hi Oliver,

hope you are well, OSA from Freiburg calling. :)

I tested this plugin for the Online Studychoice Assessments (Online Studienwahl Assistenten) at the University of Freiburg and had to made a few adjustments for make it work on a multisite installation. Now it works just great and gives us a deeper insight into the H5P test results (anonymous as you don't have to log in for doing the tests) - really useful.

For multisite installations we need a flexible way for storing the configuration of H5PxAPIkatchu plugin. Be aware that the H5P content id could be the same in different multisite blogs.

So my idea is to create different config files to store the settings of H5PxAPIkatchu plugin for each blog separately. As each blog has its own id, we could add this blog id to the name of the config file to distinguish it easily.

For example instead of /js/h5pxapikatchu-config.js in a WP multisite installation this would be /js/h5pxapikatchu-config2.js /js/h5pxapikatchu-config3.js /js/h5pxapikatchu-config4.js where 2, 3, 4 and so on are the blog ids.

I tested it successfully on our WP multisite installation. You can check my customizations here :

https://github.com/osa-freiburg/h5pxapikatchu/tree/multisite-mode

What do you think about it? If you could take a peek at it sometime, I'd really appreciate some feedback. Thanks, and take care! Anja

otacke commented 1 year ago

I am not familiar with the requirements of WordPress' multisite sites or with how they work.

Will database handling need to be adjusted as well? Will the results of all sub sites be written to the same database tables? Or does WordPress itself take care of proper routing into separate database tables? I'd assume the latter, handled via $wpdb->prefix, then that would be covered.

The code looks fine (without testing it) in general. I think it would be more concise if blocks were not duplicated, but when concatenating the segments of a path, then . $blog_id . could either be the blog id - preferably separated with a - (for multisite) or '' (for singlesite).

osa-freiburg commented 1 year ago

You are correct: WordPress creates its own separate database tables for each Multisite instance, so that database handling needs no further adjustion. Also, thanks for the suggestion for code improvement. I will definitely look into that. In our live system, it works flawlessly, by the way. :)

One small hurdle we encountered is that for security reasons we restricted access to the backend (/wp-admin) to specific IPs. In that case, the plugin cannot operate because when accessed through the frontend wp-admin/admin-ajax.php returns a 403 error. This can be resolved, by explicitly allowing access to this file (customizing the .htaccess file on Apache servers). Of course, this applies in general to all plugins that utilize admin-ajax.php.

fishfree commented 1 month ago

@otacke @osa-freiburg Hello! I tried both of your plugins in my multi-site Wordpress. However, I both cannot select H5P types to capture as the screenshot below: image

otacke commented 1 month ago

@fishfree I have not received a pull request for any changes, and as I stated before, I am not familiar with the requirements of WordPress' multisite sites or with how they work. It's probably easy-peasy, but I currently lack the free time to look into this.

otacke commented 1 month ago

@fishfree One extra question: You notice that you have the "Capture the xAPI statements of all H5P content types" option checked, right? If that is checked, you cannot select or unselect individual content types - regardless of having multisite support or not.

fishfree commented 1 month ago

@otacke Thank you very much for your tip! When I uncheck "Capture the xAPI statements of all H5P content types", then "H5P content types" can be checked or not. Sorry for my ignorance ~