Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Fix changer.com 404 which occurs when starting app #461

Merged
merged 2 commits into from Dec 28, 2017

Conversation

Nasicus
Copy link

@Nasicus 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/

- also update the list of supported coins from changer
@j-a-m-l
Copy link
Contributor

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
Copy link
Contributor

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

@Nasicus
Copy link
Author

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
Copy link
Contributor

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

@Nasicus
Copy link
Author

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
@luciorubeens
Copy link
Contributor

Nice! +5 👍.. Keep shapeshift for now

@Nasicus Nasicus deleted the bug/changer404 branch December 28, 2017 12:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants