use DB connector instead of web API #158

2019-07-08T23:11:05+02:00

nikooo777 commented

2019-07-08 23:11:05 +02:00

(Migrated from github.com)

using the web API is not smart. this PR fixes that issue

tiger5226 (Migrated from github.com) requested changes 2019-07-10 03:45:38 +02:00

tiger5226 (Migrated from github.com) left a comment

We cannot remove that logging. It's important that we see that. Otherwise, I did not test this out with the app. I know you said it is pretty solid. Maybe @tzarebczan can take a look too in dev to make sure its good to go.

server/utils/chainquery/index.js

tiger5226 (Migrated from github.com) commented

2019-07-10 03:40:11 +02:00

I put this here intentionally. I need to know when they fail so I can make sure I fix any data in chainquery. We should however, make sure its not hardcoded and only production reports.

server/utils/chainquery/index.js

tiger5226 (Migrated from github.com) commented

2019-07-10 03:43:11 +02:00

I don't know if this is right brother. I needs to be pushed as a json string for it to work properly. I would need to test this. However, I think you think this is pretty solid.

kauffj (Migrated from github.com) reviewed 2019-07-10 23:50:48 +02:00

kauffj (Migrated from github.com) left a comment

Just some best practice suggestions in a file that doesn't look like it's adhering too closely to them in the first place 😛

Just some best practice suggestions in a file that doesn't look like it's adhering too closely to them in the first place :stuck_out_tongue:

server/index.js

					
				@ -9,3 +9,3 @@

				import slack from 'node-slack';

				import Slack from 'node-slack';

				require('winston-daily-rotate-file');

kauffj (Migrated from github.com) commented

2019-07-10 23:45:01 +02:00

const is technically more appropriate than let here but it doesn't really matter

`const` is _technically_ more appropriate than `let` here but it doesn't really matter

server/utils/chainquery/index.js

kauffj (Migrated from github.com) commented

2019-07-10 23:46:20 +02:00

these lines should probably go at top of file

server/utils/chainquery/index.js

kauffj (Migrated from github.com) commented

2019-07-10 23:48:14 +02:00

A slightly better pattern would be:

connection = getConnection()

function getConnection() {
  //instantiate singleton here
}

A slightly better pattern would be: `connection = getConnection()` ``` function getConnection() { //instantiate singleton here } ```

server/utils/chainquery/index.js

kauffj (Migrated from github.com) commented

2019-07-10 23:49:32 +02:00

Not strictly necessary but you can use Array.map for this:

const claims = results.map(r => {
          id                : r.id,
          name              : r.name,
          channel           : r.channel,
          bid_state         : r.bid_state,
          effective_amount  : r.effective_amount,
          certificate_amount: r.certificate_amount,
          claimId           : r.claimId,
          value             : r.value
})

Not strictly necessary but you can use `Array.map` for this: ``` const claims = results.map(r => { id : r.id, name : r.name, channel : r.channel, bid_state : r.bid_state, effective_amount : r.effective_amount, certificate_amount: r.certificate_amount, claimId : r.claimId, value : r.value }) ```