Skip to content

Commit

Permalink
Display download count on file index (#563)
Browse files Browse the repository at this point in the history
Fixes #494
I didn't change GetEntryMetadata, even though it was required by the
description of the issue. Because download count for a single file is
already implemented and used via downloads, err :=
s.getDB(r).GetEntryDownloads(id), len(downloads). But I can change
GetEntryMetadata too if this approach (through one request to sql and
left join) looks better
  • Loading branch information
gross2001 authored Apr 5, 2024
1 parent 0f616c9 commit 01f0e73
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 19 deletions.
13 changes: 13 additions & 0 deletions e2e/download-history.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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" })
Expand Down Expand Up @@ -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" })
Expand Down
2 changes: 1 addition & 1 deletion e2e/upload.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions handlers/templates/pages/file-index.html
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ <h1 class="title">Files</h1>
<th>Note</th>
<th>Size</th>
<th>Uploaded</th>
<th>Downloaded</th>
<th>Expires</th>
<th></th>
</tr>
Expand All @@ -97,6 +98,7 @@ <h1 class="title">Files</h1>
</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>
Expand Down
17 changes: 9 additions & 8 deletions picoshare/picoshare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
33 changes: 23 additions & 10 deletions store/sqlite/entries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
}

Expand All @@ -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,
})
}

Expand Down
4 changes: 4 additions & 0 deletions store/sqlite/entries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 01f0e73

Please sign in to comment.