Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Bug) Keep data between the steps (Step 4 - Publish && Step 5 - Contribution Page) #1097

Merged
merged 8 commits into from
Aug 29, 2018

Conversation

mariano-aguero
Copy link
Contributor

@dennis00010011b
Can you check this two steps with this branch?
I Perform some tests and it seems to work well, I would need your expert eye

Thanks in advance

@coveralls
Copy link

coveralls commented Aug 17, 2018

Pull Request Test Coverage Report for Build 2926

  • 7 of 158 (4.43%) changed or added relevant lines in 14 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.5%) to 20.139%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/utils/blockchainHelpers.js 0 1 0.0%
src/utils/alerts.js 0 2 0.0%
src/stores/Web3Store.js 0 3 0.0%
src/stores/GeneralStore.js 3 6 50.0%
src/components/Common/TierBlock.js 1 6 16.67%
src/components/Common/DutchAuctionBlock.js 0 6 0.0%
src/utils/utils.js 1 10 10.0%
src/components/stepThree/index.js 0 10 0.0%
src/stores/TierStore.js 0 12 0.0%
src/components/stepFour/index.js 0 18 0.0%
Files with Coverage Reduction New Missed Lines %
src/components/Common/DutchAuctionBlock.js 1 6.12%
src/components/stepFour/index.js 2 0.0%
src/components/stepThree/GasPriceInput.js 4 0.0%
Totals Coverage Status
Change from base Build 2887: -0.5%
Covered Lines: 876
Relevant Lines: 3726

💛 - Coveralls

@dennis00010011b
Copy link

@mariano-aguero
Probably it should be separate issue
Step4-deployment process: user able to change network and continue deployment
Steps for reproduce:

  1. Set network 'Sokol' in Nifty wallet
  2. Step 1: select Minted
  3. Proceed through Step1-Step3 with valid data
  4. Step4: confirm 2 transactions
  5. Nifty wallet: switch network to Ropsten

Expected result:

  • should be warning which prevent from continue deployment
  • user won't able to continue deployment unless switch back to pristine network

Actual result:

  • user able to continue and finish deployment with other network

@dennis00010011b
Copy link

@mariano-aguero
Crowdsale page: transaction is generated if user switch between Publish and Crowdsale pages
Steps:

  1. Create Minted crowdsale with any valid parameters, proceed until Crowdsale page
  2. Go back to Publish page
  3. Click button 'Continue' and go forward to Crowdsale page
  4. Observe Wallet, new transactions

Actual result:

  • new transaction is generated each time when user switchs between Publish and crowdsale pages

https://www.useloom.com/share/bca7bd600b6d406b9c13bcb70c28ac0a

@mariano-aguero
Copy link
Contributor Author

mariano-aguero commented Aug 21, 2018

Hi @dennis00010011b

Probably it should be separate issue
Step4-deployment process: user able to change network and continue deployment

Done in this branch

How to reproduce it:

  • Create a crowdsale and fill out the forms until you reach step 4
  • Perform only the first transaction
  • Change the network from nifty wallet or metamask
  • The page has to be refreshed and a warning will appear

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

Crowdsale page: transaction is generated if user switch between Publish and Crowdsale pages

Fixed

@dennis00010011b
Copy link

dennis00010011b commented Aug 22, 2018

@mariano-aguero
Issue that isn't related RP's feature but anyway.
Minted- Publish page: additional tier is rendered after each page refreshing
screen shot 2018-08-22 at 5 46 16 am

@dennis00010011b
Copy link

@mariano-aguero
In the case when user switches between network on Step4/Publish page the warning should look like warning from contribution page in the same situation

Now
screen shot 2018-08-22 at 5 54 17 am

Should be
screen shot 2018-08-22 at 5 59 53 am

@dennis00010011b
Copy link

@mariano-aguero
Step 4: transaction is generated after each page refreshing
Steps:

  1. Create a crowdsale and fill out the forms until you reach step 4. Use any network.
  2. Refresh the page
  3. Observe count of transaction in Nifty wallet

screen shot 2018-08-22 at 6 11 22 am

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

Issue that isn't related RP's feature but anyway.

An additional tier no longer appears

https://www.useloom.com/share/56c9e4fbb6ea4f748fafb2b9405e8ca2

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

In the case when user switches between network on Step4/Publish page the warning should look like warning from contribution page in the same situation

Change alert

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

Step 4: transaction is generated after each page refreshing

Speaking with @fernandomg , we thought that the user should cancel the previous transaction (reject). The solution to this problem would imply a very high programming cost which I do not think is the time to face. I think we could create an issue for it and work it after the design application, Do you agree?

@dennis00010011b
Copy link

@mariano-aguero Ok,I agree. I'll create an issue

@dennis00010011b
Copy link

dennis00010011b commented Aug 27, 2018

@mariano-aguero
#1097 (comment)

User able to finish deployment without signing all transactions
Steps:

  1. Set up account to Sokol network.
  2. Start create the Minted crowdsale, in Step 4 sign 2-3 transaction
  3. Switch to Kovan network.
  4. Confirm warning and proceed with button Continue until Contribution page is open.
  5. Switch back to Sokol network.

Actual result:

  • created crowdsale doesn't match given in Step3 parameters (because not all contracts were deployed)

Possible solution:

  1. in Step 4 disable button continue if not all transaction signed
  2. display warning without confirmation button, until user switches back to pristine network

screen shot 2018-08-27 at 12 32 16 pm

@mariano-aguero
Copy link
Contributor Author

@dennis00010011b

User able to finish deployment without signing all transactions

I apply the solution in Step 4 disable button continue if not all transaction signed

Can you check it ? Thanks

@fernandomg fernandomg merged commit 0cbeef0 into 2.0-steps-integration Aug 29, 2018
@ghost ghost removed the awaiting for review label Aug 29, 2018
@fernandomg fernandomg deleted the issue#1033#step4 branch August 29, 2018 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants