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

Show correct error message when signing typed data #10328

Merged
merged 1 commit into from
Apr 14, 2020

Conversation

0x-r4bbit
Copy link
Member

Prior to this commit sign-message-completed effect would always use
the wrong-password label for error handling. We now ensure that wrong-password
is only used in case the error in question has a code 5, which is
associated to unmatching passwords.

In any other case we'll use the message attached to the error.

Fixes #8275

@0x-r4bbit 0x-r4bbit requested a review from a team as a code owner April 14, 2020 15:26
@status-github-bot
Copy link

Hey @PascalPrecht, and thank you so much for making your first pull request in status-react! ❤️ Please help us make your experience better by filling out this brief questionnaire https://goo.gl/forms/uWqNcVpVz7OIopXg2

@status-github-bot
Copy link

Pull Request Checklist

  • Docs: Updated the documentation, if affected
  • Docs: Added or updated inline comments explaining intention of the code
  • Tests: Ensured that all new UI elements have been assigned accessibility IDs
  • Tests: Signaled need for E2E tests with label, if applicable
  • Tests: Briefly described what was tested and what platforms were used
  • UI: In case of UI changes, ensured that UI matches Figma
  • UI: In case of UI changes, requested review from a Core UI designer
  • UI: In case of UI changes, included screenshots of implementation

@status-github-bot status-github-bot bot added this to REVIEW in Pipeline for QA Apr 14, 2020
@0x-r4bbit
Copy link
Member Author

Since this is a fix, I assume it'd be good to add an e2e test as well?
If this is desired, I'd love any pointers to get me started on that.

@cammellos
Copy link
Member

@PascalPrecht We don't add e2e for every fix (they are fairly slow to run as they run on a different platform on emulators), if you feel this needs testing you can add some tests in clojure (they are unit/integration tests).

In case the feature needs e2e tests, generally is the test team that writes them and maintains them.

@status-im-auto
Copy link
Member

status-im-auto commented Apr 14, 2020

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
b07e6ab #2 2020-04-14 15:43:13 ~8 min ios 📄log
b07e6ab #2 2020-04-14 15:45:52 ~12 min android-e2e 📄log
b07e6ab #2 2020-04-14 15:46:28 ~12 min android 📄log
✔️ e8620fc #3 2020-04-14 16:04:22 ~10 min ios 📦ipa 📲
✔️ e8620fc #3 2020-04-14 16:07:41 ~15 min android-e2e 📦apk 📲
✔️ e8620fc #3 2020-04-14 16:10:36 ~17 min android 📦apk 📲

@0x-r4bbit
Copy link
Member Author

0x-r4bbit commented Apr 14, 2020

In case the feature needs e2e tests, generally is the test team that writes them and maintains them.

@cammellos fair enough! Thanks for letting me know! 🙏

Prior to this commit `sign-message-completed` effect would always use
the `wrong-password` label for error handling. We now ensure that `wrong-password`
is only used in case the error in question has a code `5`, which is
associated to unmatching passwords.

In any other case we'll use the `message` attached to the `error`.

Fixes status-im#8275

Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
@flexsurfer flexsurfer merged commit 64580f0 into status-im:develop Apr 14, 2020
Pipeline for QA automation moved this from REVIEW to DONE Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Show proper error when signing typed data
4 participants