Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bernhardkaindl
Copy link
Contributor

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!

and display the stored HTTP referer and the WordPress post_id data.
@otacke
Copy link
Owner

otacke commented Sep 21, 2023

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants