otacke / h5pxapikatchu

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

Save the HTTP referer and WordPress post_id in a new context table #23

Open bernhardkaindl opened 1 year ago

bernhardkaindl commented 1 year ago

Hello @otacke!

Thank you very much for your note about filtering verbs!

I'd like to send you this PR for review:

As an additional context array, I'd want to have the HTTP referrer and the corresponding WordPress post_id from which the H5P content was executed. For my use case, this would give me the remaining information that I'd need. I'm a newbee in all things Web, PHP, Javascript, and I may not yet have the spare learning time to implement a fully functioning H5P gradebook for WordPress using https://github.com/h5p/h5p-php-report yet.

I even used a LRS with https://github.com/otacke/wp-h5p-xapi/tree/remove-hard-directory and that gave me the H5P content URL, but later I also discovered that I also need to have a wordpress hook which also needs the post_id (to set a Tutor LMS lession in which the H5P is used to completed):

This pull request would add a hook to get me the info that I'd need.

Your nice implementation of separate tables for different XAPI data objects that I also use for this data table minimizes the added database space.

The commit message would be like:

Save the HTTP referer and WordPress post_id in a new context table and display the stored HTTP referer and the WordPress post_id data.

So far, I used the filter below with the additonal arguments to get the information I need.

    // Call a filter for filtering the context with all other XAPI data as additional arguments
    $context = filter_insert_data_context( $context, $actor, $verb, $object, $result, $xapi );

Instead, an action hook like the one below would be sufficient for me: I have not tested the hook instead of the filter yet, but I think it should work:

    // Call a hook to provide the complete data record for additional actions:
    do_action( 'h5pxapikatchu_data_record_complete', $context, $actor, $verb, $object, $result, $xapi );

I hope that you like the added post_id and HTTP referer, and that we can find a way for me to call a hook with them!

otacke commented 1 year ago

Hi @bernhardkaindl!

Thanks for your pull request. I wish you had reached out to pre-qualify this change beforehand. I actually do not want to store more inside the database just in order to cover specific needs beyond storing xAPI. I'd rather create a separate "gradebook" plugin or a pull request with the same functionality for the H5P plugin for WordPress.

It is fully possible to create your own plugin that grabs the data from H5PxAPIkatchu, amends it with the context information that you require and that then puts everything into a database table. Side note: Yes, the current tables follow a common star schema of data warehouse modeling. That should make aggregation of data simple for you to display it in a fashion that you prefer for your plugin.

It's also possible to run a fork of this plugin, of course.

I'd also not mind transferring the plugin to you. It's stable and I don't plan any further development, so you taking over the mantle might be good for both you and people who want to use the plugin for more than having a simple xAPI storage solution. The only thing that would be required is to remove the SNORDIAN references.

bernhardkaindl commented 11 months ago

Hello Oliver!

Thank you very much for your kind review comment!

I briefly deployed this PR for testing with real users.

I found out that many of my users use privacy-oriented HTTP browsers like the Duck-Duck-Go browser.

What I also saw was that the HTTP referer was not recorded in many cases, so I could not rely on the saved data for finding the Tutor LMS lesson's post ID in order to be able to mark the lesson as completed.