mdjnelson / moodle-mod_customcert

Enables the creation of dynamically generated certificates with complete customisation via the web browser.
https://moodle.org/plugins/mod_customcert
GNU General Public License v3.0
91 stars 158 forks source link

PDF certificate name should be student dependent #455

Open ricardoh34 opened 3 years ago

ricardoh34 commented 3 years ago

I think that the name of the PDF file generated for each student should be different if the certificate name uses a variable like "student name" in it. Maybe it could also be an option (checkbox)? Anyone is working on this? Thanks!

ricardoh34 commented 3 years ago

Maybe we can change in template.php, when the PDF is generated, and add the name of the student/user?

mdjnelson commented 3 years ago

Closing as a copy of #132. Please leave any comments there.

mdjnelson commented 3 years ago

Unfortunately there is currently no one else except myself working on this module and I do it in my free time. If you want you could contact https://catalyst-au.net/contact-us to get a quote on the work required, otherwise you will have to wait for me to implement the feature. Sorry.

mdjnelson commented 3 years ago

Reopening as this could be a quick solution to the certificate name confusion.

mdjnelson commented 3 years ago

Solution from @ricardoh34.

<             $filename = clean_filename($filename . '.pdf');
---
>             $filename = clean_filename($filename . '_' . fullname($USER) . '.pdf');

@ricardoh34 Do you want to create a MR?

mdjnelson commented 3 years ago

I assume this won't break on some user names, has that been tested?

ricardoh34 commented 3 years ago

Hello @mdjnelson

No need to create a MR, I think the change is really small. I´m working on including the course name also, when I got it, I´ll tell you. I have tested some names, also null names should simply not appear. As it is included in clean_filename function I think it will also protect against strange name combinations. I will try strange names, with quotes and similar to assure it.

Thanks! Ricardo

ricardoh34 commented 3 years ago

I have tested with quotes simple, double and with < > in the full name of $USER and it seems like clean_filename erases strange symbols and allows characters and numbers.

mdjnelson commented 3 years ago

Hello @mdjnelson

No need to create a MR, I think the change is really small. I´m working on including the course name also, when I got it, I´ll tell you. I have tested some names, also null names should simply not appear. As it is included in clean_filename function I think it will also protect against strange name combinations. I will try strange names, with quotes and similar to assure it.

Thanks! Ricardo

I just wanted to credit you for the fix that's why I asked about the MR. :)

ricardoh34 commented 3 years ago

Thanks! I would love that! I have updated this change and included the name of the course after a sql query to customcert table. This afternoon I send you the patch to check if it is worth to use.

ricardoh34 commented 3 years ago

Hello @mdjnelson

I have added the name of the course, considering the template id is unique and univocally related to the courseid. This is the patch:

300a301,308
>           $sql = "SELECT course,fullname
>                   FROM {customcert} c, 
>                     {course} cr
>                   WHERE c.id = :templateid
>                 AND c.course = cr.id";
>           $course = $DB->get_record_sql($sql, array('templateid' => $this->id));
>           
>             $filename = clean_filename($filename .'_'. fullname($USER) .'_'. $course->fullname . '.pdf');
302d309
<             $filename = clean_filename($filename . '_' . fullname($USER) . '.pdf');

Hope it is useful, Ricardo

mdjnelson commented 3 years ago

Are you able to do this as a merge request @ricardoh34?

ricardoh34 commented 3 years ago

I think I have done it now, sorry, but I have to update myself to github behaviour!

Tell me if it is ok, Regards, Ricardo

Izguy commented 1 year ago

Hello, this does not apply to the pdf file name in the mail that will be send to other users. Any suggegtions?

bmoschr commented 4 months ago

The code for the single download is good. But mail feature is missing.