Feature/keeplocalcopy #584

HobieCat · 2023-10-13T10:51:25Z

Hello there,
I've added the feature to save generated PDF to the local filesystem, and to serve them when downloading.
It came from a customer request to have a (sort of) archived PDF certificates at the time it was first viewed/generated.

Also, a 'download all certificates' link was added to the navigation menus of the course (to download a zip with all of the course certificates) and the activity itself (to download all the certificates of the single activity).

If of any interest to anyone, please merge at your convenience.

mdjnelson · 2024-01-31T16:01:30Z

Sorry I haven't been able to look at this until now. Can you rebase and then solve the issues reported by GHA? Thanks for your hard work.

HobieCat · 2024-02-03T12:23:17Z

Hi, looks like I've fixed the issues.

See MDL-83705 for more information.

…djnelson#693

mdjnelson · 2025-06-23T20:09:26Z

Sorry. I have not been paying attention to the PRs and issues for a while on this plugin unless they are major breakages to Moodle versions. I appreciate your effort on this PR. Can you squash and rebased on the latested MOODLE_404_STABLE branch?

HobieCat · 2025-06-24T14:36:07Z

Hello. Should have done! Not so skilled at squashing and rebasing, hope I didn't mess it all up.

mdjnelson · 2025-07-06T11:42:47Z

Taking a look now, thanks!

…mdjnelson#684) Also did some minor changes.

…jnelson#701)

)

mdjnelson · 2025-07-13T15:09:08Z

Just fixing conflicts and will browse code. I didn't get as much time as I wanted this weekend and I was also looking at other issues.

Just for reference this should solve #38 which would be a huge win.

- add keeplocalcopy field to the customcert table - add managekeeplocalcopy, deletelocalcopy capabilities - add keeplocalcopy to the backup structure - add keeplocalcopy setting element to forms - add localfile class to manage local PDF files - add logic to store and serve local PDF files - add deletelocalcopy to the actions column - add deletelocalcopy logic - add 'download all certificates' link to nav menus - add file to download all certificates - italian translation for new lang strings

mdjnelson · 2025-07-13T16:14:05Z

I cherry-picked this to my branch and fixed conflicts, see https://github.com/mdjnelson/moodle-mod_customcert/commits/develop/. Perhaps you would like to use this to save you troubles.

mdjnelson

I have commented on a few things but am confused about what the aim is here. I was thinking there it would be a certificate setting where we keep the PDFs on the server and just serve the already saved ones instead of generating when this setting is on. What exactly is this doing? Seems to be a few things.

mdjnelson · 2025-07-13T16:11:17Z

classes/localfile.php

+     *
+     * @var \mod_customcert\template
+     */
+    protected $template;


Please add the type here.

Didn't get this, how do you need the code to look like?

