Merge pull request #41 from 67P/refactor/cleanup-models

Cleanup controller, templates and service

app/components/proposal-list/template.hbs

@@ -2,9 +2,11 @@
   <li data-proposal-id={{proposal.id}} title="({{proposal.kind}}) {{proposal.description}}">
     <span class="category {{proposal.kind}}">♥</span>
     <span class="amount">{{proposal.amount}}</span><span class="symbol">₭S</span>
-    for <span class="recipient">{{proposal.recipientName}}</span>
+    for <span class="recipient">{{proposal.contributor.name}}</span>
     <span class="votes">({{proposal.votesCount}}/{{proposal.votesNeeded}} votes)</span>
 
-    {{#unless proposal.executed}}<button {{action "confirm" proposal.id}}>+1</button>{{/unless}}
+    {{#unless proposal.isExecuted}}
+      <button {{action "confirm" proposal.id}}>+1</button>
+    {{/unless}}
   </li>
 {{/each}}

app/controllers/index.js

@@ -1,51 +1,10 @@
 import Ember from 'ember';
-import Proposal from 'kredits-web/models/proposal';
+import Controller from 'ember-controller';
+import computed, { alias, filter, filterBy, sort } from 'ember-computed';
+import injectService from 'ember-service/inject';
 
-const {
+export default Controller.extend({
-  computed,
+  kredits: injectService(),
-  inject: {
-    service
-  }
-} = Ember;
-
-export default Ember.Controller.extend({
-
-  kredits: service(),
-
-  contractInteractionEnabled: computed.alias('kredits.hasAccounts'),
-
-  proposalsOpen: function() {
-    let proposals = this.get('model.proposals')
-                        .filterBy('executed', false)
-                        .map(p => {
-                          p.set('recipientName', this.get('model.contributors').findBy('id', p.get('recipientId')).name);
-                          return p;
-                        });
-    return proposals;
-  }.property('model.proposals.[]', 'model.proposals.@each.executed', 'model.contributors.[]'),
-
-  proposalsClosed: function() {
-    let proposals = this.get('model.proposals')
-                        .filterBy('executed', true)
-                        .map(p => {
-                          p.set('recipientName', this.get('model.contributors').findBy('id', p.get('recipientId')).name);
-                          return p;
-                        });
-    return proposals;
-  }.property('model.proposals.[]', 'model.proposals.@each.executed', 'model.contributors.[]'),
-
-  proposalsSorting: ['id:desc'],
-  proposalsClosedSorted: Ember.computed.sort('proposalsClosed', 'proposalsSorting'),
-  proposalsOpenSorted: Ember.computed.sort('proposalsOpen', 'proposalsSorting'),
-
-  contributorsWithKredits: function() {
-    return this.get('model.contributors').filter(c => {
-      return c.get('balance') !== 0;
-    });
-  }.property('model.contributors.@each.balance'),
-
-  contributorsSorting: ['balance:desc'],
-  contributorsSorted: Ember.computed.sort('contributorsWithKredits', 'contributorsSorting'),
 
   init() {
     this._super(...arguments);
@@ -58,60 +17,76 @@ export default Ember.Controller.extend({
       });
   },
 
-  _handleProposalCreated(proposalId, creatorAddress, recipientAddress, amount) {
+  contributors: alias('model.contributors'),
-    if (Ember.isPresent(this.get('model.proposals')
+  contributorsWithKredits: filter('contributors', function(contributor) {
-             .findBy('id', proposalId.toNumber()))) {
+    return contributor.get('balance') !== 0;
+  }),
+  contributorsSorting: ['balance:desc'],
+  contributorsSorted: sort('contributorsWithKredits', 'contributorsSorting'),
+
+  proposals: computed('model.proposals.[]', 'contributors.[]', function() {
+    return this.get('model.proposals')
+               .map((proposal) => {
+                 let contributor = this.get('contributors')
+                                       .findBy('id', proposal.get('recipientId'));
+
+                 proposal.set('contributor', contributor);
+
+                 return proposal;
+               });
+  }),
+  proposalsOpen: filterBy('proposals', 'isExecuted', false),
+  proposalsClosed: filterBy('proposals', 'isExecuted', true),
+  proposalsSorting: ['id:desc'],
+  proposalsClosedSorted: sort('proposalsClosed', 'proposalsSorting'),
+  proposalsOpenSorted: sort('proposalsOpen', 'proposalsSorting'),
+
+  _handleProposalCreated(proposalId) {
+    // TODO: check if proposalId is already a string
+    let proposal = this.get('proposals')
+                       .findBy('id', proposalId.toString());
+    if (proposal) {
       Ember.Logger.debug('[index] proposal exists, not adding from event');
-      return false;
+      return;
     }
 
-    let proposal = Proposal.create({
+    proposal = this.get('kredits').getProposalById(proposalId);
-      id: proposalId.toNumber(),
+    this.get('proposals').pushObject(proposal);
-      creatorAddress: creatorAddress,
-      recipientAddress: recipientAddress,
-      recipientName: null,
-      votesCount: 0,
-      votesNeeded: 2,
-      amount: amount.toNumber(),
-      executed: false
-    });
-
-    this.get('model.proposals').pushObject(proposal);
   },
 
   _handleProposalExecuted(proposalId, recipientId, amount) {
-    if (this.get('model.proposals')
+    // TODO: check if proposalId is already a string
-            .findBy('id', recipientId.toNumber())
+    let proposal = this.get('proposals')
-            .get('executed')) {
+                       .findBy('id', proposalId.toString());
+
+    if (proposal.get('isExecuted')) {
       Ember.Logger.debug('[index] proposal already executed, not adding from event');
-      return false;
+      return;
     }
 
-    this.get('model.proposals')
+    proposal.setProperties({
-        .findBy('id', recipientId.toNumber())
+      'executed': true,
-        .setProperties({
+    });
-          'executed': true,
-          'votesCount': 2 // TODO use real count
-        });
 
-    this.get('model.contributors')
+    this.get('contributors')
-        .findBy('id', recipientId)
+        .findBy('id', recipientId.toString())
         .incrementProperty('balance', amount.toNumber());
   },
 
   _handleProposalVoted(proposalId, voter, totalVotes) {
-    this.get('model.proposals')
+    this.get('proposals')
-        .findBy('id', proposalId.toNumber())
+        .findBy('id', proposalId.toString())
         .setProperties({ 'votesCount': totalVotes });
   },
 
-  _handleTransfer(data) {
+  _handleTransfer(from, to, value) {
-    this.get('model.contributors')
+    value = value.toNumber();
-        .findBy('address', data.args.from)
+    this.get('contributors')
-        .incrementProperty('balance', - data.args.value.toNumber());
+        .findBy('address', from)
-    this.get('model.contributors')
+        .decrementProperty('balance', value);
-        .findBy('address', data.args.to)
+    this.get('contributors')
-        .incrementProperty('balance', data.args.value.toNumber());
+        .findBy('address', to)
+        .incrementProperty('balance', value);
   },
 
 
@@ -125,7 +100,7 @@ export default Ember.Controller.extend({
     save(contributor) {
       return this.get('kredits').addContributor(contributor)
         .then((contributor) => {
-          this.get('model.contributors').pushObject(contributor);
+          this.get('contributors').pushObject(contributor);
           return contributor;
         });
     }

app/models/proposal.js

@@ -1,4 +1,5 @@
 import EmberObject from 'ember-object';
+import { alias } from 'ember-computed';
 
 export default EmberObject.extend({
   // Contract
@@ -11,6 +12,9 @@ export default EmberObject.extend({
   executed: null,
   ipfsHash: null,
 
+  // Shortcuts
+  isExecuted: alias('executed'),
+
   // IPFS
   kind: null,
   description: null,
@@ -18,10 +22,6 @@ export default EmberObject.extend({
   url: null,
   ipfsData: '',
 
-  // Deprecated
+  // Relationships
-  recipientAddress: null,
+  contributor: null,
-  recipientName: null,
-  recipientProfile: null,
-
-  // TODO: add contributor relation
 });

app/services/kredits.js

@@ -109,24 +109,22 @@ export default Service.extend({
       });
   },
 
-  // TODO: Should be part of the service
+  buildModel(name, attributes) {
-  buildContributor(attributes) {
+    debug('[kredits] build', name, attributes);
-    debug('[kredits] buildContributor', attributes);
+    let model = getOwner(this).lookup(`model:${name}`);
 
-    let contributor = getOwner(this).lookup('model:contributor');
+    // coerce id to string
-    contributor.setProperties(attributes);
+    if (attributes.id) {
-    return contributor;
+      attributes.id = attributes.id.toString();
-  },
+    }
 
-  buildProposal(attributes) {
+    model.setProperties(attributes);
-    debug('[kredits] buildProposal', attributes);
+    return model;
-
-    let proposal = getOwner(this).lookup('model:proposal');
-    proposal.setProperties(attributes);
-    return proposal;
   },
 
   getContributorById(id) {
+    id = ethers.utils.bigNumberify(id);
+
     return this.get('contributorsContract')
       .then((contract) => contract.getContributorById(id))
       .then(this.reassembleIpfsHash)
@@ -135,7 +133,7 @@ export default Service.extend({
         let isCurrentUser = this.get('currentUserAccounts').includes(address);
 
         return {
-          id: id.toString(),
+          id: id,
           address,
           balance: balance.toNumber(),
           ipfsHash,
@@ -148,7 +146,7 @@ export default Service.extend({
         return this.fetchAndMergeIpfsData(data, ContributorSerializer);
       })
       .then((attributes) => {
-        return this.buildContributor(attributes);
+        return this.buildModel('contributor', attributes);
       });
   },
 
@@ -203,6 +201,8 @@ export default Service.extend({
   },
 
   getProposalById(id) {
+    id = ethers.utils.bigNumberify(id);
+
     return this.get('kreditsContract')
       .then((contract) => contract.proposals(id))
       .then(this.reassembleIpfsHash)
@@ -217,9 +217,9 @@ export default Service.extend({
         ipfsHash,
       }) => {
         return {
-          id: id.toString(),
+          id: id,
           creatorAddress,
-          recipientId: recipientId.toNumber(),
+          recipientId: recipientId.toString(),
           votesCount: votesCount.toNumber(),
           votesNeeded: votesNeeded.toNumber(),
           amount: amount.toNumber(),
@@ -232,7 +232,7 @@ export default Service.extend({
         return this.fetchAndMergeIpfsData(data, ContributionSerializer);
       })
       .then((attributes) => {
-        return this.buildProposal(attributes);
+        return this.buildModel('proposal', attributes);
       });
   },
 
@@ -296,7 +296,7 @@ export default Service.extend({
       })
       .then((data) => {
         debug('[kredits] add contributor response', data);
-        return this.buildContributor(attributes);
+        return this.buildModel('contributor', attributes);
       });
   },
 
@@ -333,7 +333,7 @@ export default Service.extend({
       })
       .then((data) => {
         debug('[kredits] add proposal response', data);
-        return this.buildProposal(attributes);
+        return this.buildModel('proposal', attributes);
       });
   },
 

app/templates/index.hbs

@@ -30,11 +30,13 @@
       <h2>Open Proposals</h2>
     </header>
     <div class="content">
+      {{!-- TODO: We need a better naming for kredits.hasAccounts --}}
       {{proposal-list proposals=proposalsOpenSorted
                       confirmAction="confirmProposal"
-                      contractInteractionEnabled=contractInteractionEnabled}}
+                      contractInteractionEnabled=kredits.hasAccounts}}
 
-      {{#if contractInteractionEnabled}}
+      {{!-- TODO: We need a better naming --}}
+      {{#if kredits.hasAccounts}}
         <p class="actions">
           {{#link-to 'proposals.new'}}Create new proposal{{/link-to}}
         </p>
@@ -51,7 +53,8 @@
     {{proposal-list proposals=proposalsClosedSorted
                     confirmAction="confirmProposal"}}
 
-    {{#if contractInteractionEnabled}}
+    {{!-- TODO: We need a better naming --}}
+    {{#if kredits.hasAccounts}}
       <p class="actions">
         {{#link-to 'proposals.new'}}Create new proposal{{/link-to}}
       </p>
@@ -65,11 +68,10 @@
   </header>
 
   <div class="content">
-    {{!--
+    {{#if currentUser.isCore}}
-      TODO:
+      {{add-contributor contributors=model.contributors save=(action 'save')}}
-      only show the form if currentUser.isCore else show:
+    {{else}}
       Only core team members can add new contributors. Please ask someone to set you up.
-    --}}
+    {{/if}}
-    {{add-contributor contributors=model.contributors save=(action 'save')}}
   </div>
 </section>

tests/unit/lib/kredits/serializers/contribution-test.js

@@ -7,7 +7,6 @@ module('Serializers contribution');
 
 test('#serialize returns a valid JSON-LD representation', function(assert) {
   let serialized = ContributionSerializer.serialize({
-    recipientAddress: '0xd4a64570b12da659ee4bbd41c3509b7b1f9c51ac',
     kind: 'design',
     description: 'New logo design',
     url: 'http://opensourcedesign.org',