Refactor code to reduce code duplication

[dmitriim]

While working on fixing issues #33 and #34 I found it's very difficult to apply a proper fix as there are many examples of duplicated code.

E.g. each report has it's own page that pretty much does exactly what all other report pages do, except initialising a different report class.

I think this all can be improved by:

• all report classes can be children of some abstract report class with a common logic in the base class
• all report pages can be combined on one page and report type should be passed as URL parameter
• all templates should be combined in one template
• all renders should be combined in one render

After that basic refactoring done we can apply a proper fix for #33 as one proposed in #35 sort of workaround that actually hides code issues.

Hope that all makes sense.

[marcusgreen]

I entirely agree with what you have said. However there is an alternative argument that this should really be a report using the 4.x Report writer approach with all the benefits that would bring. I am not in a position to significantly refactor the plugin at the moment as I suspect it would take 3 to 5 days in total with full testing to do it correctly. However I will look at your pulls in detail in the very near future.

[dmitriim]

Hi @marcusgreen !

Honestly I didn't expect you to work on refactoring. I have created this issues to be able to reference to it in case I would have time to do a refactoring as part of other fixes we have asked to do for the plugin. Indeed it's a big work that would need to be tested throughout.

Can you please provide a bit more information about should really be a report using the 4.x Report writer approach ? What do you mean here? Thanks!

[marcusgreen]

That sounds like an excellent plan, thank you for that detail. By a report using the 4.x approach I meant as in the api I use in this demo.
https://github.com/marcusgreen/moodle-local_reportdemo

[dmitriim]

Ah. Got it 4.x mean Moodle 4.x here. Haha :)
Basically you'd like to migrate to report builder API. Makes sense. 👍

[dmitriim]

I have created #37