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

Activity completion #33

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

james-cnz
Copy link
Contributor

TODO:

  • Code checker updates are needed to pass tests (see Code checker updates: PHP 8.2, Moodle 4.3, arrays #32).
  • I think there's problems with delete_data_for_user() and delete_data_for_users() (see delete_data_for_users fix #31), and these functions will need to be tested after being changed. I've included a test.php file for this.
  • The test.php file needs to be deleted when no longer needed.
  • There are several places (marked TODO: Set actual version number) where I've included a placeholder version number, which need to be updated to the actual version number.
  • If/when course reset is implemented, it may be necessary to prevent comment created/deleted observers from triggering, to avoid unnecessary and excessive updates of completion state (dunno). The Moodle Assignment, Lesson, and Quiz classes prevent triggering group deletion observers during course reset.

@james-cnz
Copy link
Contributor Author

I think this is done, aside from the issues it depends on, and I'm not sure what the timeline is for those, so I thought I may as well post this up now.
I've added backup of comments in the change. I figure backup is supported by the plugin as it is, and activity completion support is being added, so, for completeness, backup of the activity completion state should be supported, and that requires backup of comments. This can be disabled, resulting in the original behaviour, by unticking the 'Include comments' option during backup or restore.

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.

1 participant