From d23331d9b9e46f6862d0212ef1c0278001ac885f Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Fri, 30 Oct 2015 17:43:20 +0100 Subject: [PATCH] Remove ReactS3FineUploader's dependency on react-router's location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReactS3FineUploader used to check the current url’s query params to determine which method it should use to upload, but this decision means the component is tightly coupled with react-router and history.js. A major pain point is having to propagate the location prop all the way down to this component even when it’s not necessary. Now, ReactS3FineUploader’s parent elements can either parse the current query params themselves or, if they have a location from react-router, simply use the location. Added a few utils to help parse url params. --- .../further_details_fileuploader.js | 8 +- .../ascribe_forms/form_create_contract.js | 8 +- .../ascribe_forms/form_register_piece.js | 9 +- .../ascribe_forms/input_fineuploader.js | 10 +-- .../contract_settings_update_button.js | 8 +- .../file_drag_and_drop.js | 34 ++++---- .../file_drag_and_drop_dialog.js | 56 +++++++------ .../react_s3_fine_uploader.js | 74 +++++++---------- js/fetchers/edition_list_fetcher.js | 2 +- js/fetchers/piece_list_fetcher.js | 2 +- js/utils/fetch_api_utils.js | 57 +------------ js/utils/requests.js | 6 +- js/utils/url_utils.js | 83 +++++++++++++++++++ package.json | 3 + 14 files changed, 190 insertions(+), 170 deletions(-) create mode 100644 js/utils/url_utils.js diff --git a/js/components/ascribe_detail/further_details_fileuploader.js b/js/components/ascribe_detail/further_details_fileuploader.js index c5ef8a1c..33caf9b0 100644 --- a/js/components/ascribe_detail/further_details_fileuploader.js +++ b/js/components/ascribe_detail/further_details_fileuploader.js @@ -20,8 +20,7 @@ let FurtherDetailsFileuploader = React.createClass({ submitFile: React.PropTypes.func, isReadyForFormSubmission: React.PropTypes.func, editable: React.PropTypes.bool, - multiple: React.PropTypes.bool, - location: React.PropTypes.object + multiple: React.PropTypes.bool }, getDefaultProps() { @@ -89,11 +88,10 @@ let FurtherDetailsFileuploader = React.createClass({ }} areAssetsDownloadable={true} areAssetsEditable={this.props.editable} - multiple={this.props.multiple} - location={this.props.location}/> + multiple={this.props.multiple} /> ); } }); -export default FurtherDetailsFileuploader; \ No newline at end of file +export default FurtherDetailsFileuploader; diff --git a/js/components/ascribe_forms/form_create_contract.js b/js/components/ascribe_forms/form_create_contract.js index fe00cebc..aac4c5ea 100644 --- a/js/components/ascribe_forms/form_create_contract.js +++ b/js/components/ascribe_forms/form_create_contract.js @@ -28,8 +28,7 @@ let CreateContractForm = React.createClass({ fileClassToUpload: React.PropTypes.shape({ singular: React.PropTypes.string, plural: React.PropTypes.string - }), - location: React.PropTypes.object + }) }, getInitialState() { @@ -87,8 +86,7 @@ let CreateContractForm = React.createClass({ areAssetsEditable={true} setIsUploadReady={this.setIsUploadReady} isReadyForFormSubmission={formSubmissionValidation.atLeastOneUploadedFile} - fileClassToUpload={this.props.fileClassToUpload} - location={this.props.location}/> + fileClassToUpload={this.props.fileClassToUpload} /> + uploadMethod={this.props.location.query.method} /> + uploadMethod={this.props.uploadMethod} + fileClassToUpload={this.props.fileClassToUpload} /> ); } }); -export default InputFineUploader; \ No newline at end of file +export default InputFineUploader; diff --git a/js/components/ascribe_settings/contract_settings_update_button.js b/js/components/ascribe_settings/contract_settings_update_button.js index f3bab156..ffd5ef4b 100644 --- a/js/components/ascribe_settings/contract_settings_update_button.js +++ b/js/components/ascribe_settings/contract_settings_update_button.js @@ -20,8 +20,7 @@ import { getLangText } from '../../utils/lang_utils'; let ContractSettingsUpdateButton = React.createClass({ propTypes: { - contract: React.PropTypes.object, - location: React.PropTypes.object + contract: React.PropTypes.object }, submitFile(file) { @@ -90,10 +89,9 @@ let ContractSettingsUpdateButton = React.createClass({ plural: getLangText('UPDATE') }} isReadyForFormSubmission={formSubmissionValidation.atLeastOneUploadedFile} - submitFile={this.submitFile} - location={this.props.location}/> + submitFile={this.submitFile} /> ); } }); -export default ContractSettingsUpdateButton; \ No newline at end of file +export default ContractSettingsUpdateButton; diff --git a/js/components/ascribe_uploader/ascribe_file_drag_and_drop/file_drag_and_drop.js b/js/components/ascribe_uploader/ascribe_file_drag_and_drop/file_drag_and_drop.js index 38ec459a..430dcab3 100644 --- a/js/components/ascribe_uploader/ascribe_file_drag_and_drop/file_drag_and_drop.js +++ b/js/components/ascribe_uploader/ascribe_file_drag_and_drop/file_drag_and_drop.js @@ -27,6 +27,7 @@ let FileDragAndDrop = React.createClass({ areAssetsEditable: React.PropTypes.bool, enableLocalHashing: React.PropTypes.bool, + uploadMethod: React.PropTypes.string, // triggers a FileDragAndDrop-global spinner hashingProgress: React.PropTypes.number, @@ -41,8 +42,7 @@ let FileDragAndDrop = React.createClass({ plural: React.PropTypes.string }), - allowedExtensions: React.PropTypes.string, - location: React.PropTypes.object + allowedExtensions: React.PropTypes.string }, handleDragOver(event) { @@ -137,19 +137,19 @@ let FileDragAndDrop = React.createClass({ }, render: function () { - let { filesToUpload, - dropzoneInactive, - className, - hashingProgress, - handleCancelHashing, - multiple, - enableLocalHashing, - fileClassToUpload, - areAssetsDownloadable, - areAssetsEditable, - allowedExtensions, - location - } = this.props; + const { + filesToUpload, + dropzoneInactive, + className, + hashingProgress, + handleCancelHashing, + multiple, + enableLocalHashing, + uploadMethod, + fileClassToUpload, + areAssetsDownloadable, + areAssetsEditable, + allowedExtensions } = this.props; // has files only is true if there are files that do not have the status deleted or canceled let hasFiles = filesToUpload.filter((file) => file.status !== 'deleted' && file.status !== 'canceled' && file.size !== -1).length > 0; @@ -185,8 +185,8 @@ let FileDragAndDrop = React.createClass({ hasFiles={hasFiles} onClick={this.handleOnClick} enableLocalHashing={enableLocalHashing} - fileClassToUpload={fileClassToUpload} - location={location}/> + uploadMethod={uploadMethod} + fileClassToUpload={fileClassToUpload} /> {getLangText('Drag %s here', fileClass)}

,

{getLangText('or')}

@@ -37,26 +35,31 @@ let FileDragAndDropDialog = React.createClass({ }, render() { - const queryParams = this.props.location.query; + const { + hasFiles, + multipleFiles, + enableLocalHashing, + uploadMethod, + fileClassToUpload, + onClick } = this.props; - if(this.props.hasFiles) { + if (hasFiles) { return null; } else { - if(this.props.enableLocalHashing && !queryParams.method) { + if (enableLocalHashing && !uploadMethod) { + const currentQueryParams = getCurrentQueryParams(); - let queryParamsHash = Object.assign({}, queryParams); + const queryParamsHash = Object.assign({}, currentQueryParams); queryParamsHash.method = 'hash'; - let queryParamsUpload = Object.assign({}, queryParams); + const queryParamsUpload = Object.assign({}, currentQueryParams); queryParamsUpload.method = 'upload'; - let { location } = this.props; - return (

{getLangText('Would you rather')}

{getLangText('Hash your work')} @@ -64,9 +67,9 @@ let FileDragAndDropDialog = React.createClass({ or - + {getLangText('Upload and hash your work')} @@ -75,26 +78,27 @@ let FileDragAndDropDialog = React.createClass({
); } else { - if(this.props.multipleFiles) { + if (multipleFiles) { return ( - {this.getDragDialog(this.props.fileClassToUpload.plural)} + {this.getDragDialog(fileClassToUpload.plural)} - {getLangText('choose %s to upload', this.props.fileClassToUpload.plural)} + onClick={onClick}> + {getLangText('choose %s to upload', fileClassToUpload.plural)} ); } else { - let dialog = queryParams.method === 'hash' ? getLangText('choose a %s to hash', this.props.fileClassToUpload.singular) : getLangText('choose a %s to upload', this.props.fileClassToUpload.singular); + const dialog = uploadMethod === 'hash' ? getLangText('choose a %s to hash', fileClassToUpload.singular) + : getLangText('choose a %s to upload', fileClassToUpload.singular); return ( - {this.getDragDialog(this.props.fileClassToUpload.singular)} + {this.getDragDialog(fileClassToUpload.singular)} + onClick={onClick}> {dialog} @@ -105,4 +109,4 @@ let FileDragAndDropDialog = React.createClass({ } }); -export default FileDragAndDropDialog; \ No newline at end of file +export default FileDragAndDropDialog; diff --git a/js/components/ascribe_uploader/react_s3_fine_uploader.js b/js/components/ascribe_uploader/react_s3_fine_uploader.js index e8cc8bfa..61dbcbcc 100644 --- a/js/components/ascribe_uploader/react_s3_fine_uploader.js +++ b/js/components/ascribe_uploader/react_s3_fine_uploader.js @@ -18,7 +18,6 @@ import { displayValidFilesFilter, transformAllowedExtensionsToInputAcceptProp } import { getCookie } from '../../utils/fetch_api_utils'; import { getLangText } from '../../utils/lang_utils'; - let ReactS3FineUploader = React.createClass({ propTypes: { keyRoutine: React.PropTypes.shape({ @@ -107,11 +106,14 @@ let ReactS3FineUploader = React.createClass({ // One solution we found in the process of tackling this problem was to hash // the file in the browser using md5 and then uploading the resulting text document instead // of the actual file. - // This boolean essentially enables that behavior + // + // This boolean and string essentially enable that behavior. + // Right now, we determine which upload method to use by appending a query parameter, + // which should be passed into 'uploadMethod': + // 'hash': upload using the hash + // 'upload': upload full file (default if not specified) enableLocalHashing: React.PropTypes.bool, - - // automatically injected by React-Router - query: React.PropTypes.object, + uploadMethod: React.PropTypes.string, // A class of a file the user has to upload // Needs to be defined both in singular as well as in plural @@ -126,9 +128,7 @@ let ReactS3FineUploader = React.createClass({ fileInputElement: React.PropTypes.oneOfType([ React.PropTypes.func, React.PropTypes.element - ]), - - location: React.PropTypes.object + ]) }, getDefaultProps() { @@ -192,11 +192,11 @@ let ReactS3FineUploader = React.createClass({ filesToUpload: [], uploader: new fineUploader.s3.FineUploaderBasic(this.propsToConfig()), csrfToken: getCookie(AppConstants.csrftoken), - + // -1: aborted // -2: uninitialized hashingProgress: -2, - + // this is for logging chunks: {} }; @@ -354,7 +354,6 @@ let ReactS3FineUploader = React.createClass({ /* FineUploader specific callback function handlers */ onUploadChunk(id, name, chunkData) { - let chunks = this.state.chunks; chunks[id + '-' + chunkData.startByte + '-' + chunkData.endByte] = { @@ -370,10 +369,9 @@ let ReactS3FineUploader = React.createClass({ }, onUploadChunkSuccess(id, chunkData, responseJson, xhr) { - let chunks = this.state.chunks; let chunkKey = id + '-' + chunkData.startByte + '-' + chunkData.endByte; - + if(chunks[chunkKey]) { chunks[chunkKey].completed = true; chunks[chunkKey].responseJson = responseJson; @@ -414,7 +412,7 @@ let ReactS3FineUploader = React.createClass({ } else { console.warn('You didn\'t define submitFile in as a prop in react-s3-fine-uploader'); } - + // for explanation, check comment of if statement above if(this.props.isReadyForFormSubmission && this.props.setIsUploadReady) { // also, lets check if after the completion of this upload, @@ -597,7 +595,6 @@ let ReactS3FineUploader = React.createClass({ } else { throw new Error(getLangText('File upload could not be paused.')); } - }, handleResumeFile(fileId) { @@ -647,16 +644,14 @@ let ReactS3FineUploader = React.createClass({ // md5 hash of a file locally and just upload a txt file containing that hash. // // In the view this only happens when the user is allowed to do local hashing as well - // as when the correct query parameter is present in the url ('hash' and not 'upload') - let queryParams = this.props.location.query; - if(this.props.enableLocalHashing && queryParams && queryParams.method === 'hash') { - - let convertedFilePromises = []; + // as when the correct method prop is present ('hash' and not 'upload') + if (this.props.enableLocalHashing && this.props.uploadMethod === 'hash') { + const convertedFilePromises = []; let overallFileSize = 0; + // "files" is not a classical Javascript array but a Javascript FileList, therefore // we can not use map to convert values for(let i = 0; i < files.length; i++) { - // for calculating the overall progress of all submitted files // we'll need to calculate the overall sum of all files' sizes overallFileSize += files[i].size; @@ -668,7 +663,6 @@ let ReactS3FineUploader = React.createClass({ // we're using promises to handle that let hashedFilePromise = computeHashOfFile(files[i]); convertedFilePromises.push(hashedFilePromise); - } // To react after the computation of all files, we define the resolvement @@ -676,7 +670,6 @@ let ReactS3FineUploader = React.createClass({ // with their txt representative Q.all(convertedFilePromises) .progress(({index, value: {progress, reject}}) => { - // hashing progress has been aborted from outside // To get out of the executing, we need to call reject from the // inside of the promise's execution. @@ -696,18 +689,14 @@ let ReactS3FineUploader = React.createClass({ // currently hashing files let overallHashingProgress = 0; for(let i = 0; i < files.length; i++) { - let filesSliceOfOverall = files[i].size / overallFileSize; overallHashingProgress += filesSliceOfOverall * files[i].progress; - } // Multiply by 100, since react-progressbar expects decimal numbers this.setState({ hashingProgress: overallHashingProgress * 100}); - }) .then((convertedFiles) => { - // clear hashing progress, since its done this.setState({ hashingProgress: -2}); @@ -823,20 +812,18 @@ let ReactS3FineUploader = React.createClass({ changeSet.status = { $set: status }; let filesToUpload = React.addons.update(this.state.filesToUpload, { [fileId]: changeSet }); - + this.setState({ filesToUpload }); }, isDropzoneInactive() { - let filesToDisplay = this.state.filesToUpload.filter((file) => file.status !== 'deleted' && file.status !== 'canceled' && file.size !== -1); - let queryParams = this.props.location.query; + const filesToDisplay = this.state.filesToUpload.filter((file) => file.status !== 'deleted' && file.status !== 'canceled' && file.size !== -1); - if((this.props.enableLocalHashing && !queryParams.method) || !this.props.areAssetsEditable || !this.props.multiple && filesToDisplay.length > 0) { + if ((this.props.enableLocalHashing && !this.props.uploadMethod) || !this.props.areAssetsEditable || !this.props.multiple && filesToDisplay.length > 0) { return true; } else { return false; } - }, getAllowedExtensions() { @@ -850,17 +837,16 @@ let ReactS3FineUploader = React.createClass({ }, render() { - let { - multiple, - areAssetsDownloadable, - areAssetsEditable, - onInactive, - enableLocalHashing, - fileClassToUpload, - validation, - fileInputElement, - location - } = this.props; + const { + multiple, + areAssetsDownloadable, + areAssetsEditable, + onInactive, + enableLocalHashing, + uploadMethod, + fileClassToUpload, + validation, + fileInputElement } = this.props; // Here we initialize the template that has been either provided from the outside // or the default input that is FileDragAndDrop. @@ -870,8 +856,8 @@ let ReactS3FineUploader = React.createClass({ areAssetsEditable, onInactive, enableLocalHashing, + uploadMethod, fileClassToUpload, - location, onDrop: this.handleUploadFile, filesToUpload: this.state.filesToUpload, handleDeleteFile: this.handleDeleteFile, diff --git a/js/fetchers/edition_list_fetcher.js b/js/fetchers/edition_list_fetcher.js index b416c595..93e4553d 100644 --- a/js/fetchers/edition_list_fetcher.js +++ b/js/fetchers/edition_list_fetcher.js @@ -2,8 +2,8 @@ import requests from '../utils/requests'; -import { generateOrderingQueryParams } from '../utils/fetch_api_utils'; import { mergeOptions } from '../utils/general_utils'; +import { generateOrderingQueryParams } from '../utils/url_utils'; let EditionListFetcher = { /** diff --git a/js/fetchers/piece_list_fetcher.js b/js/fetchers/piece_list_fetcher.js index 8e58402a..6bd4eb3a 100644 --- a/js/fetchers/piece_list_fetcher.js +++ b/js/fetchers/piece_list_fetcher.js @@ -3,7 +3,7 @@ import requests from '../utils/requests'; import { mergeOptions } from '../utils/general_utils'; -import { generateOrderingQueryParams } from '../utils/fetch_api_utils'; +import { generateOrderingQueryParams } from '../utils/url_utils'; let PieceListFetcher = { /** diff --git a/js/utils/fetch_api_utils.js b/js/utils/fetch_api_utils.js index 3ed964ba..cb676fce 100644 --- a/js/utils/fetch_api_utils.js +++ b/js/utils/fetch_api_utils.js @@ -2,63 +2,10 @@ import Q from 'q'; -import { sanitize } from './general_utils'; import AppConstants from '../constants/application_constants'; // TODO: Create Unittests that test all functions - /** - * Takes a key-value object of this form: - * - * { - * 'page': 1, - * 'pageSize': 10 - * } - * - * and converts it to a query-parameter, which you can append to your URL. - * The return looks like this: - * - * ?page=1&page_size=10 - * - * CamelCase gets converted to snake_case! - * - */ -export function argsToQueryParams(obj) { - - obj = sanitize(obj); - - return Object - .keys(obj) - .map((key, i) => { - let s = ''; - - if(i === 0) { - s += '?'; - } else { - s += '&'; - } - - let snakeCaseKey = key.replace(/[A-Z]/, (match) => '_' + match.toLowerCase()); - - return s + snakeCaseKey + '=' + encodeURIComponent(obj[key]); - }) - .join(''); -} - -/** - * Takes a string and a boolean and generates a string query parameter for - * an API call. - */ -export function generateOrderingQueryParams(orderBy, orderAsc) { - let interpolation = ''; - - if(!orderAsc) { - interpolation += '-'; - } - - return interpolation + orderBy; -} - export function status(response) { if (response.status >= 200 && response.status < 300) { return response; @@ -68,7 +15,7 @@ export function status(response) { export function getCookie(name) { let parts = document.cookie.split(';'); - + for(let i = 0; i < parts.length; i++) { if(parts[i].indexOf(AppConstants.csrftoken + '=') > -1) { return parts[i].split('=').pop(); @@ -111,4 +58,4 @@ export function fetchImageAsBlob(url) { xhr.send(); }); -} \ No newline at end of file +} diff --git a/js/utils/requests.js b/js/utils/requests.js index 7e9c9a58..bf203751 100644 --- a/js/utils/requests.js +++ b/js/utils/requests.js @@ -2,11 +2,11 @@ import Q from 'q'; -import { argsToQueryParams, getCookie } from '../utils/fetch_api_utils'; - import AppConstants from '../constants/application_constants'; -import {excludePropFromObject} from '../utils/general_utils'; +import { getCookie } from '../utils/fetch_api_utils'; +import { excludePropFromObject } from '../utils/general_utils'; +import { argsToQueryParams } from '../utils/url_utils'; class Requests { _merge(defaults, options) { diff --git a/js/utils/url_utils.js b/js/utils/url_utils.js new file mode 100644 index 00000000..cc875981 --- /dev/null +++ b/js/utils/url_utils.js @@ -0,0 +1,83 @@ +'use strict' + +import camelCase from 'camelcase'; +import snakeCase from 'snake-case'; +import qs from 'qs'; + +import { sanitize } from './general_utils'; + +// TODO: Create Unittests that test all functions + +/** + * Takes a key-value dictionary of this form: + * + * { + * 'page': 1, + * 'pageSize': 10 + * } + * + * and converts it to a query-parameter, which you can append to your URL. + * The return looks like this: + * + * ?page=1&page_size=10 + * + * CamelCase gets converted to snake_case! + * + * @param {object} obj Query params dictionary + * @return {string} Query params string + */ +export function argsToQueryParams(obj) { + const sanitizedObj = sanitize(obj); + const queryParamObj = {}; + + Object + .keys(sanitizedObj) + .forEach((key) => { + queryParamObj[snakeCase(key)] = sanitizedObj[key]; + }); + + // Use bracket arrayFormat as history.js and react-router use it + return '?' + qs.stringify(queryParamObj, { arrayFormat: 'brackets' }); +} + +/** + * Get the current url's query params as an key-val dictionary. + * snake_case gets converted to CamelCase! + * @return {object} Query params dictionary + */ +export function getCurrentQueryParams() { + return queryParamsToArgs(window.location.search.substring(1)); +} + +/** + * Convert the given query param string into a key-val dictionary. + * snake_case gets converted to CamelCase! + * @param {string} queryParamString Query params string + * @return {object} Query params dictionary + */ +export function queryParamsToArgs(queryParamString) { + const qsQueryParamObj = qs.parse(queryParamString); + const camelCaseParamObj = {}; + + Object + .keys(qsQueryParamObj) + .forEach((key) => { + camelCaseParamObj[camelCase(key)] = qsQueryParamObj[key]; + }); + + return camelCaseParamObj; +} + +/** + * Takes a string and a boolean and generates a string query parameter for + * an API call. + */ +export function generateOrderingQueryParams(orderBy, orderAsc) { + let interpolation = ''; + + if(!orderAsc) { + interpolation += '-'; + } + + return interpolation + orderBy; +} diff --git a/package.json b/package.json index 2b770fd3..4e7cd6a9 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "browser-sync": "^2.7.5", "browserify": "^9.0.8", "browserify-shim": "^3.8.10", + "camelcase": "^1.2.1", "classnames": "^1.2.2", "compression": "^1.4.4", "envify": "^3.4.0", @@ -73,6 +74,7 @@ "object-assign": "^2.0.0", "opn": "^3.0.2", "q": "^1.4.1", + "qs": "^5.2.0", "raven-js": "^1.1.19", "react": "0.13.2", "react-bootstrap": "0.25.1", @@ -83,6 +85,7 @@ "react-textarea-autosize": "^2.5.2", "reactify": "^1.1.0", "shmui": "^0.1.0", + "snake-case": "^1.1.1", "spark-md5": "~1.0.0", "uglifyjs": "^2.4.10", "vinyl-buffer": "^1.0.0",