Skip to content

COMMIT 3: Extracting Logic into Hack Layer#3

Draft
awagner wants to merge 1 commit intoMBS-10152-commit2from
MBS-10152-commit3
Draft

COMMIT 3: Extracting Logic into Hack Layer#3
awagner wants to merge 1 commit intoMBS-10152-commit2from
MBS-10152-commit3

Conversation

@awagner
Copy link

@awagner awagner commented Sep 18, 2025

No description provided.

@awagner awagner marked this pull request as draft September 18, 2025 08:06
@@ -0,0 +1,61 @@
# MODIFICATIONS
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really appreciate this description of the modifications!


The latest development version can be found on Github:
https://github.com/lernlink/moodle-mod_individualfeedback

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently not aware of our contract, but we need to talk about, who is maintaining the plugin in the future.

## 4.5
## 4.6.1

- As we relay on the same DB structure as before, we DO NOT need to run a /db/upgrade.php script. All we actually only need to do, is to rename / move the current directory.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As already mentioned, the code should be able to use existing database tables. So we need to rename the tables: indfeedback_sitecourse_map, indfeedback_completedtmp to your new names: individualfeedback_sitecourse_map, individualfeedback_completedtmp

}

// Rename columns in individualfeedback_completedtmp table
$table = new xmldb_table('individualfeedback_completedtmp');
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause an upgrade error, if tables already exist, because according to the former length limit for tablenames of 28 chars, the tablenames are: indfeedback_sitecourse_map, indfeedback_completedtmp.

Therefore the tables must be renamed before the columns are renamed.

@@ -0,0 +1,290 @@
YUI.add('moodle-mod_individualfeedback-dragdrop', function(Y) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feedback module in Moodleversion 4.3.5 (version: 2024100701) does not contain any yui JS. So we expect to get rid of these JS placed in a yui module. Can you please remove and put the code into an amd module, if any of this javascript is necessary.

$module = $DB->get_record('modules', ['name' => 'individualfeedback']);
$modules = $DB->get_records('course_modules',
['course' => $this->course->id, 'module' => $module->id, 'deletioninprogress' => 0]);
foreach($modules AS $feedback_item) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codingstyle: reformat the code along moodle guidelines.

@@ -0,0 +1,18 @@
<?php
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment according to moodle guidelines?


/**
* Feedback event handler definition.
* individualfeedback event handler definition.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamings in commit 2 in general and all affected files.

* @package mod_individualfeedback
*/

defined('MOODLE_INTERNAL') || die();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No subtabs in Moodle 4.5.3 how is this file called?

@@ -0,0 +1,18 @@
function checkstartendgroupitem() {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain the meaning on this file?

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