Skip to content
This repository has been archived by the owner. It is now read-only.

Do not show delegate registration form for a delegate - Closes #946 #982

Merged
merged 2 commits into from Nov 20, 2017

Conversation

Projects
None yet
4 participants
@joerodrig
Copy link
Contributor

commented Nov 16, 2017

What was the problem?

An existing delegate was able to navigate directly to the register delegate dialog at /main/voting/register-delegate and view the delegate registration form.

How did I fix it?

An informational dialog is displayed stating that the user is already a delegate, and the registration form is no longer shown.

How to test it?

Review checklist

  • The PR solves #946
  • All new code is covered with unit tests
  • All new features are covered with e2e tests. No new features added
  • All new code follows best practices

joerodrig added some commits Nov 16, 2017

If a user is already a delegate, don't display the form
Solves #946.

Initially I attempted a redirect to the root page here so the modal
wouldn’t load if the user is a delegate. Due to the way this dialog is
opened though, that caused the dialog to quickly animate onto the page,
then disappear after the redirect was triggered. This is a quick
compromise for the UX.
Update delegate registration specs
Specs updated to

1) Confirm the InfoParagraph component is loaded when a delegate exists
2) Confirm that the delegate registration form isn’t rendered if the
account is already a delegate

@slaweet slaweet requested a review from ginacontrino Nov 16, 2017

@ginacontrino ginacontrino changed the base branch from development to 1.3.0 Nov 17, 2017

@reyraa reyraa changed the title #946 Do not show delegate registration form for a delegate Do not show delegate registration form for a delegate - Closes #946 Nov 20, 2017

@ginacontrino
Copy link
Contributor

left a comment

Thank you for your changes! :) Good job

@ginacontrino ginacontrino merged commit d248a23 into LiskArchive:1.3.0 Nov 20, 2017

3 checks passed

Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
coverage/coveralls First build on PR-982 at 91.889%
Details

@karmacoma karmacoma added the ready label Nov 14, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.