From 9be39e7339766d206fdebe2569e8ae68d0d1bb83 Mon Sep 17 00:00:00 2001 From: Steven Yuan Date: Thu, 7 Sep 2017 22:45:50 -0700 Subject: [PATCH] Add unauthorized download state When a user downloads a file to a location that they do not have access to, it currently just says "Downloading... 0%". This is to add a download state to indicate that the user doesn't have access to the director they are trying to download into. Commit list: - Add new download state - Update state if directory is unauthorized - Update electron download state change - Add 'Unauthorized' download state to 'clear completed downloads' reducer - Add isUnauthorized getter - Add unauthorized state as part of condition for hiding progress display - Add isUnauthorized to condition for rendering a download item - Set unauthorized as a stop condition for downloads - Set getL10nId for unauthorized downloads - Add 'unauthorized' as a style class for finished download items - Add downloadUnauthorized locale id for all languages - Update tests - Fix styling Fixes #7747 --- app/browser/electronDownloadItem.js | 2 +- app/browser/reducers/downloadsReducer.js | 2 +- app/extensions/brave/locales/en-US/downloads.properties | 3 ++- app/filtering.js | 9 +++++++++ app/renderer/components/download/downloadItem.js | 8 ++++++-- js/constants/downloadStates.js | 3 ++- js/state/downloadUtil.js | 4 +++- less/downloadBar.less | 2 +- .../app/renderer/components/download/downloadItemTest.js | 8 ++++---- test/unit/app/sessionStoreTest.js | 4 ++++ 10 files changed, 33 insertions(+), 12 deletions(-) diff --git a/app/browser/electronDownloadItem.js b/app/browser/electronDownloadItem.js index 88aaf6e6ab3..445bf73dec9 100644 --- a/app/browser/electronDownloadItem.js +++ b/app/browser/electronDownloadItem.js @@ -28,7 +28,7 @@ const progressDownloadItems = () => { } module.exports.updateElectronDownloadItem = (win, downloadId, item, state) => { - if (state === downloadStates.INTERRUPTED || state === downloadStates.CANCELLED || state === downloadStates.COMPLETED) { + if (state === downloadStates.INTERRUPTED || state === downloadStates.CANCELLED || state === downloadStates.UNAUTHORIZED || state === downloadStates.COMPLETED) { if (app.dock && state === downloadStates.COMPLETED) { app.dock.downloadFinished(item.getSavePath()) } diff --git a/app/browser/reducers/downloadsReducer.js b/app/browser/reducers/downloadsReducer.js index 49cf11bc295..246663f1b41 100644 --- a/app/browser/reducers/downloadsReducer.js +++ b/app/browser/reducers/downloadsReducer.js @@ -88,7 +88,7 @@ const downloadsReducer = (state, action) => { if (state.get('downloads')) { const downloads = state.get('downloads') .filter((download) => - ![downloadStates.COMPLETED, downloadStates.INTERRUPTED, downloadStates.CANCELLED].includes(download.get('state'))) + ![downloadStates.COMPLETED, downloadStates.INTERRUPTED, downloadStates.UNAUTHORIZED, downloadStates.CANCELLED].includes(download.get('state'))) state = state.set('downloads', downloads) } break diff --git a/app/extensions/brave/locales/en-US/downloads.properties b/app/extensions/brave/locales/en-US/downloads.properties index 7a0ece481d0..1c69d56e8dd 100644 --- a/app/extensions/brave/locales/en-US/downloads.properties +++ b/app/extensions/brave/locales/en-US/downloads.properties @@ -7,6 +7,7 @@ downloadDeleteConfirmation=Delete? downloadInProgress=Downloading: {{downloadPercent}} downloadInProgressUnknownTotal=Downloading… downloadInterrupted=Interrupted +downloadUnauthorized=Failed - Unauthorized access to download location downloadLocalFile=Local file downloadOpenPath.title=Open Folder Path downloadPause.title=Pause Download @@ -16,4 +17,4 @@ downloadRemoveFromList.title=Remove Download From List downloadResume.title=Resume Download downloads=Downloads downloadViewAll=View All -noDownloads=There are no downloads \ No newline at end of file +noDownloads=There are no downloads diff --git a/app/filtering.js b/app/filtering.js index 4fd57ae5bb5..cb657db89b8 100644 --- a/app/filtering.js +++ b/app/filtering.js @@ -595,6 +595,15 @@ function registerForDownloadListener (session) { updateElectronDownloadItem(downloadId, item, downloadStates.CANCELLED) appActions.mergeDownloadDetail(downloadId) }) + + // Change state if download path is protected + const fs = require('fs') + fs.access(path.dirname(item.getSavePath()), fs.constants.R_OK | fs.constants.W_OK, (err) => { + if (err) { + const state = downloadStates.UNAUTHORIZED + updateDownloadState(win, downloadId, item, state) + } + }) }) item.on('done', function (e, state) { diff --git a/app/renderer/components/download/downloadItem.js b/app/renderer/components/download/downloadItem.js index 32ad3673f0a..2cbbad08ab2 100644 --- a/app/renderer/components/download/downloadItem.js +++ b/app/renderer/components/download/downloadItem.js @@ -80,6 +80,10 @@ class DownloadItem extends React.Component { return this.props.downloadState === downloadStates.INTERRUPTED } + get isUnauthorized () { + return this.props.downloadState === downloadStates.UNAUTHORIZED + } + get isInProgress () { return this.props.downloadState === downloadStates.IN_PROGRESS } @@ -132,7 +136,7 @@ class DownloadItem extends React.Component { width: this.props.percentageComplete } - if (this.isCancelled || this.isInterrupted) { + if (this.isCancelled || this.isInterrupted || this.isUnauthorized) { progressStyle.display = 'none' } else if (this.props.isPendingState) { l10nStateArgs.downloadPercent = this.props.percentageComplete @@ -269,7 +273,7 @@ class DownloadItem extends React.Component { : null } { - this.isCancelled || this.isInterrupted || this.isCompleted || this.isPaused || this.isInProgress + this.isCancelled || this.isInterrupted || this.isUnauthorized || this.isCompleted || this.isPaused || this.isInProgress ?
: null } diff --git a/js/constants/downloadStates.js b/js/constants/downloadStates.js index 051fb27ab05..49421ee500e 100644 --- a/js/constants/downloadStates.js +++ b/js/constants/downloadStates.js @@ -12,7 +12,8 @@ const downloadStates = { PAUSED: _, COMPLETED: _, CANCELLED: _, - INTERRUPTED: _ + INTERRUPTED: _, + UNAUTHORIZED: _ } module.exports = mapValuesByKeys(downloadStates) diff --git a/js/state/downloadUtil.js b/js/state/downloadUtil.js index e31806aaa4c..ba65bb14d35 100644 --- a/js/state/downloadUtil.js +++ b/js/state/downloadUtil.js @@ -6,7 +6,7 @@ const downloadStates = require('../constants/downloadStates') const domUtil = require('../../app/renderer/lib/domUtil') const pendingStates = [downloadStates.IN_PROGRESS, downloadStates.PAUSED] -const stopStates = [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED] +const stopStates = [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.UNAUTHORIZED, downloadStates.COMPLETED] const notErrorStates = [downloadStates.IN_PROGRESS, downloadStates.PAUSED, downloadStates.COMPLETED] const downloadIsInState = (download, list) => @@ -42,6 +42,8 @@ const getL10nId = (download) => { return 'downloadInterrupted' case downloadStates.CANCELLED: return 'downloadCancelled' + case downloadStates.UNAUTHORIZED: + return 'downloadUnauthorized' case downloadStates.IN_PROGRESS: if (!download.get('totalBytes')) { return 'downloadInProgressUnknownTotal' diff --git a/less/downloadBar.less b/less/downloadBar.less index 65773a556dc..d62a3d87724 100644 --- a/less/downloadBar.less +++ b/less/downloadBar.less @@ -77,7 +77,7 @@ } } - &.completed, &.interrupted, &.cancelled { + &.completed, &.interrupted, &.cancelled, &.unauthorized { background-color: #e6e6e6; .downloadState { font-weight: bold; diff --git a/test/unit/app/renderer/components/download/downloadItemTest.js b/test/unit/app/renderer/components/download/downloadItemTest.js index 0424b87e33a..79ed12f5cf4 100644 --- a/test/unit/app/renderer/components/download/downloadItemTest.js +++ b/test/unit/app/renderer/components/download/downloadItemTest.js @@ -139,7 +139,7 @@ describe('downloadItem component', function () { appActions.downloadActionPerformed.restore() }) - testButton('[data-test-id="redownloadButton"]', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED], function (button) { + testButton('[data-test-id="redownloadButton"]', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.UNAUTHORIZED, downloadStates.COMPLETED], function (button) { const spy = sinon.spy(appActions, 'downloadRedownloaded') button.simulate('click') assert(spy.withArgs(downloadId).calledOnce) @@ -160,7 +160,7 @@ describe('downloadItem component', function () { appActions.downloadRevealed.restore() }) - testButton('[data-test-id="deleteButton"]', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED], function (button) { + testButton('[data-test-id="deleteButton"]', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.UNAUTHORIZED, downloadStates.COMPLETED], function (button) { const spy = sinon.spy(appActions, 'showDownloadDeleteConfirmation') try { // Confirmation should NOT be visible by default @@ -175,7 +175,7 @@ describe('downloadItem component', function () { }) }) - if ([downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED].includes(state)) { + if ([downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.UNAUTHORIZED, downloadStates.COMPLETED].includes(state)) { describe(`${state} download item when delete button has been clicked`, function () { before(function () { downloadId = uuid.v4() @@ -183,7 +183,7 @@ describe('downloadItem component', function () { result = mount() }) - testButton('[data-test-id="confirmDeleteButton"]', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED], function (button) { + testButton('[data-test-id="confirmDeleteButton"]', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.UNAUTHORIZED, downloadStates.COMPLETED], function (button) { const spy = sinon.spy(appActions, 'downloadDeleted') try { // Accepting confirmation should delete the item diff --git a/test/unit/app/sessionStoreTest.js b/test/unit/app/sessionStoreTest.js index 41e4e58a8bf..5533df99596 100644 --- a/test/unit/app/sessionStoreTest.js +++ b/test/unit/app/sessionStoreTest.js @@ -703,6 +703,10 @@ describe('sessionStore unit tests', function () { result = sessionStore.cleanAppData(data, false) assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.CANCELLED) + data = getEntry(downloadStates.UNAUTHORIZED) + result = sessionStore.cleanAppData(data, false) + assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.UNAUTHORIZED) + data = getEntry(downloadStates.PENDING) result = sessionStore.cleanAppData(data, false) assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.PENDING)