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 changer.com 404 which occurs when starting app #461

Merged
merged 2 commits into from Dec 28, 2017

Conversation

Projects
None yet
3 participants
@Nasicus
Contributor

Nasicus commented Dec 15, 2017

There's an annoying error which always occurs when starting the app and you have the dev tools open (it's not a real problem for the user, however doing calls which we know will result in a 404 should in my opinion be prevented):

image

The reason the error always occurs is because at app start we always want to check the rate for ARK vs BTC. However changer doesn't support Ark.

There are multiple solutions to this:

  • Remove changer service and all calls to it completely since the service is in the whole code NEVER used successfully (it's always used with Ark and Ark isn't supported by Changer)
    • This would be my preferred approach, however I thought it would be a little "bold" of me if I just do that, so I thought I'd do a gentler approach (but let me know if you think this would acutally be the way to go)
  • Don't call the changer service anymore
  • Let the service internally see if he supports the passed in coin types and if not just return a "reject" promise
    • that's what I did because I felt like this is a clean way and doesn't feel like a hack

Note that while fixing this I also updated the supported currencies according to:
https://www.changer.com/api/

fix changer.com 404 which occurs when starting app
- also update the list of supported coins from changer
@j-a-m-l

This comment has been minimized.

Member

j-a-m-l commented Dec 19, 2017

@luciorubeens what do you think?

The "Exchange" feature haven't been enabled for a "long" time, so maybe we could remove it entirely or replace Changer by other service.

@luciorubeens

This comment has been minimized.

Member

luciorubeens commented Dec 27, 2017

Yeah, so let's remove the changer service completely.

@Nasicus

This comment has been minimized.

Contributor

Nasicus commented Dec 27, 2017

@luciorubeens
Do you want to keep the exchange tab? Or should I delete it too?

Also if you guys sell get rid of "changer" completely: Can I delete everything concerning exchanges? :P A lot of methods do not make "sense" anymore if the changerService is gone.

For example. the sell function completely relies on the changer service:

    self.sell = function () {
      if (self.exchangeEmail) storageService.set('email', self.exchangeEmail)
      changerService.makeExchange(self.exchangeEmail, self.sellAmount, 'ark_ARK', self.selectedCoin, self.recipientAddress).then(function (resp) {
        accountService.createTransaction(0, {
          fromAddress: self.selected.address,
          toAddress: resp.payee,
          amount: parseInt(resp.send_amount * ARKTOSHI_UNIT),
          masterpassphrase: self.passphrase,
          secondpassphrase: self.secondpassphrase
        }).then(function (transaction) {
          console.log(transaction)

          timeService.getTimestamp().then(
            function (timestamp) {
              completeExchangeSell(timestamp)
            },
            function (timestamp) {
              completeExchangeSell(timestamp)
            }
          )
        },
          function (error) {
            formatAndToastError(error, 10000)
          })
        self.passphrase = null
        self.secondpassphrase = null
      }, function (error) {
        formatAndToastError(error, 10000)
        self.exchangeSell = null
      })
    }

But if I delete this function then other functions e.g. completeExchangeSell is not used anymore (it's basically a chain-reaction ;)) and stuff like self.exchangeEmail is also not used anywhere anymore, so we could also get rid of that....

Basically no problem, just asking to make sure that "everything should be deleted".

@luciorubeens

This comment has been minimized.

Member

luciorubeens commented Dec 27, 2017

Do it! We never use this, but keep the Exchange tab because if any exchange service support us we will implement

@Nasicus

This comment has been minimized.

Contributor

Nasicus commented Dec 27, 2017

Done I think.

Just wondering: I left ShapeShifter alone since it's something different, but also not used anywhere. Do we need it? (There's a whole "widget" concerning ShapeShifter).

@luciorubeens luciorubeens merged commit fccec29 into ArkEcosystem:master Dec 28, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@luciorubeens

This comment has been minimized.

Member

luciorubeens commented Dec 28, 2017

Nice! +5 👍.. Keep shapeshift for now

@Nasicus Nasicus deleted the Nasicus:bug/changer404 branch Dec 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment