(Bug) Keep data between the steps (Step 2)

mariano-aguero commented 6 years ago

This is an integration PR for the step 2.

How to test it and what to test:

Go to the home
Create a crowdsale
In the first step, press button "New crowdsale"
Select the type of strategy
Go to the second step
Go back to step 1
Should keep the selected strategy
Refresh the step 1 page
Should keep the selected strategy
Got to the second step
Complete the form
Refresh the page, should keep the data
Add reserved tokens
Refresh the page, should keep the data
Go to the home, and press button "New crowdsale"
The selected strategy must be the default, "WHITELIST WITH CAP"
Go to the second step
The form must be empty

BlackDuckCoPilot commented 6 years ago

Black Duck Security Report

Merging #1071 into 2.0-steps-integration will not change security risk.

Added Components

Clean: 1

Click here to see full report

dennis00010011b commented 6 years ago

@mariano-aguero Minted/Dutch-Step2: data losed after refreshing if any field is empty

https://www.useloom.com/share/84b3389adf20463e8a2ec25550eace50

dennis00010011b commented 6 years ago

@mariano-aguero Minted- Step2: button clear all doesn't clear reserved addresses

https://www.useloom.com/share/4ca437c75b254eda821f42f6ff9db8e1

dennis00010011b commented 6 years ago

@mariano-aguero Data from Minted passed to Dutch. Steps:

Select Minted, go to Step2
Fill out all fields, add reserved address
Press button back
Select Dutch, click continue
Observe fields, decimals field is disabled https://www.useloom.com/share/6ba8bd970bab40168b4b875788e076fb

mariano-aguero commented 6 years ago

@dennis00010011b

Minted/Dutch-Step2: data losed after refreshing if any field is empty

Can you execute npm i in the terminal ? I think a new package is missing, in my computer: https://www.useloom.com/share/dd2764271ce242a9b822604d369c9362

mariano-aguero commented 6 years ago

@dennis00010011b

Minted- Step2: button clear all doesn't clear reserved addresses

Fixed

mariano-aguero commented 6 years ago

@dennis00010011b

Data from Minted passed to Dutch. Steps:

Select Minted, go to Step2 Fill out all fields, add reserved address Press button back Select Dutch, click continue Observe fields, decimals field is disabled

Fix input decimals, I check the strategy selected See video https://www.useloom.com/share/353383626685438e87f033bc2325388e

dennis00010011b commented 6 years ago

@mariano-aguero Data still passed from Minted to Dutch and vice versa. See screenshot: possible situation when decimals =0 but reserved address has fractional value of tokens.

dennis00010011b commented 6 years ago

Minted/Dutch-Step2: data lost after refreshing if any field is empty

Can you execute npm i in the terminal ? I think a new package is missing, in my computer:

I tested after npm i. Issue still reproducible. I want to clarify: data lost after refresh if at least one field empty in Step2:

Minted: name or ticker field
Dutch: name or ticker field, or supply=0 https://www.useloom.com/share/84b3389adf20463e8a2ec25550eace50

mariano-aguero commented 6 years ago

@dennis00010011b

I tested after npm i. Issue still reproducible. I want to clarify: data lost after refresh if at least one field empty in Step2: Minted: name or ticker field Dutch: name or ticker field, or supply=0

I change how isEmpty function works, Done!

mariano-aguero commented 6 years ago

@dennis00010011b

Data still passed from Minted to Dutch and vice versa. See screenshot: possible situation when decimals =0 but reserved address has fractional value of tokens.

Can you test again? The above change should affect the behavior. The decimals field is disabled if already exists reserved tokens (business rule)

dennis00010011b commented 6 years ago

@mariano-aguero

Minted/Dutch-Step2: data lost after refreshing if any field is empty

It fixed.

Data still passed from Minted to Dutch and vice versa. See screenshot: possible situation when decimals =0 but reserved address has fractional value of tokens.

Issue still here. Steps for reproduce:

In Step 1 select Minted, go to Step 2.
Fill out fields name, ticker with any valid data.
Set decimals = 10. Add any reserved address with value 0.123456789 tokens
Go back to Step1.
Select Dutch, open Step2
Set decimals = 0
Go back to Step1.
Select Minted, go to Step 2.

Actual result:

token is not divisible (decimals=0) but reserved address should receive value <1 token

Expected result:

should be validation:decimals of reserved value should not exceed the amount of decimals specified

mariano-aguero commented 6 years ago

Great report @dennis00010011b ! i will check it right now

mariano-aguero commented 6 years ago

Issue still here. Steps for reproduce: In Step 1 select Minted, go to Step 2. Fill out fields name, ticker with any valid data. Set decimals = 10. Add any reserved address with value 0.123456789 tokens Go back to Step1. Select Dutch, open Step2 Set decimals = 0 Go back to Step1. Select Minted, go to Step 2. Actual result: token is not divisible (decimals=0) but reserved address should receive value <1 token Expected result: should be validation:decimals of reserved value should not exceed the amount of decimals specified

I create a function to cap decimals in this situation, can you checked @dennis00010011b ?

mariano-aguero commented 6 years ago

@fernandomg

A behavior I'm not sure is worth considering in this PR... but will just leave it here for the record...

Fixed

dennis00010011b commented 6 years ago

@mariano-aguero It is rare user's behavior but it leads to crash

User able to enter invalid value of decimals Steps:

Select Minted, go to Step2
Fill out name, ticker, decimals with valid data
Add reserved tokens
Go back to Step1
Select Dutch, go to Step 2
Enter invalid decimals
Go back to Step 1
Select Minted, go to Step2

Actual result: wizard failed with error message https://www.useloom.com/share/2c4860f44dbb402cad064c83582be7d9

mariano-aguero commented 6 years ago

@dennis00010011b

It is rare user's behavior but it leads to crash

Apply BigNumber and fixed

dennis00010011b commented 6 years ago

Tested it. No issues were found

mariano-aguero commented 6 years ago

Updated with changes from branch 2.0

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 2866

52 of 105 (49.52%) changed or added relevant lines in 16 files are covered.
7 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.2%) to 20.64%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/utils/blockchainHelpers.js	0	1	0.0%
src/App.js	0	2	0.0%
src/stores/TokenStore.js	0	2	0.0%
src/components/stepTwo/index.js	0	4	0.0%
src/components/stepTwo/StepTwoForm.js	6	10	60.0%
src/utils/utils.js	0	4	0.0%
src/stores/ReservedTokenStore.js	0	8	0.0%
src/components/Home/index.js	0	14	0.0%
src/components/stepOne/index.js	0	14	0.0%
<!--	Total:	52	105	49.52%	-->