Repository: mtlynch/picoshare

Issue:
Display download count on file index
It would be helpful to show the download count of each file on the index page:

![image](https://github.com/mtlynch/picoshare/assets/7783288/dca239c2-5fe3-4d8f-8525-39826c06ae7b)

To do it, we'd have to:

1. Add a `DownloadCount` field to [`UploadMetadata`](https://github.com/mtlynch/picoshare/blob/bdb82fc9b305d38d49fb907aa75554c777ca34c3/picoshare/picoshare.go#L18-L27)
2. Adjust [`GetEntriesMetadata` and `GetEntryMetadata`](https://github.com/mtlynch/picoshare/blob/bdb82fc9b305d38d49fb907aa75554c777ca34c3/store/sqlite/entries.go) to populate the `DownloadCount` field.
3. Adjust [the `file-index.html` template](https://github.com/mtlynch/picoshare/blob/bdb82fc9b305d38d49fb907aa75554c777ca34c3/handlers/templates/pages/file-index.html) to add a table column for download count.

Hints from issue comments:
@Karmalakas: @mtlynch, after update from v1.4.5 to v1.5.1 (on TrueNAS as custom app) I can no longer see download count and the list looks like in the screenshot on OG issue 🤔 Should I create a new issue or is there something I'm missing?

P.S. I still see the count if I open the info page for a file@mtlynch: @Karmalakas - Yeah, it was too expensive to calculate for everything at once, so they slowed page loads on low-resource systems, so I removed this functionality in https://github.com/mtlynch/picoshare/pull/683

The download count is still available in the info page for each file.@Karmalakas: I see 🙂

If you ever get to it and would think it would be worth your time, maybe it would be good to have this as an option in settings (to show or not) 🤔 I won't create a feature request and leave for you to decide. Not a necessity 🙂

Thanks for the explanation 🙏

Solution patch:
diff --git a/e2e/download-history.spec.ts b/e2e/download-history.spec.ts
index f04f51f..f3543cf 100644
--- a/e2e/download-history.spec.ts
+++ b/e2e/download-history.spec.ts
@@ -2,6 +2,7 @@ import { test, expect } from "@playwright/test";
 import { login } from "./helpers/login.js";
 
 const browserColumn = 3;
+const downloadCountColumn = 4;
 
 test("upload a file and verify it has no download history", async ({
   page,
@@ -22,6 +23,12 @@ test("upload a file and verify it has no download history", async ({
   await page.getByRole("menuitem", { name: "Files" }).click();
 
   await expect(page).toHaveURL(/\/files$/);
+  const matchingRow = await page
+    .getByRole("row")
+    .filter({ hasText: "simple-upload.txt" });
+  await expect(
+    matchingRow.getByRole("cell").nth(downloadCountColumn)
+  ).toHaveText("0 times");
   await page
     .getByRole("row")
     .filter({ hasText: "simple-upload.txt" })
@@ -69,6 +76,12 @@ test("upload a file, download it, and verify it has a download history", async (
   await page.getByRole("menuitem", { name: "Files" }).click();
 
   await expect(page).toHaveURL(/\/files$/);
+  const matchingRow = await page
+    .getByRole("row")
+    .filter({ hasText: "simple-upload.txt" });
+  await expect(
+    matchingRow.getByRole("cell").nth(downloadCountColumn)
+  ).toHaveText("1 times");
   await page
     .getByRole("row")
     .filter({ hasText: "simple-upload.txt" })
diff --git a/e2e/upload.spec.ts b/e2e/upload.spec.ts
index 21834d1..33a6e43 100644
--- a/e2e/upload.spec.ts
+++ b/e2e/upload.spec.ts
@@ -2,7 +2,7 @@ import { test, expect } from "@playwright/test";
 import { login } from "./helpers/login.js";
 
 const noteColumn = 1;
-const expirationColumn = 4;
+const expirationColumn = 5;
 
 test("uploads a file without specifying any parameters", async ({
   page,
diff --git a/handlers/templates/pages/file-index.html b/handlers/templates/pages/file-index.html
index 359bb9f..4b4c36f 100644
--- a/handlers/templates/pages/file-index.html
+++ b/handlers/templates/pages/file-index.html
@@ -77,6 +77,7 @@
           <th>Note</th>
           <th>Size</th>
           <th>Uploaded</th>
+          <th>Downloaded</th>
           <th>Expires</th>
           <th></th>
         </tr>
@@ -97,6 +98,7 @@
             </td>
             <td class="is-vcentered">{{ formatFileSize .Size }}</td>
             <td class="is-vcentered">{{ formatDate .Uploaded }}</td>
+            <td class="is-vcentered">{{ .DownloadCount }} times</td>
             <td class="is-vcentered">
               {{- formatExpiration .Expires -}}
             </td>
diff --git a/picoshare/picoshare.go b/picoshare/picoshare.go
index 6f2d2dc..be95add 100644
--- a/picoshare/picoshare.go
+++ b/picoshare/picoshare.go
@@ -16,14 +16,15 @@ type (
 	}
 
 	UploadMetadata struct {
-		ID          EntryID
-		Filename    Filename
-		Note        FileNote
-		ContentType ContentType
-		Uploaded    time.Time
-		Expires     ExpirationTime
-		Size        uint64
-		GuestLink   GuestLink
+		ID            EntryID
+		Filename      Filename
+		Note          FileNote
+		ContentType   ContentType
+		Uploaded      time.Time
+		Expires       ExpirationTime
+		Size          uint64
+		GuestLink     GuestLink
+		DownloadCount uint64
 	}
 
 	DownloadRecord struct {
diff --git a/store/sqlite/entries.go b/store/sqlite/entries.go
index 0b5f743..be41174 100644
--- a/store/sqlite/entries.go
+++ b/store/sqlite/entries.go
@@ -20,7 +20,8 @@ func (s Store) GetEntriesMetadata() ([]picoshare.UploadMetadata, error) {
 		entries.content_type AS content_type,
 		entries.upload_time AS upload_time,
 		entries.expiration_time AS expiration_time,
-		sizes.file_size AS file_size
+		sizes.file_size AS file_size,
+		IFNULL(downloads.download_count, 0) AS download_count
 	FROM
 		entries
 	INNER JOIN
@@ -32,7 +33,17 @@ func (s Store) GetEntriesMetadata() ([]picoshare.UploadMetadata, error) {
 				entries_data
 			GROUP BY
 				id
-		) sizes ON entries.id = sizes.id`)
+		) sizes ON entries.id = sizes.id
+	LEFT OUTER JOIN
+		(
+			SELECT
+				entry_id,
+				COUNT (entry_id) as download_count
+			FROM
+				downloads
+			GROUP BY
+				entry_id
+		) downloads ON entries.id = downloads.entry_id`)
 	if err != nil {
 		return []picoshare.UploadMetadata{}, err
 	}
@@ -46,7 +57,8 @@ func (s Store) GetEntriesMetadata() ([]picoshare.UploadMetadata, error) {
 		var uploadTimeRaw string
 		var expirationTimeRaw string
 		var fileSize uint64
-		if err = rows.Scan(&id, &filename, &note, &contentType, &uploadTimeRaw, &expirationTimeRaw, &fileSize); err != nil {
+		var downloadCount uint64
+		if err = rows.Scan(&id, &filename, &note, &contentType, &uploadTimeRaw, &expirationTimeRaw, &fileSize, &downloadCount); err != nil {
 			return []picoshare.UploadMetadata{}, err
 		}
 
@@ -61,13 +73,14 @@ func (s Store) GetEntriesMetadata() ([]picoshare.UploadMetadata, error) {
 		}
 
 		ee = append(ee, picoshare.UploadMetadata{
-			ID:          picoshare.EntryID(id),
-			Filename:    picoshare.Filename(filename),
-			Note:        picoshare.FileNote{Value: note},
-			ContentType: picoshare.ContentType(contentType),
-			Uploaded:    ut,
-			Expires:     picoshare.ExpirationTime(et),
-			Size:        fileSize,
+			ID:            picoshare.EntryID(id),
+			Filename:      picoshare.Filename(filename),
+			Note:          picoshare.FileNote{Value: note},
+			ContentType:   picoshare.ContentType(contentType),
+			Uploaded:      ut,
+			Expires:       picoshare.ExpirationTime(et),
+			Size:          fileSize,
+			DownloadCount: downloadCount,
 		})
 	}
 



Test patch:
diff --git a/store/sqlite/entries_test.go b/store/sqlite/entries_test.go
index 84e6a46..3445f41 100644
--- a/store/sqlite/entries_test.go
+++ b/store/sqlite/entries_test.go
@@ -51,6 +51,10 @@ func TestInsertDeleteSingleEntry(t *testing.T) {
 		t.Fatalf("unexpected file size in entry metadata: got %v, want %v", meta[0].Size, len(expected))
 	}
 
+	if meta[0].DownloadCount != 0 {
+		t.Fatalf("unexpected download count in entry metadata: got %v, want %v", meta[0].DownloadCount, 0)
+	}
+
 	expectedFilename := picoshare.Filename("dummy-file.txt")
 	if meta[0].Filename != expectedFilename {
 		t.Fatalf("unexpected filename: got %v, want %v", meta[0].Filename, expectedFilename)