From 8e996b5113abad14026b1060d3cfbf4419c582f7 Mon Sep 17 00:00:00 2001
From: Tobias Kohr <tobias.kohr@camptocamp.com>
Date: Wed, 18 Dec 2024 10:03:20 +0100
Subject: [PATCH] fix(download-item): add filename with extension to download
 attribute

for json and geojson files. other file formats should have a name and don't need download attribute. note that download attribute only takes effect on same-origin
---
 apps/datahub-e2e/src/e2e/dataset.cy.ts        | 23 +++++++++++++++----
 .../download-item.component.html              |  2 +-
 .../download-item/download-item.component.ts  | 11 +++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/apps/datahub-e2e/src/e2e/dataset.cy.ts b/apps/datahub-e2e/src/e2e/dataset.cy.ts
index c97aa55b..c79bdaca 100644
--- a/apps/datahub-e2e/src/e2e/dataset.cy.ts
+++ b/apps/datahub-e2e/src/e2e/dataset.cy.ts
@@ -245,11 +245,11 @@ describe('datasets', () => {
     beforeEach(() => {
       cy.visit('/dataset/04bcec79-5b25-4b16-b635-73115f7456e4')
     })
-    it('should download button should contain the correct link and open in new tab', () => {
+    it('should contain the correct link in download button', () => {
       cy.get('[data-cy="download-button"]')
-        .first()
-        .invoke('attr', 'href')
-        .as('downloadLink')
+      .first()
+      .invoke('attr', 'href')
+      .as('downloadLink')
       cy.get('@downloadLink').should(
         'contain',
         '/geoserver/insee/ows?SERVICE=WFS&REQUEST=GetFeature&VERSION=2.0.0&TYPENAMES=insee%3Arectangles_200m_menage_erbm&OUTPUTFORMAT=csv'
@@ -258,6 +258,21 @@ describe('datasets', () => {
         .first()
         .should('have.attr', 'target', '_blank')
     })
+    it('should contain empty download attribute for other files than json and geojson', () => {
+      cy.get('[data-cy="download-button"]')
+        .first()
+        .should('have.attr', 'download', '')
+    })
+    it('should contain download attribute with filename data.json for json files', () => {
+      cy.get('[data-cy="download-button"]')
+        .eq(2)
+        .should('have.attr', 'download', 'data.json')
+    })
+    it('should open link in new tab as fallback (if download attribute is ignored, for not same-origin)', () => {
+      cy.get('[data-cy="download-button"]')
+        .first()
+        .should('have.attr', 'target', '_blank')
+    })
     it('should copy the link resource to clipboard when clicking on copy button', () => {
       cy.get('body').focus()
       cy.get('[data-cy="copy-button"]').first().click()
diff --git a/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.html b/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.html
index 136a1c0b..abb9a52c 100644
--- a/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.html
+++ b/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.html
@@ -28,7 +28,7 @@
       rel="noopener"
       [class]="'mel-primary-button font-medium text-nowrap'"
       data-cy="download-button"
-      download
+      [download]="downloadFileName"
       ><span translate>mel.dataset.download</span>
       <img src="assets/icons/download.svg" alt="download" />
     </a>
diff --git a/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.ts b/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.ts
index 75ccaae6..2d84aa61 100644
--- a/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.ts
+++ b/apps/datahub/src/app/dataset/dataset-downloads/download-item/download-item.component.ts
@@ -12,4 +12,15 @@ export class MelDownloadItemComponent extends DownloadItemComponent {
     navigator.clipboard.writeText(this.link.url.href)
     ;(event.target as HTMLElement).blur()
   }
+
+  // note that the download attribute calling this getter only takes effect on same-origin resources
+  get downloadFileName() {
+    let fileName = ''
+    if (this.format === 'geojson') {
+      fileName = 'data.geojson'
+    } else if (this.format === 'json') {
+      fileName = 'data.json'
+    } 
+    return fileName
+  }
 }