Skip to content

Commit

Permalink
MDL-77665 mod_h5pactivity: Improve generator package upload
Browse files Browse the repository at this point in the history
* Allow the specification of the username in order to set the owner
of the H5P file package when creating a new H5P instance.
  • Loading branch information
laurentdavid committed Jun 13, 2024
1 parent 0a69871 commit 96d55a2
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 10 deletions.
6 changes: 3 additions & 3 deletions h5p/tests/behat/h5p_deployment.feature
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ Feature: Undeployed H5P content should be only available to users that can deplo
| capability | permission | role | contextlevel | reference |
| moodle/h5p:updatelibraries | Allow | editingteacher | System | |
And the following "activities" exist:
| activity | name | intro | introformat | course | content | contentformat | idnumber |
| activity | name | intro | introformat | course | content | contentformat | idnumber |
| page | H5PPage | PageDesc1 | 1 | C1 | H5Ptest | 1 | 1 |
And I am on the H5PPage "page activity editing" page logged in as teacher1
And the following "contentbank content" exist:
| contextlevel | reference | contenttype | user | contentname | filepath |
| Course | C1 | contenttype_h5p | teacher1 | filltheblanks.h5p | /h5p/tests/fixtures/filltheblanks.h5p |
| contextlevel | reference | contenttype | user | contentname | filepath |
| Course | C1 | contenttype_h5p | teacher1 | filltheblanks.h5p | /h5p/tests/fixtures/filltheblanks.h5p |
And I click on the "Configure H5P content" button for the "Page content" TinyMCE editor
And I click on "Browse repositories..." "button" in the "Insert H5P content" "dialogue"
And I click on "Content bank" "link" in the ".fp-repo-area" "css_element"
Expand Down
51 changes: 51 additions & 0 deletions mod/h5pactivity/tests/behat/h5pactivity_deployment.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
@mod @mod_h5pactivity @core_h5p @_file_upload @_switch_iframe
Feature: Undeployed H5P activities packages should be available only to any user that can deploy packages.

Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| teacher2 | Teacher | 2 | teacher2@example.com |
| student1 | Student | 1 | student1@example.com |
And the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| teacher2 | C1 | editingteacher |
| student1 | C1 | student |
# Make sure that the teacher2 can update libraries so it show the right info when.
And the following "permission overrides" exist:
| capability | permission | role | contextlevel | reference |
| moodle/h5p:updatelibraries | Allow | editingteacher | System | |
# Now create the activity as teacher1.
And the following "activities" exist:
| activity | course | name | username | packagefilepath |
| h5pactivity | C1 | Music history | teacher1 | h5p/tests/fixtures/filltheblanks.h5p |
And I log in as "admin"
And I navigate to "Users > Accounts > Browse list of users" in site administration
And I press "Delete" action in the "Teacher 1" report row
And I click on "Delete" "button" in the "Delete user" "dialogue"
And I should see "Deleted user Teacher 1"

@javascript
Scenario: In an H5P activity, as student I should not be able to deploy the package if not deployed by the teacher
beforehand. Then if a second teacher deploys the package, I can see it.
Given I am on the "Music history" "h5pactivity activity" page logged in as student1
And I switch to "h5p-player" class iframe
And I should see "This file can't be displayed"
And I switch to the main frame
And I log out
# Then teacher2 will be allowed to deploy the package.
And I am on the "Music history" "h5pactivity activity" page logged in as teacher2
And I switch to "h5p-player" class iframe
When I switch to "h5p-iframe" class iframe
Then I should see "Of which countries are Berlin"
And I switch to the main frame
And I log out
# Now student1 should be able to see the package.
And I am on the "Music history" "h5pactivity activity" page logged in as student1
And I switch to "h5p-player" class iframe
When I switch to "h5p-iframe" class iframe
Then I should see "Of which countries are Berlin"
25 changes: 18 additions & 7 deletions mod/h5pactivity/tests/generator/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ public function create_instance($record = null, array $options = null): stdClass
if (!isset($record->reviewmode)) {
$record->reviewmode = manager::REVIEWCOMPLETION;
}

$globaluser = $USER;
if (!empty($record->username)) {
$user = core_user::get_user_by_username($record->username);
$this->set_user($user);
}
// The 'packagefile' value corresponds to the draft file area ID. If not specified, create from packagefilepath.
if (empty($record->packagefile)) {
if (!isloggedin() || isguestuser()) {
Expand All @@ -85,21 +89,28 @@ public function create_instance($record = null, array $options = null): stdClass
if (!file_exists($record->packagefilepath)) {
throw new coding_exception("File {$record->packagefilepath} does not exist");
}

$usercontext = context_user::instance($USER->id);

// Pick a random context id for specified user.
$record->packagefile = file_get_unused_draft_itemid();

// Add actual file there.
$filerecord = ['component' => 'user', 'filearea' => 'draft',
'contextid' => $usercontext->id, 'itemid' => $record->packagefile,
'filename' => basename($record->packagefilepath), 'filepath' => '/'];
$filerecord = [
'component' => 'user',
'filearea' => 'draft',
'contextid' => $usercontext->id,
'itemid' => $record->packagefile,
'filename' => basename($record->packagefilepath),
'filepath' => '/',
'userid' => $USER->id,
];
$fs = get_file_storage();
$fs->create_file_from_pathname($filerecord, $record->packagefilepath);
}

// Do work to actually add the instance.
return parent::create_instance($record, (array)$options);
$instance = parent::create_instance($record, (array)$options);
$this->set_user($globaluser);
return $instance;
}

/**
Expand Down

0 comments on commit 96d55a2

Please sign in to comment.