search

bozoh / moodle-mod_simplecertificate

It's NOT RECOMMENDED to install version below 2.2.6 (MOODLE_31), due a security issues See more in README
17 stars 46 forks source link

{DATE} variable limit reached? #193

Closed Jarvil closed 6 years ago

Jarvil commented 6 years ago

Hello,

I have an course that has started in 2016 which student's have completed on that year and also in years 2017 and 2018. On that course is a Simple Certificate issued with settings:

When I view issued certificates it states "date received" above the PDF-file correctly from the year 2016 but when I open the certificate PDF-file it has the current date (at the moment 08.05.2018).

It seems that all certificates that has course completion date from the year 2016 do not show the correct date but the current date.

Certificates issued on years 2017 or 2018 shows the {DATE} correctly as the course completion date.

Is there a bug here? How long back in the past do your plugin look for the course completion date? I'm using mod_simplecertificate 2.2.7 (2017120400) and Moodle 3.3.4 @bozoh

anieminen commented 6 years ago

Hi,

I think the problem is here: https://github.com/bozoh/moodle-mod_simplecertificate/blob/master/locallib.php#L1580-L1584

I tested that if statement in a local script by setting a static user id and course id. The $date parameter is not set, although a completion record can be found by making the sql query directly to the database.

anieminen commented 6 years ago

I made a small fix & PR, that seems to fix the problem.

I'm not sure how the completion year affects to showing the correct date, as it states in the issue. But it might be that the problem occurs only when there's a lot of completion records in the database.

bozoh commented 6 years ago

this fix do the same thing of old code, i think, it's only split it in 2 "if" statements

bozoh commented 6 years ago

the plugin searches all completions data (by user id and course id), but only get the biggest (most recent) ...

anieminen commented 6 years ago

Yes it does the same thing. But as I said it might be related to size of the database, that the old code didn't always work (in my case there are over 30000 completion records). So I think it's better to get the completion record first to make sure it exists, before checking the timecompleted value from that record.