Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PDF certificate name should be student dependent #455

Open
ricardoh34 opened this issue Aug 15, 2021 · 15 comments
Open

PDF certificate name should be student dependent #455

ricardoh34 opened this issue Aug 15, 2021 · 15 comments

Comments

@ricardoh34
Copy link

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
Copy link
Author

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

@mdjnelson
Copy link
Owner

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

@mdjnelson
Copy link
Owner

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 mdjnelson reopened this Aug 25, 2021
@mdjnelson
Copy link
Owner

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

@mdjnelson
Copy link
Owner

Solution from @ricardoh34.

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

@ricardoh34 Do you want to create a MR?

@mdjnelson
Copy link
Owner

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

@ricardoh34
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

ricardoh34 commented Sep 3, 2021

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
Copy link
Owner

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

ricardoh34 added a commit to ricardoh34/moodle-mod_customcert that referenced this issue Sep 3, 2021
Include name of student and name of the course in the PDF certificate so as to ease organization of the certificates downloaded, solving issue mdjnelson#455
@ricardoh34
Copy link
Author

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

@mdjnelson mdjnelson linked a pull request Oct 27, 2021 that will close this issue
@Izguy
Copy link

Izguy commented Apr 18, 2023

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

@bmoschr
Copy link

bmoschr commented Apr 18, 2024

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

@mdjnelson mdjnelson reopened this Aug 18, 2024
ricardoh34 added a commit to ricardoh34/moodle-mod_customcert that referenced this issue Aug 26, 2024
Include name of student and name of the course in the PDF certificate so as to ease organization of the certificates downloaded, solving issue mdjnelson#455

Erase tabs

Added new space in line 266

Added original space in line 266.

Corrected mistakes from @mdjnelson comments

@mdjnelson ammendments and corrections. I have changed full_name to fullname again. I expect everything works now.

Add student name to PDF certificate file

Add student name to PDF certificate file

Update template.php

Changed tabs and try to distinguish fullname and full_name. If still persists the error maybe we can´t use fullname($USER)? I have added also defined('moodle_internal') || die(); if that´s the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants