From e93165cb5d9a38b645373b509649ac3bd6fa9a0b Mon Sep 17 00:00:00 2001 From: fernandomg Date: Mon, 9 Apr 2018 15:00:37 -0300 Subject: [PATCH 1/6] Add specific validation for min/max whitelist values --- src/utils/constants.js | 2 +- src/utils/validations.js | 22 ++++++++++++++ src/utils/validations.spec.js | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/utils/constants.js b/src/utils/constants.js index 174c8391f..4b900f7a3 100644 --- a/src/utils/constants.js +++ b/src/utils/constants.js @@ -116,7 +116,7 @@ export const VALIDATION_MESSAGES = { REQUIRED: 'This field is required', DECIMAL_PLACES: 'Decimals should not exceed the amount of decimals specified', LESS_OR_EQUAL: 'Should be less or equal than the specified value', - GREATER_OR_EQUAL: 'Should be greater than the specified value', + GREATER_OR_EQUAL: 'Should be greater or equal than the specified value', INTEGER: 'Should be integer', DATE_IN_FUTURE: 'Should be set in the future', DATE_IS_PREVIOUS: 'Should be previous than specified time', diff --git a/src/utils/validations.js b/src/utils/validations.js index 9e11afb01..67da12b8f 100644 --- a/src/utils/validations.js +++ b/src/utils/validations.js @@ -26,6 +26,28 @@ export const validateDecimals = (value) => { return isValid ? undefined : VALIDATION_MESSAGES.DECIMALS } +export const validateWhitelistMin = ({ min, max, decimals }) => { + const listOfErrors = composeValidators( + isRequired(), + isNonNegative(), + isDecimalPlacesNotGreaterThan(`Decimals should not exceed ${decimals} places`)(decimals), + isLessOrEqualThan('Should be less or equal than max')(max) + )(min) + + return listOfErrors ? listOfErrors.shift() : '' +} + +export const validateWhitelistMax = ({ min, max, decimals }) => { + const listOfErrors = composeValidators( + isRequired(), + isNonNegative(), + isDecimalPlacesNotGreaterThan(`Decimals should not exceed ${decimals} places`)(decimals), + isGreaterOrEqualThan('Should be greater or equal than min')(min) + )(max) + + return listOfErrors ? listOfErrors.shift() : '' +} + export const isPositive = (errorMsg = VALIDATION_MESSAGES.POSITIVE) => (value) => { const isValid = value > 0 return isValid ? undefined : errorMsg diff --git a/src/utils/validations.spec.js b/src/utils/validations.spec.js index 5659ba0c8..dafcdace4 100644 --- a/src/utils/validations.spec.js +++ b/src/utils/validations.spec.js @@ -16,6 +16,8 @@ import { validateDecimals, validateTicker, validateTokenName, + validateWhitelistMax, + validateWhitelistMin, validators } from './validations' import { VALIDATION_MESSAGES } from './constants' @@ -87,6 +89,58 @@ describe('validateDecimals', () => { }) }) +describe('validateWhitelistMin', () => { + const testCases = [ + { value: { min: '0', max: '1', decimals: '0' }, expected: '' }, + { value: { min: '0', max: '10', decimals: '0' }, expected: '' }, + { value: { min: '5', max: '11', decimals: '5' }, expected: '' }, + { value: { min: '10', max: '10', decimals: '0' }, expected: '' }, + { value: { min: '10', max: '10', decimals: '7' }, expected: '' }, + { value: { min: '5', max: '11.123456', decimals: '3' }, expected: '' }, + { value: { min: '5.123', max: '11.123456', decimals: '3' }, expected: '' }, + { value: { min: '5.123456', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, + { value: { min: '5.123456', max: '11.123', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, + { value: { min: '25.123456', max: '11.123', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, + { value: { min: '25.123', max: '11.123', decimals: '3' }, expected: 'Should be less or equal than max' }, + { value: { min: '5', max: '3', decimals: '0' }, expected: 'Should be less or equal than max' }, + ] + + testCases.forEach(testCase => { + const action = testCase.expected ? 'fail' : 'pass' + const { min, max, decimals } = testCase.value + + it(`Should ${action} for { min: '${min}', max: '${max}', decimals: '${decimals}' }`, () => { + expect(validateWhitelistMin({ ...testCase.value })).toBe(testCase.expected) + }) + }) +}) + +describe('validateWhitelistMax', () => { + const testCases = [ + { value: { min: '0', max: '1', decimals: '0' }, expected: '' }, + { value: { min: '0', max: '10', decimals: '0' }, expected: '' }, + { value: { min: '5', max: '11', decimals: '5' }, expected: '' }, + { value: { min: '10', max: '10', decimals: '0' }, expected: '' }, + { value: { min: '10', max: '10', decimals: '7' }, expected: '' }, + { value: { min: '5.123456', max: '11', decimals: '3' }, expected: '' }, + { value: { min: '5.123345', max: '11.123', decimals: '3' }, expected: '' }, + { value: { min: '5.123456', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, + { value: { min: '5.123', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, + { value: { min: '25.123', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, + { value: { min: '25.123', max: '11.123', decimals: '3' }, expected: 'Should be greater or equal than min' }, + { value: { min: '5', max: '3', decimals: '0' }, expected: 'Should be greater or equal than min' }, + ] + + testCases.forEach(testCase => { + const action = testCase.expected ? 'fail' : 'pass' + const { min, max, decimals } = testCase.value + + it(`Should ${action} for { min: '${min}', max: '${max}', decimals: '${decimals}' }`, () => { + expect(validateWhitelistMax({ ...testCase.value })).toBe(testCase.expected) + }) + }) +}) + describe('isPositive', () => { const testCases = [ { value: '1.01', errorMessage: undefined, expected: undefined }, @@ -541,3 +595,4 @@ describe('composeValidators', () => { expect(listOfErrors).toBeUndefined() }) }) + From f0091391713d22243fbf0e3381d32c9dc13cfc7f Mon Sep 17 00:00:00 2001 From: fernandomg Date: Mon, 9 Apr 2018 15:04:02 -0300 Subject: [PATCH 2/6] Validate min/max values for whitelist items in StepThree --- src/components/Common/TierBlock.js | 2 +- src/components/Common/WhitelistInputBlock.js | 115 ++++++++++++++++-- src/components/stepThree/StepThreeForm.js | 1 + .../__snapshots__/CrowdsaleBlock.spec.js.snap | 6 + 4 files changed, 116 insertions(+), 8 deletions(-) diff --git a/src/components/Common/TierBlock.js b/src/components/Common/TierBlock.js index 1af9690fb..08204652a 100644 --- a/src/components/Common/TierBlock.js +++ b/src/components/Common/TierBlock.js @@ -186,7 +186,7 @@ export const TierBlock = ({ fields, ...props }) => {

Whitelist

- + ) : null } diff --git a/src/components/Common/WhitelistInputBlock.js b/src/components/Common/WhitelistInputBlock.js index 21a175bd4..27141f5e7 100644 --- a/src/components/Common/WhitelistInputBlock.js +++ b/src/components/Common/WhitelistInputBlock.js @@ -5,11 +5,12 @@ import Dropzone from 'react-dropzone'; import Papa from 'papaparse' import '../../assets/stylesheets/application.css'; import { InputField } from './InputField' -import { TEXT_FIELDS, VALIDATION_TYPES } from '../../utils/constants' +import { TEXT_FIELDS, VALIDATION_MESSAGES, VALIDATION_TYPES } from '../../utils/constants' import { WhitelistItem } from './WhitelistItem' import { inject, observer } from 'mobx-react' import { clearingWhitelist, whitelistImported } from '../../utils/alerts' import processWhitelist from '../../utils/processWhitelist' +import { validateWhitelistMax, validateWhitelistMin } from '../../utils/validations' const { ADDRESS, MIN, MAX } = TEXT_FIELDS const {VALID, INVALID} = VALIDATION_TYPES; @@ -26,7 +27,17 @@ export class WhitelistInputBlock extends React.Component { address: { pristine: true, valid: INVALID - } + }, + min: { + pristine: true, + valid: INVALID, + errorMessage: VALIDATION_MESSAGES.REQUIRED + }, + max: { + pristine: true, + valid: INVALID, + errorMessage: VALIDATION_MESSAGES.REQUIRED + }, } } } @@ -40,11 +51,23 @@ export class WhitelistInputBlock extends React.Component { validation: { address: { pristine: { $set: false } - } + }, + min: { + pristine: { $set: false } + }, + max: { + pristine: { $set: false } + }, } })) - if (!addr || !min || !max || this.state.validation.address.valid === INVALID) { + const { + address: { valid: addrValid }, + min: { valid: minValid }, + max: { valid: maxValid }, + } = this.state.validation + + if (!addr || !min || !max || addrValid === INVALID || minValid === INVALID || maxValid === INVALID) { return } @@ -56,7 +79,17 @@ export class WhitelistInputBlock extends React.Component { address: { pristine: true, valid: INVALID - } + }, + min: { + pristine: true, + valid: INVALID, + errorMessage: VALIDATION_MESSAGES.REQUIRED + }, + max: { + pristine: true, + valid: INVALID, + errorMessage: VALIDATION_MESSAGES.REQUIRED + }, } }) @@ -81,6 +114,68 @@ export class WhitelistInputBlock extends React.Component { this.setState(newState) } + handleMinChange = ({ min }) => { + const errorMessage = validateWhitelistMin({ + min, + max: this.state.max, + decimals: this.props.decimals + }) + + return new Promise((resolve) => { + this.setState(update(this.state, { + min: { $set: min }, + validation: { + min: { + $set: { + pristine: false, + valid: errorMessage ? INVALID : VALID, + errorMessage + } + } + } + }), resolve) + }) + } + + handleMaxChange = ({ max }) => { + const errorMessage = validateWhitelistMax({ + min: this.state.min, + max, + decimals: this.props.decimals + }) + + return new Promise((resolve) => { + this.setState(update(this.state, { + max: { $set: max }, + validation: { + max: { + $set: { + pristine: false, + valid: errorMessage ? INVALID : VALID, + errorMessage + } + } + } + }), resolve) + }) + } + + handleMinMaxChange = ({ min, max }) => { + if (min !== undefined) { + this.handleMinChange({ min }) + .then(() => { + if (!this.state.validation.max.pristine) this.handleMaxChange({ max: this.state.max }) + }) + } + + if (max !== undefined) { + this.handleMaxChange({ max }) + .then(() => { + if (!this.state.validation.min.pristine) this.handleMinChange({ min: this.state.min }) + }) + } + } + onDrop = (acceptedFiles, rejectedFiles) => { acceptedFiles.forEach(file => { Papa.parse(file, { @@ -149,16 +244,22 @@ export class WhitelistInputBlock extends React.Component { type='number' title={MIN} value={this.state.min} - onChange={e => this.setState({ min: e.target.value })} + onChange={e => this.handleMinMaxChange({ min: e.target.value })} description={`Minimum amount tokens to buy. Not a minimal size of a transaction. If minCap is 1 and user bought 1 token in a previous transaction and buying 0.1 token it will allow him to buy.`} + pristine={this.state.validation.min.pristine} + valid={this.state.validation.min.valid} + errorMessage={this.state.validation.min.errorMessage} /> this.setState({ max: e.target.value })} + onChange={e => this.handleMinMaxChange({ max: e.target.value })} description={`Maximum is the hard limit.`} + pristine={this.state.validation.max.pristine} + valid={this.state.validation.max.valid} + errorMessage={this.state.validation.max.errorMessage} />
diff --git a/src/components/stepThree/StepThreeForm.js b/src/components/stepThree/StepThreeForm.js index b613c1f3d..a9927a1b3 100644 --- a/src/components/stepThree/StepThreeForm.js +++ b/src/components/stepThree/StepThreeForm.js @@ -154,6 +154,7 @@ export const StepThreeForm = ({ handleSubmit, values, invalid, pristine, mutator )} diff --git a/src/components/stepThree/__snapshots__/CrowdsaleBlock.spec.js.snap b/src/components/stepThree/__snapshots__/CrowdsaleBlock.spec.js.snap index 7130d98c1..ab482b38c 100644 --- a/src/components/stepThree/__snapshots__/CrowdsaleBlock.spec.js.snap +++ b/src/components/stepThree/__snapshots__/CrowdsaleBlock.spec.js.snap @@ -1567,10 +1567,13 @@ exports[`CrowdsaleBlock Should render the component for the second Tier with whi
Date: Mon, 9 Apr 2018 15:34:41 -0300 Subject: [PATCH 3/6] Update manage screen to support min/max whitelist validation --- src/components/manage/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/manage/index.js b/src/components/manage/index.js index d272a85b0..04fe37821 100644 --- a/src/components/manage/index.js +++ b/src/components/manage/index.js @@ -361,10 +361,12 @@ export class Manage extends Component { } whitelistInputBlock = index => { + const { tokenStore } = this.props return ( ) } From bab0e896cc38c78dffae88e21ccb7d53639d128d Mon Sep 17 00:00:00 2001 From: fernandomg Date: Mon, 9 Apr 2018 15:37:12 -0300 Subject: [PATCH 4/6] Update bulk CSV import to support min/max whitelist validation --- src/components/Common/WhitelistInputBlock.js | 5 ++++- src/utils/processWhitelist.js | 19 +++++++++++-------- src/utils/processWhitelist.spec.js | 12 ++++++------ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/components/Common/WhitelistInputBlock.js b/src/components/Common/WhitelistInputBlock.js index 27141f5e7..cc6e02680 100644 --- a/src/components/Common/WhitelistInputBlock.js +++ b/src/components/Common/WhitelistInputBlock.js @@ -181,7 +181,10 @@ export class WhitelistInputBlock extends React.Component { Papa.parse(file, { skipEmptyLines: true, complete: results => { - const { called } = processWhitelist(results.data, item => { + const { called } = processWhitelist({ + rows: results.data, + decimals: this.props.decimals + }, item => { this.props.tierStore.addWhitelistItem(item, this.props.num) }) diff --git a/src/utils/processWhitelist.js b/src/utils/processWhitelist.js index 6d220e2e0..153c0494b 100644 --- a/src/utils/processWhitelist.js +++ b/src/utils/processWhitelist.js @@ -1,26 +1,29 @@ -import Web3 from 'web3' - -const isNumber = (number) => !isNaN(parseFloat(number)) +import { isAddress, validateWhitelistMin, validateWhitelistMax } from './validations' /** * Execute a callback with each valid whitelist item in the given list * - * @param {Array} rows Array of whitelist items. Each element in the array has the structure `[address, min, max]`, for - * example: `['0x1234567890123456789012345678901234567890', '1', '10']` + * @param {Object} whitelistInformation + * @param {Array} whitelistInformation.rows Array of whitelist items. Each element in the array has the structure + * `[address, min, max]`, for example: `['0x1234567890123456789012345678901234567890', '1', '10']` + * @param {Number} whitelistInformation.decimals Amount of decimals allowed for the min and max values * @param {Function} cb The function to be called with each valid item * @returns {Object} Object with a `called` property, indicating the number of times the callback was called */ -export default function (rows, cb) { +export default function ({ rows, decimals }, cb) { let called = 0 rows.forEach((row) => { if (row.length !== 3) return const [addr, min, max] = row - if (!Web3.utils.isAddress(addr) || !isNumber(min) || !isNumber(max)) return + if ( + isAddress()(addr) || + validateWhitelistMin({ min, max, decimals }) || + validateWhitelistMax({ min, max, decimals }) + ) return cb({ addr, min, max }) - called++ }) diff --git a/src/utils/processWhitelist.spec.js b/src/utils/processWhitelist.spec.js index b03d9fa42..294be21ee 100644 --- a/src/utils/processWhitelist.spec.js +++ b/src/utils/processWhitelist.spec.js @@ -11,7 +11,7 @@ describe('processWhitelist function', () => { const cb = jest.fn() // When - processWhitelist(rows, cb) + processWhitelist({ rows, decimals: 3 }, cb) // Then expect(cb).toHaveBeenCalledTimes(3) @@ -20,7 +20,7 @@ describe('processWhitelist function', () => { expect(cb.mock.calls[2]).toEqual([{ addr: rows[2][0], min: rows[2][1], max: rows[2][2] }]) }) - it('should ignore items that don\t have 3 elements', () => { + it('should ignore items that don\'t have 3 elements', () => { // Given const rows = [ ['1', '10'], @@ -33,7 +33,7 @@ describe('processWhitelist function', () => { const cb = jest.fn() // When - processWhitelist(rows, cb) + processWhitelist({ rows, decimals: 3 }, cb) // Then expect(cb).toHaveBeenCalledTimes(0) @@ -49,7 +49,7 @@ describe('processWhitelist function', () => { const cb = jest.fn() // When - const { called } = processWhitelist(rows, cb) + const { called } = processWhitelist({ rows, decimals: 3 }, cb) // Then expect(called).toBe(3) @@ -66,7 +66,7 @@ describe('processWhitelist function', () => { const cb = jest.fn() // When - const { called } = processWhitelist(rows, cb) + const { called } = processWhitelist({ rows, decimals: 3 }, cb) // Then expect(called).toBe(0) @@ -83,7 +83,7 @@ describe('processWhitelist function', () => { const cb = jest.fn() // When - const { called } = processWhitelist(rows, cb) + const { called } = processWhitelist({ rows, decimals: 3 }, cb) // Then expect(called).toBe(0) From a234139ad0589ac6a6d5c0b6b49da54cf4a42fd6 Mon Sep 17 00:00:00 2001 From: fernandomg Date: Thu, 12 Apr 2018 09:50:21 -0300 Subject: [PATCH 5/6] Use `undefined` as a return value for validations --- src/utils/validations.js | 4 ++-- src/utils/validations.spec.js | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/utils/validations.js b/src/utils/validations.js index 1a9fb3cb2..88fe45bcf 100644 --- a/src/utils/validations.js +++ b/src/utils/validations.js @@ -28,7 +28,7 @@ export const validateWhitelistMin = ({ min, max, decimals }) => { isLessOrEqualThan('Should be less or equal than max')(max) )(min) - return listOfErrors ? listOfErrors.shift() : '' + return listOfErrors ? listOfErrors.shift() : undefined } export const validateWhitelistMax = ({ min, max, decimals }) => { @@ -39,7 +39,7 @@ export const validateWhitelistMax = ({ min, max, decimals }) => { isGreaterOrEqualThan('Should be greater or equal than min')(min) )(max) - return listOfErrors ? listOfErrors.shift() : '' + return listOfErrors ? listOfErrors.shift() : undefined } export const isPositive = (errorMsg = VALIDATION_MESSAGES.POSITIVE) => (value) => { diff --git a/src/utils/validations.spec.js b/src/utils/validations.spec.js index 92a0338ca..fc1791ffd 100644 --- a/src/utils/validations.spec.js +++ b/src/utils/validations.spec.js @@ -66,13 +66,13 @@ describe('validateTicker', () => { describe('validateWhitelistMin', () => { const testCases = [ - { value: { min: '0', max: '1', decimals: '0' }, expected: '' }, - { value: { min: '0', max: '10', decimals: '0' }, expected: '' }, - { value: { min: '5', max: '11', decimals: '5' }, expected: '' }, - { value: { min: '10', max: '10', decimals: '0' }, expected: '' }, - { value: { min: '10', max: '10', decimals: '7' }, expected: '' }, - { value: { min: '5', max: '11.123456', decimals: '3' }, expected: '' }, - { value: { min: '5.123', max: '11.123456', decimals: '3' }, expected: '' }, + { value: { min: '0', max: '1', decimals: '0' }, expected: undefined }, + { value: { min: '0', max: '10', decimals: '0' }, expected: undefined }, + { value: { min: '5', max: '11', decimals: '5' }, expected: undefined }, + { value: { min: '10', max: '10', decimals: '0' }, expected: undefined }, + { value: { min: '10', max: '10', decimals: '7' }, expected: undefined }, + { value: { min: '5', max: '11.123456', decimals: '3' }, expected: undefined }, + { value: { min: '5.123', max: '11.123456', decimals: '3' }, expected: undefined }, { value: { min: '5.123456', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, { value: { min: '5.123456', max: '11.123', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, { value: { min: '25.123456', max: '11.123', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, @@ -92,13 +92,13 @@ describe('validateWhitelistMin', () => { describe('validateWhitelistMax', () => { const testCases = [ - { value: { min: '0', max: '1', decimals: '0' }, expected: '' }, - { value: { min: '0', max: '10', decimals: '0' }, expected: '' }, - { value: { min: '5', max: '11', decimals: '5' }, expected: '' }, - { value: { min: '10', max: '10', decimals: '0' }, expected: '' }, - { value: { min: '10', max: '10', decimals: '7' }, expected: '' }, - { value: { min: '5.123456', max: '11', decimals: '3' }, expected: '' }, - { value: { min: '5.123345', max: '11.123', decimals: '3' }, expected: '' }, + { value: { min: '0', max: '1', decimals: '0' }, expected: undefined }, + { value: { min: '0', max: '10', decimals: '0' }, expected: undefined }, + { value: { min: '5', max: '11', decimals: '5' }, expected: undefined }, + { value: { min: '10', max: '10', decimals: '0' }, expected: undefined }, + { value: { min: '10', max: '10', decimals: '7' }, expected: undefined }, + { value: { min: '5.123456', max: '11', decimals: '3' }, expected: undefined }, + { value: { min: '5.123345', max: '11.123', decimals: '3' }, expected: undefined }, { value: { min: '5.123456', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, { value: { min: '5.123', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, { value: { min: '25.123', max: '11.123456', decimals: '3' }, expected: 'Decimals should not exceed 3 places' }, From 3970d86afdd733f8dfb3c38b1a0e9f45048da0ee Mon Sep 17 00:00:00 2001 From: fernandomg Date: Thu, 12 Apr 2018 10:10:19 -0300 Subject: [PATCH 6/6] Add tests for decimals and min/max --- src/utils/processWhitelist.spec.js | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/utils/processWhitelist.spec.js b/src/utils/processWhitelist.spec.js index 294be21ee..aa55b8293 100644 --- a/src/utils/processWhitelist.spec.js +++ b/src/utils/processWhitelist.spec.js @@ -88,4 +88,40 @@ describe('processWhitelist function', () => { // Then expect(called).toBe(0) }) + + it('should reject invalid decimals', () => { + // Given + const rows = [ + ['0x1111111111111111111111111111111111111111', '10', '10.1'], + ['0x3333333333333333333333333333333333333333', '10.1234', '10'], + ['0x2222222222222222222222222222222222222222', '10.12', '10.123'], + ['0x4444444444444444444444444444444444444444', '10.123456', '10.123456'] + ] + const cb = jest.fn() + + // When + const { called } = processWhitelist({ rows, decimals: 3 }, cb) + + // Then + expect(called).toBe(2) + }) + + it('should reject min > max', () => { + // Given + const rows = [ + ['0x1111111111111111111111111111111111111111', '15', '10.1'], + ['0x2222222222222222222222222222222222222222', '10.13', '10.123'], + ['0x3333333333333333333333333333333333333333', '100', '99.999999999999999999'], + ['0x3333333333333333333333333333333333333333', '11', '11'], + ['0x3333333333333333333333333333333333333333', '10.124', '11'], + ['0x4444444444444444444444444444444444444444', '10.124', '10.125'] + ] + const cb = jest.fn() + + // When + const { called } = processWhitelist({ rows, decimals: 3 }, cb) + + // Then + expect(called).toBe(3) + }) })