Skip to content

Commit

Permalink
UPLOAD-1695 Show upload status of file in info (#475)
Browse files Browse the repository at this point in the history
* updated the /info/{uploadId} endpoint to add upload_status to the response
	* upload_status contains 
		* status [Initiated, In Progress, Complete]
		* chunk_received_at
* updated the upload page to display the file detail data from the /info/{uploadId} when the upload status is In Progress or Complete
* added checks to determine the host (client uploading the file) and the peer (client viewing the file status)
* added ability for host to resume an upload
* added ability for host to pause and resume an upload
* added logic for handling different upload statuses for hosts and peers
	* if the upload status is Initiated, the upload form is displayed
	* if the host begins uploading a file, the upload form is hidden, the progress bar is shown, and the file details are shown
	* if the host refreshes while uploading, a resume upload form is shown and the file details are shown
	* if the host resumes an upload, the resume upload form is hidden, the progress bar is shown, and the file details are shown
	* if the peer views the page while the upload status is In Progress, only the file details are shown
	* if the upload status is complete, only the file details are shown
* added styling to the file details
* various clean ups for the html, css, and javascript
* playwright tests and accessibility changes

---------

Co-authored-by: Nicole Zonnenberg <[email protected]>
Co-authored-by: Alex de los Reyes <[email protected]>
  • Loading branch information
3 people authored Sep 17, 2024
1 parent a1a1a2f commit 78b0695
Show file tree
Hide file tree
Showing 31 changed files with 1,645 additions and 546 deletions.
411 changes: 222 additions & 189 deletions docs/openapi.yml

Large diffs are not rendered by default.

22 changes: 10 additions & 12 deletions tests/smoke/playwright/test/upload-test-accessibility.spec.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,30 @@
import { test, expect } from '@playwright/test';
import AxeBuilder from '@axe-core/playwright';


test.describe.configure({ mode: 'parallel' });
test.describe('Upload User Interface', () => {

const axeRuleTags = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa"];
const axeRuleTags = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa"];

test('Checks accessiblity for the upload landing page', async ({ page }, testInfo) => {
test.describe('Upload Landing Page', () => {
test('has accessible features when loaded', async ({ page }, testInfo) => {
await page.goto(`/`)
const results = await new AxeBuilder({ page })
.withTags(axeRuleTags)
.analyze();

expect(results.violations).toEqual([]);
});
});

test.describe('Upload Manifest Page', () => {
[
{ dataStream: "celr", route: "csv" },
{ dataStream: "celr", route: "hl7v2" },
{ dataStream: "covid", route: "all-monthly-vaccination-csv" },
{ dataStream: "covid", route: "bridge-vaccination-csv" },
{ dataStream: "daart", route: "hl7" },
{ dataStream: "dex", route: "hl7-hl7ingress" },
{ dataStream: "dextesting", route: "testevent1" },
{ dataStream: "ehdi", route: "csv" },
{ dataStream: "eicr", route: "fhir" },
{ dataStream: "h5", route: "influenza-vaccination-csv" },
{ dataStream: "influenza", route: "vaccination-csv" },
{ dataStream: "ndlp", route: "aplhistoricaldata" },
{ dataStream: "ndlp", route: "covidallmonthlyvaccination" },
{ dataStream: "ndlp", route: "covidbridgevaccination" },
{ dataStream: "ndlp", route: "influenzavaccination" },
Expand All @@ -45,14 +42,15 @@ test.describe('Upload User Interface', () => {
expect(results.violations).toEqual([]);
})
});
});

test(`Checks accessibliity for the upload page for the daart/hl7 manifest`, async ({ page }) => {
await page.goto(`/manifest?data_stream=daart&data_stream_route=hl7`);
test.describe('File Upload Page', () => {
test(`Checks accessibliity for the upload page for the dextesting/testevent1 manifest`, async ({ page }) => {
await page.goto(`/manifest?data_stream=dextesting&data_stream_route=testevent1`);
await page.getByLabel('Sender Id').fill('Sender123');
await page.getByLabel('Data Producer Id').fill('Producer123');
await page.getByLabel('Jurisdiction').fill('Jurisdiction123');
await page.getByLabel('Received Filename').fill('small-test-file');
await page.getByLabel('Original File Timestamp').fill('Timestamp123');
await page.getByRole('button', { name: /next/i }).click();
await expect(page).toHaveURL(/status/)
const results = await new AxeBuilder({ page })
Expand Down
8 changes: 4 additions & 4 deletions tests/smoke/playwright/test/upload-test-e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ test.describe("Upload End to End Tests", () => {
await page.goto(`/`);
await page.getByLabel('Data Stream', {exact: true}).fill(dataStream);
await page.getByLabel('Data Stream Route').fill(route);
await page.getByRole('button', {name: /submit/i }).click();
await page.getByRole('button', {name: /next/i }).click();

await page.getByLabel('Sender Id').fill('Sender123')
await page.getByLabel('Data Producer Id').fill('Producer123')
Expand All @@ -18,11 +18,11 @@ test.describe("Upload End to End Tests", () => {

const fileChooserPromise = page.waitForEvent('filechooser');

await page.locator('input[type="file"]').click();
await page.getByRole('button', {name: 'Browse Files'}).click();
const fileChooser = await fileChooserPromise;
await fileChooser.setFiles('../upload-files/10KB-test-file');
await fileChooser.setFiles('../upload-files/10KB-test-file');

await page.getByText('Download 10KB-test-file')
await expect(page.getByText('Upload Status: Complete')).toBeVisible();
})
})

122 changes: 116 additions & 6 deletions tests/smoke/playwright/test/upload-test-pages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,29 @@ import { expect, test } from '@playwright/test';

test.describe.configure({ mode: 'parallel' });

test.describe("Upload Landing Page", () => {
test("has the expected elements to start a file upload process", async ({page}) => {
await page.goto(`/`);
const nav = page.getByRole('navigation')
await expect(nav.getByRole("link").and(nav.getByText('Skip to main content Upload'))).toBeHidden()
await expect(nav.getByRole("link").and(nav.getByText('Upload'))).toBeVisible()
await expect(page.getByRole('heading', { level: 1 })).toHaveText('Welcome to DEX Upload')
await expect(page.getByRole('heading', { level: 2 })).toHaveText('Start the upload process by entering a data stream and route.')
await expect(page.getByRole("textbox", { name: "Data Stream", exact: true })).toBeVisible()
await expect(page.getByRole("textbox", { name: "Data Stream Route", exact: true })).toBeVisible()
await expect(page.getByRole("button", {name: "Next"})).toBeVisible()
})
});

[
{ dataStream: "celr", route: "csv" },
{ dataStream: "celr", route: "hl7v2" },
{ dataStream: "covid", route: "all-monthly-vaccination-csv" },
{ dataStream: "covid", route: "bridge-vaccination-csv" },
{ dataStream: "daart", route: "hl7" },
{ dataStream: "dex", route: "hl7-hl7ingress" },
{ dataStream: "dextesting", route: "testevent1" },
{ dataStream: "ehdi", route: "csv" },
{ dataStream: "eicr", route: "fhir" },
{ dataStream: "h5", route: "influenza-vaccination-csv" },
{ dataStream: "influenza", route: "vaccination-csv" },
{ dataStream: "ndlp", route: "aplhistoricaldata" },
{ dataStream: "ndlp", route: "covidallmonthlyvaccination" },
{ dataStream: "ndlp", route: "covidbridgevaccination" },
{ dataStream: "ndlp", route: "influenzavaccination" },
Expand All @@ -23,9 +34,12 @@ test.describe.configure({ mode: 'parallel' });
{ dataStream: "routine", route: "immunization-other" },
{ dataStream: "rsv", route: "prevention-csv" },
].forEach(({ dataStream, route }) => {
test.describe("Manifest UI endpoint", () => {
test(`Data stream: ${dataStream} / Route: ${route} displays the appropriate UI fields`, async ({ page }) => {
test.describe("Upload Manifest Page", () => {
test(`has the expected metadata elements for Data stream: ${dataStream} / Route: ${route}`, async ({ page }) => {
await page.goto(`/manifest?data_stream=${dataStream}&data_stream_route=${route}`);
const nav = page.locator('nav')
await expect(nav.getByRole("link").and(nav.getByText('Skip to main content'))).toBeHidden()
await expect(nav.getByRole("link").and(nav.getByText('Upload'))).toBeVisible()
const title = page.locator('h1');
await expect(title).toHaveText('Please fill in the sender manifest form for your file');
// TODO: Add more assertions on individual manifest page elements
Expand All @@ -34,3 +48,99 @@ test.describe.configure({ mode: 'parallel' });
})
})
});

test.describe("File Uploader Page", () => {
test("has the expected elements to prepare to upload a file", async ({ page, baseURL }) => {
const apiURL = baseURL.replace('8081', '8080')
const dataStream = 'dextesting';
const route = 'testevent1';

await page.goto(`/manifest?data_stream=${dataStream}&data_stream_route=${route}`);

await page.getByLabel('Sender Id').fill('Sender123')
await page.getByLabel('Data Producer Id').fill('Producer123')
await page.getByLabel('Jurisdiction').fill('Jurisdiction123')
await page.getByLabel('Received Filename').fill('small-test-file')
await page.getByRole('button', { name: /next/i }).click();

await expect(page.getByRole('heading', { level: 1, includeHidden: false }).nth(0)).toHaveText('File Uploader')
const uploadEndpoint = page.getByRole('textbox', { name: 'Upload Endpoint' });
// not the greatest way to interpret the endpoint value here, but this will have to work for now...
await expect(uploadEndpoint).toHaveValue(`${apiURL}/files/`)

const chunkSize = page.getByLabel('Chunk size (bytes)');
const chunkSizeLabel = page.locator('label', { hasText: 'Chunk size (bytes)' })
await expect(chunkSizeLabel).toContainText('Note: Chunksize should be set on the client for uploading files of large size (1GB or over).')
await expect(chunkSize).toHaveValue('40000000')

const parallelUploadRequests = page.getByRole('spinbutton', { name: 'Parallel upload requests' })
await expect(parallelUploadRequests).toHaveValue('1')

const browseFilesButton = page.getByRole('button', { name: 'Browse Files' })
await expect(browseFilesButton).toHaveAttribute("onclick", "files.click()")
})
})

test.describe("Upload Status Page", () => {

test("has the expected elements to display upload status", async ({ page, baseURL }) => {
const apiURL = baseURL.replace('8081', '8080')
const dataStream = 'dextesting';
const route = 'testevent1';
const expectedFileName = 'small-test-file'
const expectedSender = 'Sender123'
const expectedDataProducer = 'Producer123'
const expectedJurisdiction = 'Jurisdiction123'

await page.goto(`/manifest?data_stream=${dataStream}&data_stream_route=${route}`);

await page.getByLabel('Sender Id').fill(expectedSender)
await page.getByLabel('Data Producer Id').fill(expectedDataProducer)
await page.getByLabel('Jurisdiction').fill(expectedJurisdiction)
await page.getByLabel('Received Filename').fill(expectedFileName)
await page.getByRole('button', {name: /next/i }).click();

const fileChooserPromise = page.waitForEvent('filechooser');
const uploadId = page.url().split('/').slice(-1)[0]

const uploadHeadResponsePromise = page.waitForResponse(response =>
response.url() === `${apiURL}/files/${uploadId}` && response.status() === 200
&& response.request().method() === 'HEAD'
);

const uploadPatchResponsePromise = page.waitForResponse(response =>
response.url() === `${apiURL}/files/${uploadId}` && response.status() === 204
&& response.request().method() === 'PATCH'
);

await page.getByRole('button', {name: 'Browse Files'}).click();

const fileChooser = await fileChooserPromise;
await fileChooser.setFiles('../upload-files/10KB-test-file');

await expect((await uploadPatchResponsePromise).ok()).toBeTruthy()
await expect((await uploadHeadResponsePromise).ok()).toBeTruthy()

await page.reload();

const fileHeaderContainer= page.locator('.file-header-container')
await expect(fileHeaderContainer.getByRole('heading', { level: 1 }).nth(0)).toHaveText(expectedFileName)
await expect(fileHeaderContainer.getByRole('heading', { level: 1 }).nth(1)).toHaveText("Upload Status: Complete")
await expect(fileHeaderContainer).toContainText(`ID: ${uploadId}`)

const fileDeliveriesContainer = page.locator('.file-deliveries-container');
await expect(fileDeliveriesContainer.getByRole('heading', { level: 2 }).nth(0)).toHaveText('Delivery Status')
await expect(fileDeliveriesContainer.getByRole('heading', { level: 2 }).nth(1)).toHaveText('EDAV')
await expect(fileDeliveriesContainer.getByRole('heading', {level: 3})).toHaveText('Delivery Status: SUCCESS')
await expect(fileDeliveriesContainer).toContainText(`Location: uploads/edav/${uploadId}`)

const uploadDetailsContainer = page.locator('.file-details-container')
await expect(uploadDetailsContainer.getByRole('heading', { level: 2 })).toHaveText('Upload Details')
await expect(uploadDetailsContainer).toContainText(`File Size: 10 KB`)
await expect(uploadDetailsContainer).toContainText(`Sender ID: ${expectedSender}`)
await expect(uploadDetailsContainer).toContainText(`Producer ID: ${expectedDataProducer}`)
await expect(uploadDetailsContainer).toContainText(`Stream ID: ${dataStream}`)
await expect(uploadDetailsContainer).toContainText(`Stream Route: ${route}`)
await expect(uploadDetailsContainer).toContainText(`Jurisdiction: ${expectedJurisdiction}`)
})
})
2 changes: 1 addition & 1 deletion tests/smoke/playwright/test/upload-test.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ dotenv.config({ path: '../../.env' });


// Use test.describe to group your tests and use hooks like beforeAll
test.describe('File Upload and Trace Response Flow', () => {
test.describe.skip('File Upload and Trace Response Flow', () => {
let uploadId;
let accessToken: string;
let psApiUrl: string;
Expand Down
5 changes: 3 additions & 2 deletions upload-server/cmd/cli/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package cli
import (
"context"
"fmt"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/stores3"
"github.com/tus/tusd/v2/pkg/s3store"
"os"
"path/filepath"

"github.com/cdcgov/data-exchange-upload/upload-server/internal/stores3"
"github.com/tus/tusd/v2/pkg/s3store"

"github.com/cdcgov/data-exchange-upload/upload-server/internal/appconfig"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/handlertusd"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/health"
Expand Down
33 changes: 19 additions & 14 deletions upload-server/cmd/cli/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"

"github.com/cdcgov/data-exchange-upload/upload-server/internal/stores3"
"github.com/cdcgov/data-exchange-upload/upload-server/pkg/azureinspector"
"github.com/cdcgov/data-exchange-upload/upload-server/pkg/fileinspector"
"github.com/cdcgov/data-exchange-upload/upload-server/pkg/s3inspector"
"net/http"

"github.com/cdcgov/data-exchange-upload/upload-server/internal/appconfig"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/storeaz"
Expand All @@ -35,11 +36,6 @@ func (ih *InfoHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
return
}

response := &info.InfoResponse{
Manifest: fileInfo,
Deliveries: []info.FileDeliveryStatus{},
}

uploadedFileInfo, err := ih.inspector.InspectUploadedFile(r.Context(), id)
if err != nil {
// skip not found errors to handle deferred uploads.
Expand All @@ -49,17 +45,30 @@ func (ih *InfoHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
}
}

uploadStatus, err := ih.statusInspector.InspectFileUploadStatus(r.Context(), id)
if err != nil {
// skip not found errors to handle deferred uploads.
if !errors.Is(err, info.ErrNotFound) {
http.Error(rw, fmt.Sprintf("error getting file upload status. Manifest: %#v", fileInfo), getStatusFromError(err))
return
}
}

deliveries, err := ih.statusInspector.InspectFileDeliveryStatus(r.Context(), id)
if err != nil {
// skip not found errors to handle deferred uploads.
if !errors.Is(err, info.ErrNotFound) {
http.Error(rw, fmt.Sprintf("error getting file status. Manifest: %#v", fileInfo), getStatusFromError(err))
http.Error(rw, fmt.Sprintf("error getting file delivery status. Manifest: %#v", fileInfo), getStatusFromError(err))
return
}
}

response.FileInfo = uploadedFileInfo
response.Deliveries = deliveries
response := &info.InfoResponse{
Manifest: fileInfo,
FileInfo: uploadedFileInfo,
UploadStatus: uploadStatus,
Deliveries: deliveries,
}

rw.Header().Set("Content-Type", "application/json")
enc := json.NewEncoder(rw)
Expand Down Expand Up @@ -90,11 +99,7 @@ func createInspector(ctx context.Context, appConfig *appconfig.AppConfig) (Uploa
return nil, err
}

return &s3inspector.S3UploadInspector{
Client: s3Client,
BucketName: appConfig.S3Connection.BucketName,
TusPrefix: appConfig.TusUploadPrefix,
}, nil
return s3inspector.NewS3UploadInspector(s3Client, appConfig.S3Connection.BucketName, appConfig.TusUploadPrefix), nil
}
if appConfig.LocalFolderUploadsTus != "" {
return fileinspector.NewFileSystemUploadInspector(appConfig.LocalFolderUploadsTus, appConfig.TusUploadPrefix), nil
Expand Down
2 changes: 2 additions & 0 deletions upload-server/cmd/cli/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package cli

import (
"context"

"github.com/cdcgov/data-exchange-upload/upload-server/pkg/info"
)

type UploadStatusInspector interface {
InspectFileDeliveryStatus(ctx context.Context, id string) ([]info.FileDeliveryStatus, error)
InspectFileUploadStatus(ctx context.Context, id string) (info.FileUploadStatus, error)
}
1 change: 1 addition & 0 deletions upload-server/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/dustin/go-humanize v1.0.1 // indirect
github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
Expand Down
2 changes: 2 additions & 0 deletions upload-server/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/eventials/go-tus v0.0.0-20220610120217-05d0564bb571 h1:0i+Y7klNNqXwzAQ2qlIWeZyiMtDB/rf5fSaFzIW7lsk=
Expand Down
7 changes: 4 additions & 3 deletions upload-server/internal/delivery/deliver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ import (
"bytes"
"context"
"fmt"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/metadata"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/stores3"
metadataPkg "github.com/cdcgov/data-exchange-upload/upload-server/pkg/metadata"
"io"
"log/slog"
"os"
Expand All @@ -16,6 +13,10 @@ import (
"text/template"
"time"

"github.com/cdcgov/data-exchange-upload/upload-server/internal/metadata"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/stores3"
metadataPkg "github.com/cdcgov/data-exchange-upload/upload-server/pkg/metadata"

"github.com/cdcgov/data-exchange-upload/upload-server/internal/appconfig"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/health"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/storeaz"
Expand Down
7 changes: 4 additions & 3 deletions upload-server/internal/delivery/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ package delivery
import (
"context"
"fmt"
"io"
"log/slog"
"strings"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/feature/s3/manager"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/appconfig"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/models"
"github.com/cdcgov/data-exchange-upload/upload-server/internal/stores3"
"io"
"log/slog"
"strings"
)

func NewS3Destination(ctx context.Context, target string, conn *appconfig.S3StorageConfig) (*S3Destination, error) {
Expand Down
Loading

0 comments on commit 78b0695

Please sign in to comment.