search

danielneis / moodle-availability_maxviews

An availability condition for Moodle that limits the number of views of an activity by users
GNU General Public License v3.0
5 stars 9 forks source link

plugin does not work for all modules. #2

Open danmarsden opened 9 years ago

danmarsden commented 9 years ago

Some modules work well but others don't.

For example - Lesson module doesn't appear to be supported at all.

I'm guessing the "allow_add()" function might allow you to flag it as being available for certain modules?

danielneis commented 8 years ago

Hello, @danmarsden

I've added some Behat Tests (https://github.com/danielneis/moodle-availability_maxviews/blob/c5eba7bbbca999988233910eaff7b8d19a70b3b3/tests/behat/availability_maxviews.feature) using the Page module. Should I add tests with all standard modules? Wouldn't it be an issue with the lesson module?

Kind regards, Daniel

danmarsden commented 8 years ago

adding tests for all standard modules is probably a great idea - does it still rely on legacy logs? - I think the issue I had with Lesson I posted in Issue #1

danielneis commented 8 years ago

Hello,

I've started adding some tests with other modules. It seems to be working with Page and URL, but not working with Lesson, Wiki and Label. Also, for some reason, tests with Assignments seems to be differente from another tests and I could not make it to find the "Add restriction" button as you can see on https://travis-ci.org/danielneis/moodle-availability_maxviews/jobs/105031797 , I would appreciate very much if you could help me with this.

Here is the link for the code that actually do the test for max views: https://github.com/danielneis/moodle-availability_maxviews/blob/master/classes/condition.php#L88-L103

Any suggestion are welcome =)

danielneis commented 8 years ago

Hello,

The fact that the Assignment test was not working was because I've forgot a "@javascript" annotation. The fact that the Lesson was not working was because I needs at least a page added to it for students to view it, if no page is created, an error is raised and the view event is not triggered. I also added a test for the Forum module and it also works.

So it is not working for label, what was expected because it is on the course page and the user does not access a view.php page that would trigger the event, but I would propose that label_get_coursemodule_info() function trigger the view event. It could be very useful for some things that should show only once or a few times.

It is also not working for the Wiki module, what was not expected, but looking at the code, there is different conditions to trigger a course_module_viewed event OR a page_viewed event. I would suggest at first that the course_module_viewed should be always triggered and sometimes an extra page_viewed would be nice, but we should talk to maintainers first.

I will keep adding more tests with other standard modules to see what happens.

About the legacy logs, I do not really know. Is it enabled by default on Moodle 3.0 ? Because if it is disabled, and tests are not failing, they are not needed anymore. Also, I do rely on core\log\sql_reader or sql_select_reader for backwards compatibility.

danielneis commented 8 years ago

Hello, @danmarsden

I've added the allow_add() function to the plugin in a way that it does not allow adding to book, label and wiki modules: https://github.com/danielneis/moodle-availability_maxviews/blob/master/classes/frontend.php#L48

Is this enough to allow the plugin to be added to the plugins repository?

Kind regards, Daniel

danmarsden commented 8 years ago

I don't think it needs to work for all modules - if there's a way to only show it to the modules that it does work in I think that should be enough! - go ahead and submit a new version to the plugins repo! - just be aware there's a long plugins review queue at the moment. Some of the items in the existing queue have been sitting for over 30 days.

danielneis commented 8 years ago

Ok, nice! I've uploaded a new version and requested pre-approval. Don't need to hurry =)

I will let this issue open until I manage to fix those things in Moodle and understand why book is not working yet =)

Kind regards, Daniel