From ad91f78fb9851159286ddf9a83106d412dd1aaf4 Mon Sep 17 00:00:00 2001
From: Samuel Allan <samuel@opencraft.com>
Date: Fri, 15 Aug 2025 15:32:50 +0930
Subject: [PATCH] feat: remove PDF download button and option

Most browser PDF viewers will display an option for downloading the PDF.
Because of this, the separate PDF download button is redundant.
Also the option to uncheck "PDF Download Allowed" is misleading,
because removing the button can't disallow downloading.

Private-ref: https://tasks.opencraft.com/browse/BB-9935
---
 pdf/pdf.py                       | 24 +-----------------------
 pdf/static/js/pdf_edit.js        |  1 -
 pdf/static/js/pdf_view.js        |  7 -------
 pdf/templates/html/pdf_edit.html | 11 -----------
 pdf/templates/html/pdf_view.html |  5 -----
 5 files changed, 1 insertion(+), 47 deletions(-)

diff --git a/pdf/pdf.py b/pdf/pdf.py
index 907a9b7..972475f 100644
--- a/pdf/pdf.py
+++ b/pdf/pdf.py
@@ -27,7 +27,7 @@ class PdfBlock(XBlock):
     Icon of the XBlock. Values : [other (default), video, problem]
     """
     icon_class = "other"
-    editable_fields = ('display_name', 'url', 'allow_download', 'source_text', 'source_url')
+    editable_fields = ('display_name', 'url', 'source_text', 'source_url')
 
     # Fields
     display_name = String(
@@ -44,13 +44,6 @@ class PdfBlock(XBlock):
         help=_("The URL for your PDF.")
     )
 
-    allow_download = Boolean(
-        display_name=_("PDF Download Allowed"),
-        default=True,
-        scope=Scope.content,
-        help=_("Display a download button for this PDF.")
-    )
-
     source_text = String(
         display_name=_("Source document button text"),
         default="",
@@ -97,7 +90,6 @@ def student_view(self, context=None):
         context = {
             'display_name': self.display_name,
             'url': self.url,
-            'allow_download': self.allow_download,
             'disable_all_download': is_all_download_disabled(),
             'source_text': self.source_text,
             'source_url': self.source_url,
@@ -129,7 +121,6 @@ def studio_view(self, context=None):
         context = {
             'display_name': self.display_name,
             'url': self.url,
-            'allow_download': self.allow_download,
             'disable_all_download': is_all_download_disabled(),
             'source_text': self.source_text,
             'source_url': self.source_url,
@@ -145,18 +136,6 @@ def studio_view(self, context=None):
         frag.initialize_js('pdfXBlockInitEdit')
         return frag
 
-    @XBlock.json_handler
-    def on_download(self, data, suffix=''):  # pylint: disable=unused-argument
-        """
-        The download file event handler
-        """
-        event_type = 'edx.pdf.downloaded'
-        event_data = {
-            'url': self.url,
-            'source_url': self.source_url,
-        }
-        self.runtime.publish(self, event_type, event_data)
-
     def _generate_pdf_from_source(self):
         """
         Uses the Gotenberg API to convert the source document to a PDF.
@@ -178,7 +157,6 @@ def save_pdf(self, data, suffix=''):  # pylint: disable=unused-argument
         self.url = data['url']
 
         if not is_all_download_disabled():
-            self.allow_download = bool_from_str(data['allow_download'])
             self.source_text = data['source_text']
             self.source_url = data['source_url']
             if data['source_url'] and bool_from_str(data['pdf_auto_generate']):
diff --git a/pdf/static/js/pdf_edit.js b/pdf/static/js/pdf_edit.js
index ecc5863..449764f 100644
--- a/pdf/static/js/pdf_edit.js
+++ b/pdf/static/js/pdf_edit.js
@@ -9,7 +9,6 @@ function pdfXBlockInitEdit(runtime, element) {
         const data = {
             'display_name': $('#pdf_edit_display_name').val(),
             'url': $('#pdf_edit_url').val(),
-            'allow_download': $('#pdf_edit_allow_download').val() || '',
             'source_text': $('#pdf_edit_source_text').val() || '',
             'source_url': $('#pdf_edit_source_url').val() || '',
             'pdf_auto_generate': $('#pdf_auto_generate').val() || '',
diff --git a/pdf/static/js/pdf_view.js b/pdf/static/js/pdf_view.js
index 6b527b0..015ebc9 100644
--- a/pdf/static/js/pdf_view.js
+++ b/pdf/static/js/pdf_view.js
@@ -8,11 +8,4 @@ function pdfXBlockInitView(runtime, element) {
     if (element.innerHTML) {
         element = $(element);
     }
-
-    $(function () {
-        element.find('.pdf-download-button').on('click', function () {
-            const handlerUrl = runtime.handlerUrl(element, 'on_download');
-            $.post(handlerUrl, '{}');
-        });
-    });
 }
diff --git a/pdf/templates/html/pdf_edit.html b/pdf/templates/html/pdf_edit.html
index 239e9a4..f9ca984 100644
--- a/pdf/templates/html/pdf_edit.html
+++ b/pdf/templates/html/pdf_edit.html
@@ -19,17 +19,6 @@
     </li>
 
     {% if not disable_all_download %}
-    <li class="field comp-setting-entry is-set">
-      <div class="wrapper-comp-setting">
-        <label class="label setting-label" for="pdf_edit_allow_download">{% trans "PDF Download Allowed" %}</label>
-        <select class="input setting-input" id="pdf_edit_allow_download">
-          <option value="True" {% if allow_download %}selected{% endif %}>{% trans "True" %}</option>
-          <option value="False" {% if not allow_download %}selected{% endif %}>{% trans "False" %}</option>
-        </select>
-      </div>
-      <span class="tip setting-help">{% trans "Display a download button for this PDF." %}</span>
-    </li>
-
     <li class="field comp-setting-entry is-set">
       <div class="wrapper-comp-setting">
         <label class="label setting-label" for="pdf_edit_source_text">{% trans "Source document button text" %}</label>
diff --git a/pdf/templates/html/pdf_view.html b/pdf/templates/html/pdf_view.html
index 8a51ff5..337a37f 100644
--- a/pdf/templates/html/pdf_view.html
+++ b/pdf/templates/html/pdf_view.html
@@ -8,11 +8,6 @@ <h2>{{ display_name }}</h2>
   </object>
   {% if not disable_all_download %}
   <ul>
-    {% if allow_download %}
-    <li class="pdf-download-button">
-      <a href="{{ url }}" download>{% trans "Download the PDF" %}</a>
-    </li>
-    {% endif %}
     {% if source_url != "" %}
     <li class="pdf-download-button">
       <a href="{{ source_url }}" download>{% if source_text == "" %}{% trans "Download the source document" %}{% else %}{{ source_text }}{% endif %}</a>