From 4668a42950e04c62bb2c2228396f5d5625d7b34f Mon Sep 17 00:00:00 2001 From: Cevo Date: Thu, 29 Oct 2015 15:44:05 +0100 Subject: [PATCH 01/11] font size increased to 13 --- sass/ascribe_acl_information.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sass/ascribe_acl_information.scss b/sass/ascribe_acl_information.scss index 9abec899..063c8ae6 100644 --- a/sass/ascribe_acl_information.scss +++ b/sass/ascribe_acl_information.scss @@ -8,7 +8,7 @@ } span { - font-size: 11px; + font-size: 13px; } .title { From d23331d9b9e46f6862d0212ef1c0278001ac885f Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Fri, 30 Oct 2015 17:43:20 +0100 Subject: [PATCH 02/11] 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", From 1e328b722b8009a6ec3e9c8a2f4e7998041d5ac4 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Fri, 30 Oct 2015 17:46:51 +0100 Subject: [PATCH 03/11] Sanitize utility should not modify given object Mutating arguments and then returning them is redundant and confusing behaviour (why pass it back if they already have it? Am I getting a new copy since it returns something?). --- js/utils/general_utils.js | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/js/utils/general_utils.js b/js/utils/general_utils.js index 7c13f9b5..cd73ba45 100644 --- a/js/utils/general_utils.js +++ b/js/utils/general_utils.js @@ -1,29 +1,23 @@ 'use strict'; +import _ from 'lodash'; + /** - * Takes an object and deletes all keys that are - * - * tagged as false by the passed in filter function + * Takes an object and returns a shallow copy without any keys + * that fail the passed in filter function. + * Does not modify the passed in object. * * @param {object} obj regular javascript object * @return {object} regular javascript object without null values or empty strings */ export function sanitize(obj, filterFn) { - if(!filterFn) { + if (!filterFn) { // By matching null with a double equal, we can match undefined and null // http://stackoverflow.com/a/15992131 filterFn = (val) => val == null || val === ''; } - Object - .keys(obj) - .map((key) => { - if(filterFn(obj[key])) { - delete obj[key]; - } - }); - - return obj; + return _.omit(obj, filterFn); } /** From a513af984d0d7e98d419b8ff376e3411d2d01360 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 10:41:59 +0100 Subject: [PATCH 04/11] Update cyland for FineUploader changes --- .../cyland/cyland_detail/cyland_piece_container.js | 4 +--- .../cyland/cyland_forms/cyland_additional_data_form.js | 8 +++----- .../wallet/components/cyland/cyland_register_piece.js | 3 +-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/js/components/whitelabel/wallet/components/cyland/cyland_detail/cyland_piece_container.js b/js/components/whitelabel/wallet/components/cyland/cyland_detail/cyland_piece_container.js index 79d63abf..7e784981 100644 --- a/js/components/whitelabel/wallet/components/cyland/cyland_detail/cyland_piece_container.js +++ b/js/components/whitelabel/wallet/components/cyland/cyland_detail/cyland_piece_container.js @@ -33,7 +33,6 @@ import { mergeOptions } from '../../../../../../utils/general_utils'; let CylandPieceContainer = React.createClass({ propTypes: { - location: React.PropTypes.object, params: React.PropTypes.object }, @@ -107,8 +106,7 @@ let CylandPieceContainer = React.createClass({ + isInline={true} /> ); diff --git a/js/components/whitelabel/wallet/components/cyland/cyland_forms/cyland_additional_data_form.js b/js/components/whitelabel/wallet/components/cyland/cyland_forms/cyland_additional_data_form.js index 63863b2d..0adfcb40 100644 --- a/js/components/whitelabel/wallet/components/cyland/cyland_forms/cyland_additional_data_form.js +++ b/js/components/whitelabel/wallet/components/cyland/cyland_forms/cyland_additional_data_form.js @@ -26,8 +26,7 @@ let CylandAdditionalDataForm = React.createClass({ handleSuccess: React.PropTypes.func, piece: React.PropTypes.object.isRequired, disabled: React.PropTypes.bool, - isInline: React.PropTypes.bool, - location: React.PropTypes.object + isInline: React.PropTypes.bool }, getDefaultProps() { @@ -143,8 +142,7 @@ let CylandAdditionalDataForm = React.createClass({ isReadyForFormSubmission={formSubmissionValidation.fileOptional} pieceId={piece.id} otherData={piece.other_data} - multiple={true} - location={this.props.location}/> + multiple={true} /> ); } else { @@ -157,4 +155,4 @@ let CylandAdditionalDataForm = React.createClass({ } }); -export default CylandAdditionalDataForm; \ No newline at end of file +export default CylandAdditionalDataForm; diff --git a/js/components/whitelabel/wallet/components/cyland/cyland_register_piece.js b/js/components/whitelabel/wallet/components/cyland/cyland_register_piece.js index ca755cf4..1903c7a2 100644 --- a/js/components/whitelabel/wallet/components/cyland/cyland_register_piece.js +++ b/js/components/whitelabel/wallet/components/cyland/cyland_register_piece.js @@ -210,8 +210,7 @@ let CylandRegisterPiece = React.createClass({ 1} handleSuccess={this.handleAdditionalDataSuccess} - piece={this.state.piece} - location={this.props.location}/> + piece={this.state.piece} /> From 5f5461c10ddd11362a79708913e3d937cdd74970 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 10:42:15 +0100 Subject: [PATCH 05/11] Remove warning for missing prop from FurtherDetailsFileUploader --- js/components/ascribe_detail/further_details_fileuploader.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/components/ascribe_detail/further_details_fileuploader.js b/js/components/ascribe_detail/further_details_fileuploader.js index 33caf9b0..9a1f091c 100644 --- a/js/components/ascribe_detail/further_details_fileuploader.js +++ b/js/components/ascribe_detail/further_details_fileuploader.js @@ -43,6 +43,7 @@ let FurtherDetailsFileuploader = React.createClass({ return ( Date: Mon, 2 Nov 2015 12:10:41 +0100 Subject: [PATCH 06/11] Check for a new csrf token on componentWillReceiveProps instead of componentWillUpdate this.setState() should not be used in componentWillUpdate(): https://facebook.github.io/react/docs/component-specs.html#updating-comp onentwillupdate --- js/components/ascribe_uploader/react_s3_fine_uploader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/components/ascribe_uploader/react_s3_fine_uploader.js b/js/components/ascribe_uploader/react_s3_fine_uploader.js index 61dbcbcc..685c2b2f 100644 --- a/js/components/ascribe_uploader/react_s3_fine_uploader.js +++ b/js/components/ascribe_uploader/react_s3_fine_uploader.js @@ -202,7 +202,7 @@ let ReactS3FineUploader = React.createClass({ }; }, - componentWillUpdate() { + componentWillReceiveProps() { // since the csrf header is defined in this component's props, // everytime the csrf cookie is changed we'll need to reinitalize // fineuploader and update the actual csrf token From fe4e33769073b3137ae6c7122b5e7559521c7807 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 18:27:45 +0100 Subject: [PATCH 07/11] Use Object.assign() instead of writing own merge function --- js/components/ascribe_forms/form.js | 8 ++--- js/utils/general_utils.js | 53 ++--------------------------- js/utils/requests.js | 27 +++++---------- 3 files changed, 14 insertions(+), 74 deletions(-) diff --git a/js/components/ascribe_forms/form.js b/js/components/ascribe_forms/form.js index fe15f537..deee14d9 100644 --- a/js/components/ascribe_forms/form.js +++ b/js/components/ascribe_forms/form.js @@ -12,8 +12,6 @@ import GlobalNotificationActions from '../../actions/global_notification_actions import requests from '../../utils/requests'; import { getLangText } from '../../utils/lang_utils'; -import { mergeOptionsWithDuplicates } from '../../utils/general_utils'; - let Form = React.createClass({ propTypes: { @@ -124,12 +122,12 @@ let Form = React.createClass({ getFormData() { let data = {}; - for(let ref in this.refs) { + for (let ref in this.refs) { data[this.refs[ref].props.name] = this.refs[ref].state.value; } - if(typeof this.props.getFormData === 'function') { - data = mergeOptionsWithDuplicates(data, this.props.getFormData()); + if (typeof this.props.getFormData === 'function') { + data = Object.assign(data, this.props.getFormData()); } return data; diff --git a/js/utils/general_utils.js b/js/utils/general_utils.js index cd73ba45..bce2dee6 100644 --- a/js/utils/general_utils.js +++ b/js/utils/general_utils.js @@ -76,8 +76,8 @@ export function formatText() { }); } -/* - Checks a list of objects for key duplicates and returns a boolean +/** + * Checks a list of objects for key duplicates and returns a boolean */ function _doesObjectListHaveDuplicates(l) { let mergedList = []; @@ -115,35 +115,7 @@ export function mergeOptions(...l) { throw new Error('The objects you submitted for merging have duplicates. Merge aborted.'); } - let newObj = {}; - - for(let i = 1; i < l.length; i++) { - newObj = _mergeOptions(newObj, _mergeOptions(l[i - 1], l[i])); - } - - return newObj; -} - -/** - * Merges a number of objects even if there're having duplicates. - * - * DOES NOT RETURN AN ERROR! - * - * Takes a list of object and merges their keys to one object. - * Uses mergeOptions for two objects. - * @param {[type]} l [description] - * @return {[type]} [description] - */ -export function mergeOptionsWithDuplicates(...l) { - // If the objects submitted in the list have duplicates,in their key names, - // abort the merge and tell the function's user to check his objects. - let newObj = {}; - - for(let i = 1; i < l.length; i++) { - newObj = _mergeOptions(newObj, _mergeOptions(l[i - 1], l[i])); - } - - return newObj; + return Object.assign({}, ...l); } /** @@ -159,25 +131,6 @@ export function update(a, ...l) { return a; } -/** - * Overwrites obj1's values with obj2's and adds obj2's if non existent in obj1 - * @param obj1 - * @param obj2 - * @returns obj3 a new object based on obj1 and obj2 - * Taken from: http://stackoverflow.com/a/171256/1263876 - */ -function _mergeOptions(obj1, obj2) { - let obj3 = {}; - - for (let attrname in obj1) { - obj3[attrname] = obj1[attrname]; - } - for (let attrname in obj2) { - obj3[attrname] = obj2[attrname]; - } - return obj3; -} - /** * Escape HTML in a string so it can be injected safely using * React's `dangerouslySetInnerHTML` diff --git a/js/utils/requests.js b/js/utils/requests.js index bf203751..ce24aaba 100644 --- a/js/utils/requests.js +++ b/js/utils/requests.js @@ -9,17 +9,6 @@ import { excludePropFromObject } from '../utils/general_utils'; import { argsToQueryParams } from '../utils/url_utils'; class Requests { - _merge(defaults, options) { - let merged = {}; - for (let key in defaults) { - merged[key] = defaults[key]; - } - for (let key in options) { - merged[key] = options[key]; - } - return merged; - } - unpackResponse(response) { if (response.status >= 500) { throw new Error(response.status + ' - ' + response.statusText + ' - on URL:' + response.url); @@ -112,7 +101,7 @@ class Requests { request(verb, url, options) { options = options || {}; - let merged = this._merge(this.httpOptions, options); + let merged = Object.assign({}, this.httpOptions, options); let csrftoken = getCookie(AppConstants.csrftoken); if (csrftoken) { merged.headers['X-CSRFToken'] = csrftoken; @@ -124,23 +113,23 @@ class Requests { } get(url, params) { - if (url === undefined){ + if (url === undefined) { throw new Error('Url undefined'); } - let paramsCopy = this._merge(params); + let paramsCopy = Object.assign({}, params); let newUrl = this.prepareUrl(url, paramsCopy, true); return this.request('get', newUrl); } delete(url, params) { - let paramsCopy = this._merge(params); + let paramsCopy = Object.assign({}, params); let newUrl = this.prepareUrl(url, paramsCopy, true); return this.request('delete', newUrl); } - _putOrPost(url, paramsAndBody, method){ - let paramsCopy = this._merge(paramsAndBody); let params = excludePropFromObject(paramsAndBody, ['body']); + _putOrPost(url, paramsAndBody, method) { + let paramsCopy = Object.assign({}, paramsAndBody); let newUrl = this.prepareUrl(url, params); let body = null; if (paramsCopy && paramsCopy.body) { @@ -153,11 +142,11 @@ class Requests { return this._putOrPost(url, params, 'post'); } - put(url, params){ + put(url, params) { return this._putOrPost(url, params, 'put'); } - patch(url, params){ + patch(url, params) { return this._putOrPost(url, params, 'patch'); } From 955e20d6b6643865f418f7ce3a586f6fb9cd3700 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 18:29:06 +0100 Subject: [PATCH 08/11] Reduce dependency footprint of new includes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opted for decamelize instead of snake-case as it’s much smaller and we don’t need the extra functionality of snake-case. --- js/utils/url_utils.js | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/js/utils/url_utils.js b/js/utils/url_utils.js index cc875981..86c8dfc5 100644 --- a/js/utils/url_utils.js +++ b/js/utils/url_utils.js @@ -1,7 +1,7 @@ 'use strict' import camelCase from 'camelcase'; -import snakeCase from 'snake-case'; +import decamelize from 'decamelize'; import qs from 'qs'; import { sanitize } from './general_utils'; @@ -33,7 +33,7 @@ export function argsToQueryParams(obj) { Object .keys(sanitizedObj) .forEach((key) => { - queryParamObj[snakeCase(key)] = sanitizedObj[key]; + queryParamObj[decamelize(key)] = sanitizedObj[key]; }); // Use bracket arrayFormat as history.js and react-router use it diff --git a/package.json b/package.json index 4e7cd6a9..63c6d1e0 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "camelcase": "^1.2.1", "classnames": "^1.2.2", "compression": "^1.4.4", + "decamelize": "^1.1.1", "envify": "^3.4.0", "eslint": "^0.22.1", "eslint-plugin-react": "^2.5.0", @@ -74,7 +75,7 @@ "object-assign": "^2.0.0", "opn": "^3.0.2", "q": "^1.4.1", - "qs": "^5.2.0", + "qs": "^4.0.0", "raven-js": "^1.1.19", "react": "0.13.2", "react-bootstrap": "0.25.1", @@ -85,7 +86,6 @@ "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", From 1a3dffe8bc396ca6af2db768c86036573509ba65 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 18:31:01 +0100 Subject: [PATCH 09/11] Modify excludePropFromObject to be similar to lodash.omit() --- js/utils/general_utils.js | 39 +++++++++++++++++++++++++++++++++------ js/utils/requests.js | 4 ++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/js/utils/general_utils.js b/js/utils/general_utils.js index bce2dee6..c13e936e 100644 --- a/js/utils/general_utils.js +++ b/js/utils/general_utils.js @@ -143,14 +143,41 @@ export function escapeHTML(s) { return document.createElement('div').appendChild(document.createTextNode(s)).parentNode.innerHTML; } -export function excludePropFromObject(obj, propList){ - let clonedObj = mergeOptions({}, obj); - for (let item in propList){ - if (clonedObj[propList[item]]){ - delete clonedObj[propList[item]]; +/** + * Returns a copy of the given object's own and inherited enumerable + * properties, omitting any keys that pass the given filter function. + */ +function filterObjOnFn(obj, filterFn) { + const filteredObj = {}; + + for (let key in obj) { + const val = obj[key]; + if (filterFn == null || !filterFn(val, key)) { + filteredObj[key] = val; } } - return clonedObj; + + return filteredObj; +} + +/** + * Similar to lodash's _.omit(), this returns a copy of the given object's + * own and inherited enumerable properties, omitting any keys that are + * in the given array or whose value pass the given filter function. + * @param {object} obj Source object + * @param {array|function} filter Array of key names to omit or function to invoke per iteration + * @return {object} The new object +*/ +export function omitFromObject(obj, filter) { + if (filter && filter.constructor === Array) { + return filterObjOnFn(obj, (_, key) => { + return filter.indexOf(key) >= 0; + }); + } else if (filter && typeof filter === 'function') { + return filterObjOnFn(obj, filter); + } else { + throw new Error('The given filter is not an array or function. Exclude aborted'); + } } /** diff --git a/js/utils/requests.js b/js/utils/requests.js index ce24aaba..8f015a11 100644 --- a/js/utils/requests.js +++ b/js/utils/requests.js @@ -5,7 +5,7 @@ import Q from 'q'; import AppConstants from '../constants/application_constants'; import { getCookie } from '../utils/fetch_api_utils'; -import { excludePropFromObject } from '../utils/general_utils'; +import { omitFromObject } from '../utils/general_utils'; import { argsToQueryParams } from '../utils/url_utils'; class Requests { @@ -127,9 +127,9 @@ class Requests { return this.request('delete', newUrl); } - let params = excludePropFromObject(paramsAndBody, ['body']); _putOrPost(url, paramsAndBody, method) { let paramsCopy = Object.assign({}, paramsAndBody); + let params = omitFromObject(paramsAndBody, ['body']); let newUrl = this.prepareUrl(url, params); let body = null; if (paramsCopy && paramsCopy.body) { From 0b4cc3123d9967d62301f846f7ed372f9b26f92d Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 18:45:06 +0100 Subject: [PATCH 10/11] Remove lodash dependency --- js/utils/general_utils.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/js/utils/general_utils.js b/js/utils/general_utils.js index c13e936e..06e6d3ee 100644 --- a/js/utils/general_utils.js +++ b/js/utils/general_utils.js @@ -1,7 +1,5 @@ 'use strict'; -import _ from 'lodash'; - /** * Takes an object and returns a shallow copy without any keys * that fail the passed in filter function. From cb6e94c8f13d1183ae04a2a08d51d6eea0c70a54 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Mon, 2 Nov 2015 18:57:56 +0100 Subject: [PATCH 11/11] Forgot to use our own omit instead of lodash's --- js/utils/general_utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/utils/general_utils.js b/js/utils/general_utils.js index 06e6d3ee..e717fa75 100644 --- a/js/utils/general_utils.js +++ b/js/utils/general_utils.js @@ -15,7 +15,7 @@ export function sanitize(obj, filterFn) { filterFn = (val) => val == null || val === ''; } - return _.omit(obj, filterFn); + return omitFromObject(obj, filterFn); } /**