search

moodleou / moodle-report_customsql

A Moodle report plugin that lets you easily create simple reports that can be expressed as a single SQL query
48 stars 101 forks source link

Adding pull request with Web Services #149

Open ojnadjarm opened 7 months ago

ojnadjarm commented 7 months ago

Hi, I'm a developer from Moodle US we develop a bunch of WS for a client that uses this plugins, we discuss with the client and we want to deploy it to the comunity here it is the PR, I have test it on 3.5, 4.1 and I think 4.3 Check it out and let me know what you think, I have create a postman collection for it too. https://speeding-space-323438.postman.co/workspace/ab00b294-0dbe-4b0d-b378-af3dc4a0b4a0/documentation/6644297-e042bc92-b578-498f-8298-f1ee0edc8eb1

timhunt commented 7 months ago

In principle this is a good thing.

In practice, very weird for you do to all that work before talking to us about it at all.

To warn you, I am too busy to look at this for the next few weeks.

However, I can see at a glace you your patch makes irrelevant changes that are wrong. (Overridden methods should not have PHPdoc comments, despite the fase-positives from moodlechec https://moodledev.io/general/development/policies/codingstyle#functionsk). It is more likely that I make time to review your chages if you don't do things like that, so let me know when the patch is cleaned up.

dvdcastro commented 6 months ago

Hi @timhunt sorry we didn't communicate this before. We've been building these changes for a client. We will review that the build works OK and we have a better pull request for you to review. We are basically adding management web services and other things. We'll have to document these changes as well, so we'll let you know when we have all of those things.