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

[FIX #1209] resources/js: wrap undefined responses from jail in a String #2584

Merged
merged 1 commit into from
Dec 11, 2017

Conversation

vitvly
Copy link
Contributor

@vitvly vitvly commented Dec 2, 2017

Addresses #1209.

Summary:

Sometimes jail returns responses as 'undefined'. These are not getting displayed in the chat screen. Adding 'undefined' as a string instead of a value ensures that it will be added to the result under :message-text key instead of being dropped. Hence, it will be displayed on screen in the chat window.

Steps to test:

  • Open Status
  • Open Console
  • Type any text that will refer to undefined endpoint, e.g. web3.abcd
  • Actual result: no response from Console
  • Expected result: should see 'undefined', same as when issuing the same command in Geth for instance

status: ready

@oskarth oskarth closed this Dec 6, 2017
@oskarth oskarth reopened this Dec 6, 2017
@oskarth
Copy link
Contributor

oskarth commented Dec 6, 2017

Hey @siphiuel and thank you for your contribution! Can you please fill out the PR template (https://github.com/status-im/status-react/blob/develop/.github/PULL_REQUEST_TEMPLATE.md)? Specifically with a link to the issue, how to test it and whether it is ready or not. Thanks!

@oskarth
Copy link
Contributor

oskarth commented Dec 6, 2017

@siphiuel

Adding 'undefined' as a string instead of a value

What about defaulting to the empty string "" instead?

@oskarth oskarth requested a review from janherich December 6, 2017 14:17
@vitvly
Copy link
Contributor Author

vitvly commented Dec 6, 2017

Whatever is best from user's perspective.

Printing 'undefined' is what Geth does in this case, so I wanted to replicate that in Console.

@janherich
Copy link
Contributor

I'm good with this as a quick fix, however, the whole console bot partially implemented in jail and partially in cljs is a mess a we will likely change it substantially once new on-boarding process and better, simplified JailAPI are finalised.

@rasom rasom merged commit 3feb43d into status-im:develop Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants