kredits/kredits-web
9
0
Fork 0
Code Issues 22 Pull Requests 1 Actions Releases 18 Activity

Fix kredits balance handling #104

Merged
bumi merged 3 commits from feature/handle-kredits-balances into master 2019-04-19 13:44:36 +00:00
Conversation 13 Commits 3 Files Changed 5
+34 -3
bumi commented 2019-04-19 09:43:30 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Kredits are stored on the Token contract as uint256 / bignumbers with 18
decimal points. Just like Ether and required by the ERC20 standard.
So we need to work with bignumbers and format a bignumber value here.

Kredits are stored on the Token contract as uint256 / bignumbers with 18 decimal points. Just like Ether and required by the ERC20 standard. So we need to work with bignumbers and format a bignumber value here.
bumi (Migrated from github.com) reviewed 2019-04-19 09:48:18 +00:00
app/utils/cps/kredits.js
@@ -0,0 +1,17 @@
import { computed } from '@ember/object';
bumi (Migrated from github.com) commented 2019-04-19 09:48:17 +00:00
Copy Link
Copy Source

how do I do that import here? somehow I did not get this working.

how do I do that import here? somehow I did not get this working.
raucao commented 2019-04-19 09:48:59 +00:00 (Migrated from github.com)
Copy Link
Copy Source

I completely disagree with this being added to kredits-web. We actually discussed this and agreed that it needs to be fixed in the wrapper, so that clients don't have to repeat all that implementation, and also because kredits are integers in the first place.

(Btw, @bumi, I'm trying to talk on XMPP, but it seems like you're not reading the dev channel.)

I completely disagree with this being added to kredits-web. We actually discussed this and agreed that it needs to be fixed in the wrapper, so that clients don't have to repeat all that implementation, and also because kredits are integers in the first place. (Btw, @bumi, I'm trying to talk on XMPP, but it seems like you're not reading the dev channel.)
bumi commented 2019-04-19 10:08:26 +00:00 (Migrated from github.com)
Copy Link
Copy Source

see that PR in kredits-contracts.

on the ERC20 tokens we can not make sure that kredits are always full integers.

in those lists here in kredits-web we must display the contribution kredits - not the token balances.

see that PR in kredits-contracts. on the ERC20 tokens we can not make sure that kredits are always full integers. in those lists here in kredits-web we must display the contribution kredits - not the token balances.
fsmanuel (Migrated from github.com) reviewed 2019-04-19 10:08:32 +00:00
app/utils/cps/kredits.js
@@ -0,0 +1,17 @@
import { computed } from '@ember/object';
fsmanuel (Migrated from github.com) commented 2019-04-19 10:08:31 +00:00
Copy Link
Copy Source

Without .js or a relative import: import formatKredits from '../format-kredits';

Without `.js` or a relative import: `import formatKredits from '../format-kredits';`
fsmanuel (Migrated from github.com) reviewed 2019-04-19 10:09:53 +00:00
app/services/kredits.js
@@ -109,7 +111,9 @@ export default Service.extend({
},
fsmanuel (Migrated from github.com) commented 2019-04-19 10:09:53 +00:00
Copy Link
Copy Source

No need for Promise.resolve: return formatKredits(total);

No need for `Promise.resolve`: `return formatKredits(total);`
fsmanuel (Migrated from github.com) reviewed 2019-04-19 10:10:19 +00:00
app/models/contributor.js
@@ -1,12 +1,13 @@
import { computed } from '@ember/object';
import EmberObject from '@ember/object';
import bignumber from 'kredits-web/utils/cps/bignumber';
fsmanuel (Migrated from github.com) commented 2019-04-19 10:10:19 +00:00
Copy Link
Copy Source

No longer needed, right?

No longer needed, right?
raucao commented 2019-04-19 10:11:48 +00:00 (Migrated from github.com)
Copy Link
Copy Source

on the ERC20 tokens we can not make sure that kredits are always full integers.

How so? What good is an ERC20 config property for decimal points, if we cannot configure and/or handle it the way we want? (This is not a good place to discuss that, btw. It would really be helpful to either chat or call about it.)

> on the ERC20 tokens we can not make sure that kredits are always full integers. How so? What good is an ERC20 config property for decimal points, if we cannot configure and/or handle it the way we want? (This is not a good place to discuss that, btw. It would really be helpful to either chat or call about it.)
fsmanuel (Migrated from github.com) reviewed 2019-04-19 10:22:56 +00:00
app/models/contributor.js
@@ -1,12 +1,13 @@
import { computed } from '@ember/object';
import EmberObject from '@ember/object';
import bignumber from 'kredits-web/utils/cps/bignumber';
fsmanuel (Migrated from github.com) commented 2019-04-19 10:22:56 +00:00
Copy Link
Copy Source

Ah sorry, we still need it for the id?!

Ah sorry, we still need it for the id?!
bumi commented 2019-04-19 13:09:19 +00:00 (Migrated from github.com)
Copy Link
Copy Source

With this PR kredits-web with the current contracts would work. We could deploy that to have a working version?
With the new PRs in kredits-contracts and an upcoming PR here we could switch to showing the earned kredits from contributions and not the token balances. (which imo would be more what we want in the UI right now).

then we need to plan on how we show the different balances.

With this PR kredits-web with the current contracts would work. We could deploy that to have a working version? With the new PRs in kredits-contracts and an upcoming PR here we could switch to showing the earned kredits from contributions and not the token balances. (which imo would be more what we want in the UI right now). then we need to plan on how we show the different balances.
raucao commented 2019-04-19 13:33:32 +00:00 (Migrated from github.com)
Copy Link
Copy Source

I don't think anyone objected to the merging of this PR.

I don't think anyone objected to the merging of this PR.
raucao commented 2019-04-19 13:33:57 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Kredits label missing btw.

Kredits label missing btw.
bumi (Migrated from github.com) reviewed 2019-04-19 13:43:40 +00:00
app/utils/cps/kredits.js
@@ -0,0 +1,17 @@
import { computed } from '@ember/object';
bumi (Migrated from github.com) commented 2019-04-19 13:43:40 +00:00
Copy Link
Copy Source

ahhh, probably the only option that I did not try...thx.

ahhh, probably the only option that I did not try...thx.
bumi commented 2019-04-19 13:44:18 +00:00 (Migrated from github.com)
Copy Link
Copy Source

does it create enough value for a kredits label :D

does it create enough value for a kredits label :D
Sign in to join this conversation.
Reviewers
No Reviewers
fsmanuel
bumi
Labels
Clear labels
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/kredits-web#104
Delete Branch "feature/handle-kredits-balances"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?