mdjnelson · 2025-07-13T16:11:39Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose certificate we want to save
+     * @return stored_file|false the stored_file object on success, false on error
+     */
+    public function savePDF(string $pdfcontent, ?int $userid = null) {


To keep consistent with Moodle function naming can you call this save_pdf().

mdjnelson · 2025-07-13T16:12:28Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose certificate we want to get
+     * @return \stored_file|false the stored_file object on success, false on error
+     */
+    public function getPDF(?int $userid = null) {


Again, please call this get_pdf().

mdjnelson · 2025-07-13T16:15:09Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose certificate we want to get
+     * @return bool true on success
+     */
+    public function deletePDF(?int $userid = null) {


Again, delete_pdf().

mdjnelson · 2025-07-13T16:15:27Z

classes/localfile.php

+            }
+            return false;
+        } catch (file_exception $e) {
+            // maybe log the exception


Remove comment please.

mdjnelson · 2025-07-13T16:16:42Z

view.php

-    // Trigger event.
-    $cm = get_coursemodule_from_instance('customcert', $customcert->id, 0, false, MUST_EXIST);
-    $context = \context_module::instance($cm->id);
-    $event = \mod_customcert\event\issue_deleted::create([


Why are we now removing this event?

mdjnelson · 2025-07-13T16:18:38Z

downloadcerts.php

+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Handles zip and download of certificates.


We already have the file https://github.com/mdjnelson/moodle-mod_customcert/blob/MOODLE_500_STABLE/download_all_certificates.php. Is this just a copy of that?

mdjnelson · 2025-07-13T16:18:52Z

classes/template.php

+    /**
+     * @var \mod_customcert\localfile the local file for the template.
+     */
+    protected $localfile;


Please add type.

Didn't get this, how do you need the code to look like?

mdjnelson · 2025-07-13T16:21:35Z

Also please address issues found in https://github.com/mdjnelson/moodle-mod_customcert/actions/runs/16251043172.

mdjnelson · 2025-07-13T16:22:25Z

classes/localfile.php

+     * @param integer|null $templateid the template id of the customcert we want to check
+     * @return \stored_file|false the stored_file object on success, false on error
+     */
+    public static function existsPDF($cm, ?int $userid = null, ?int $templateid = null) {


Please call does_pdf_exist().

mdjnelson · 2025-07-13T16:22:37Z

classes/localfile.php

+     * @return array the fileinfo array
+     */
+    protected function buildFileInfo(?int $userid = null) {
+


No space needed here.

mdjnelson · 2025-07-13T16:22:48Z

classes/localfile.php

+     * @return array the fileinfo array
+     */
+    private static function buildFileInfoArr ($cm, ?int $userid = null, ?int $templateid = null) {
+


No space needed here.

mdjnelson · 2025-07-13T16:23:21Z

classes/localfile.php

+     * @param integer|null $templateid the template id of the customcert of the array we want to generate
+     * @return array the fileinfo array
+     */
+    private static function buildFileInfoArr ($cm, ?int $userid = null, ?int $templateid = null) {


Could you not call this build_file_info and specify the return type as array?

mdjnelson · 2025-07-13T16:23:35Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose fileinfo array we want to generate
+     * @return array the fileinfo array
+     */
+    protected function buildFileInfo(?int $userid = null) {


Please specify return type.

mdjnelson · 2025-07-13T16:23:45Z

classes/localfile.php

+     * @param integer|null $templateid the template id of the customcert we want to check
+     * @return \stored_file|false the stored_file object on success, false on error
+     */
+    public static function existsPDF($cm, ?int $userid = null, ?int $templateid = null) {


Please specify return type.

mdjnelson · 2025-07-13T16:23:52Z

classes/localfile.php

+     * @param bool $return Do we want to return the contents of the PDF?
+     * @return string|void Can return the PDF in string format if specified.
+     */
+    public function sendPDF(?int $userid = NULL, string $deliveryoption = certificate::DELIVERY_OPTION_DOWNLOAD, bool $return = false) {


Please specify return type.

mdjnelson · 2025-07-13T16:23:59Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose certificate we want to get
+     * @return bool true on success
+     */
+    public function deletePDF(?int $userid = null) {


Please specify return type.

mdjnelson · 2025-07-13T16:24:05Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose certificate we want to get
+     * @return \stored_file|false the stored_file object on success, false on error
+     */
+    public function getPDF(?int $userid = null) {


Please specify return type.

mdjnelson · 2025-07-13T16:24:10Z

classes/localfile.php

+     * @param integer|null $userid the id of the user whose certificate we want to save
+     * @return stored_file|false the stored_file object on success, false on error
+     */
+    public function savePDF(string $pdfcontent, ?int $userid = null) {


Please specify return type.

mdjnelson · 2025-07-13T16:24:31Z

classes/localfile.php

+     * @param string $courseShortname
+     * @return string the PDF file name
+     */
+    public static function buildFileName($username, $templateid, $courseShortname) {


Please specify return type.

mdjnelson · 2025-07-13T16:25:23Z

lib.php

+ * @param \course_context $context
+ * @return \navigation_node|null null if there are no certifcates available for the download
+ */
+function build_downloadall_node(bool $isCourseNav, stdClass $course, \context_course $context = null) {


Again, this already exists in the certificate settings.

mdjnelson · 2025-07-13T16:28:44Z

What we want here is to have a certificate setting to save the PDFs to the server and serve those instead of generating them each time. The parts to do with bulk downloading have already been done in #205. If we can address the issues and just keep this as a simple "keep local copy" setting as described earlier that would be great. Thanks!

HobieCat · 2025-07-13T18:22:48Z

At the time I made this PR, there was no download_all_certificates.php, at least in the branch I was using at that time.
Ok, I can remove that code.
The "event removed" is my mistake. As said, I'm not so good at rebasing and squasing.
For other comments, I can try to go through each one and fix code as requested.

At this point, don't you think it would be better to make a new PR? From what branch should I start?

mdjnelson · 2025-07-13T18:28:30Z

Thanks @HobieCat. I apologise as the problem is on my end and I did not properly pay attention to this until now. Removing the code for downloading all certificates and addressing https://github.com/mdjnelson/moodle-mod_customcert/actions/runs/16251043172 as well as any points I made not covered by GHA will mean we are getting much closer. :)

mdjnelson · 2025-07-13T18:32:40Z

You can stay on the same branch and run git fetch followed by git reset --hard <marksremote>/develop - just note this will overwrite local changes. That gives you a clean history to work from, with your work rebased on top of the latest MOODLE_500_STABLE. You’ll need to force push afterward.

If you want to completely reset and discard your branch’s changes, use git reset --hard <marksremote>/MOODLE_500_STABLE instead.

HobieCat · 2025-07-20T20:21:39Z

Hi,
should have done as per your suggestion with a hard reset and forced push, thanks.
Afterwards I commited the required changes, but unexpected conflicts and changed files are now showing up.
If this is ok at your side, I'm ok as well. Please let me know

Thaveesha222 and others added 7 commits June 7, 2025 18:05

Updated subplugins.json file to match latest format (mdjnelson#688)

a1dc566

See MDL-83705 for more information.

Updated GHA for Moodle 5.0

2cdc66c

Updated CHANGES.md

b41d838

Fix code style complaints

13565ec

Fixed null array offset warning in QR code element on PHP 8.4 in GHA

1479075

Bumped version

8484023

Fix notifications when the instance of the activity is on site home m…

c7f98e3

…djnelson#693

HobieCat force-pushed the feature/keeplocalcopy branch from 53e7779 to 60daa6d Compare June 24, 2025 14:33

HobieCat changed the base branch from MOODLE_402_STABLE to MOODLE_404_STABLE June 24, 2025 14:34

Raza403 and others added 14 commits July 6, 2025 19:14

Added customisable filename options for certificates (mdjnelson#684)

3fe710b

Fixed version bump issue (mdjnelson#684)

00c6da5

Added newly introduced fields to install.xml (mdjnelson#684)

5f982ff

Make language string consistent with others

cf542a6

Removed unused language strings (mdjnelson#684)

733428c

Removed unnecessary code for filling in form inputs (mdjnelson#684)

349a4b4

Fixed issue with language strings not matching logic for placeholders (…

06af2ac

…mdjnelson#684) Also did some minor changes.

Added missing fields to backups (mdjnelson#705)

8305d26

Added fields to backup file (mdjnelson#684)

1472921

Removed hack for Oracle databases as they are no longer supported (md…

ac2fdda

…jnelson#701)

Add setting to add a button to return to course (mdjnelson#655)

1d74bcb

Make the setting description more descriptive (mdjnelson#655)

df719c4

Removed incorrect passing of boolean value no longer used (mdjnelson#655

9b462e5

)

Do not refer to the course page as a menu (mdjnelson#655)

14e91a8

mdjnelson reviewed Jul 13, 2025

View reviewed changes

HobieCat force-pushed the feature/keeplocalcopy branch from 60daa6d to 0637ca1 Compare July 20, 2025 20:13

rename methods, add return types and remove downloadcerts custom feature

ab5e388

Feature/keeplocalcopy #584

Are you sure you want to change the base?

Feature/keeplocalcopy #584

Uh oh!

Conversation

HobieCat commented Oct 13, 2023

Uh oh!

mdjnelson commented Jan 31, 2024

Uh oh!

HobieCat commented Feb 3, 2024

Uh oh!

mdjnelson commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HobieCat commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdjnelson commented Jul 6, 2025

Uh oh!

mdjnelson commented Jul 13, 2025

Uh oh!

mdjnelson commented Jul 13, 2025

Uh oh!

mdjnelson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdjnelson commented Jul 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdjnelson commented Jul 13, 2025

Uh oh!

HobieCat commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdjnelson commented Jun 23, 2025 •

edited

Loading

HobieCat commented Jun 24, 2025 •

edited

Loading

mdjnelson left a comment •

edited

Loading

HobieCat commented Jul 13, 2025 •

edited

Loading

mdjnelson commented Jul 13, 2025 •

edited

Loading