Group same contributions without URL, make grouping more precise

tests/unit/services/kredits-test.js

@@ -23,7 +23,7 @@ module('Unit | Service | kredits', function(hooks) {
     service.set('currentBlock', 1023);
 
     const items = service.contributionsUnconfirmed;
-    assert.equal(items.length, 3);
+    assert.equal(items.length, 5);
   });
 
   test('#kreditsByContributor', function(assert) {
@@ -38,18 +38,18 @@ module('Unit | Service | kredits', function(hooks) {
 
     const c1 = kreditsByContributor.find(k => k.contributor.id == 1);
     assert.equal(c1.amountConfirmed, 11500, 'correct amount confirmed');
-    assert.equal(c1.amountUnconfirmed, 1500, 'correct amount unconfirmed');
-    assert.equal(c1.amountTotal, 13000, 'correct amount total');
+    assert.equal(c1.amountUnconfirmed, 5000, 'correct amount unconfirmed');
+    assert.equal(c1.amountTotal, 16500, 'correct amount total');
 
     const c2 = kreditsByContributor.find(k => k.contributor.id == 2);
     assert.equal(c2.amountConfirmed, 3000, 'correct amount confirmed');
-    assert.equal(c2.amountUnconfirmed, 0, 'correct amount unconfirmed');
-    assert.equal(c2.amountTotal, 3000, 'correct amount total');
+    assert.equal(c2.amountUnconfirmed, 1500, 'correct amount unconfirmed');
+    assert.equal(c2.amountTotal, 4500, 'correct amount total');
 
     const c3 = kreditsByContributor.find(k => k.contributor.id == 3);
     assert.equal(c3.amountConfirmed, 0, 'correct amount confirmed');
-    assert.equal(c3.amountUnconfirmed, 5000, 'correct amount unconfirmed');
-    assert.equal(c3.amountTotal, 5000, 'correct amount total');
+    assert.equal(c3.amountUnconfirmed, 6500, 'correct amount unconfirmed');
+    assert.equal(c3.amountTotal, 6500, 'correct amount total');
   });
 
   test('#contributorsSorted', function(assert) {
@@ -67,11 +67,10 @@ module('Unit | Service | kredits', function(hooks) {
 
     const grouped = service.contributionsGrouped;
 
-    // 9 contributions, with ids 1+3 sharing a url and ids 7+8 sharing a url,
-    // so 7 groups total (2 grouped + 5 singletons).
-    assert.equal(grouped.length, 7, 'produces one group per unique key plus singletons');
+    // 11 contributions: 3 groups ({1,3}, {7,8}, {10,11}) + 5 singletons
+    assert.equal(grouped.length, 8, 'produces one group per unique key plus singletons');
 
-    const ghGroup = grouped.findBy('groupId', 'https://github.com/67P/kredits-contracts/pull/196');
+    const ghGroup = grouped.findBy('groupId', contributions.findBy('id', 1).groupId);
     assert.ok(ghGroup, 'github url group exists');
     assert.ok(ghGroup.isGrouped, 'github group is flagged as grouped');
     assert.equal(ghGroup.items.length, 2, 'github group has two contributions');
@@ -81,14 +80,20 @@ module('Unit | Service | kredits', function(hooks) {
     assert.equal(ghGroup.kind, 'dev', 'github group takes kind from first item');
     assert.equal(ghGroup.url, 'https://github.com/67P/kredits-contracts/pull/196');
 
-    const giteaGroup = grouped.findBy('groupId', 'https://gitea.kosmos.org/kredits/kredits-web/issues/231');
+    const giteaGroup = grouped.findBy('groupId', contributions.findBy('id', 7).groupId);
     assert.ok(giteaGroup, 'gitea url group exists');
     assert.ok(giteaGroup.isGrouped, 'gitea group is flagged as grouped');
     assert.deepEqual(giteaGroup.contributorIds, [3, 1], 'gitea group lists both contributor ids');
-    assert.equal(giteaGroup.totalAmount, 6500, 'gitea group sums amounts (5000 + 1500)');
+    assert.equal(giteaGroup.totalAmount, 10000, 'gitea group sums amounts (5000 + 5000)');
+
+    const manualGroup = grouped.findBy('groupId', contributions.findBy('id', 10).groupId);
+    assert.ok(manualGroup, 'manual (no-url) group exists');
+    assert.ok(manualGroup.isGrouped, 'manual group is flagged as grouped');
+    assert.deepEqual(manualGroup.contributorIds, [2, 3], 'manual group lists both contributor ids');
+    assert.equal(manualGroup.totalAmount, 3000, 'manual group sums amounts (1500 + 1500)');
   });
 
-  test('#contributionsGrouped treats manual contributions without url as singletons', function(assert) {
+  test('#contributionsGrouped treats contributions without enough fields as singletons', function(assert) {
     let service = this.owner.lookup('service:kredits');
     service.set('contributors', contributors);
     service.set('contributions', contributions);
@@ -96,11 +101,10 @@ module('Unit | Service | kredits', function(hooks) {
     const grouped = service.contributionsGrouped;
     const singletons = grouped.filterBy('isGrouped', false);
 
-    // 5 contributions without a url: ids 2, 4, 5, 6, 9
-    assert.equal(singletons.length, 5, 'one singleton group per url-less contribution');
+    // 5 singletons: ids 2, 4, 5, 6, 9 (each has unique description/date/amount/kind)
+    assert.equal(singletons.length, 5, 'one singleton group per non-grouped contribution');
     singletons.forEach(g => {
       assert.equal(g.items.length, 1, 'singleton group has exactly one item');
-      assert.notOk(g.groupId, 'singleton group has a null/empty groupId');
     });
   });
 
@@ -115,13 +119,13 @@ module('Unit | Service | kredits', function(hooks) {
     assert.equal(siblings[0].id, 3, 'the sibling is contribution 3');
   });
 
-  test('#siblingContributions returns empty for manual contributions', function(assert) {
+  test('#siblingContributions returns empty for contributions without siblings', function(assert) {
     let service = this.owner.lookup('service:kredits');
     service.set('contributors', contributors);
     service.set('contributions', contributions);
 
     const c2 = contributions.findBy('id', 2);
-    assert.deepEqual(service.siblingContributions(c2), [], 'no siblings for url-less contribution');
+    assert.deepEqual(service.siblingContributions(c2), [], 'no siblings for unique contribution');
   });
 
   test('#siblingContributions returns empty for null contribution', function(assert) {

tests/unit/utils/contribution-grouping-key-test.js

@@ -2,43 +2,94 @@ import { module, test } from 'qunit';
 import EmberObject from '@ember/object';
 import contributionGroupingKey from 'kredits-web/utils/contribution-grouping-key';
 
-function makeContribution (url) {
-  return EmberObject.create({ url });
+function makeContribution (attrs) {
+  return EmberObject.create(attrs);
 }
 
+const baseAttrs = {
+  url: 'https://github.com/67P/kredits-contracts/pull/196',
+  description: 'Chore/dependency updates',
+  date: '2020-05-27',
+  amount: 1500,
+  kind: 'dev'
+};
+
 module('Unit | Utils | contribution-grouping-key', function() {
 
-  test('returns the url when present', function(assert) {
-    const c = makeContribution('https://github.com/67P/kredits-contracts/pull/196');
-    assert.equal(contributionGroupingKey(c), 'https://github.com/67P/kredits-contracts/pull/196');
+  test('returns a 6-char hex string when all fields are present', function(assert) {
+    const c = makeContribution(baseAttrs);
+    const key = contributionGroupingKey(c);
+    assert.equal(key.length, 6, '6 chars');
+    assert.ok(/^[0-9a-f]{6}$/.test(key), 'is lowercase hex: ' + key);
   });
 
-  test('returns the same key for identical github urls', function(assert) {
-    const a = makeContribution('https://github.com/67P/kredits-contracts/pull/196');
-    const b = makeContribution('https://github.com/67P/kredits-contracts/pull/196');
+  test('returns the same key for identical contributions with url', function(assert) {
+    const a = makeContribution(baseAttrs);
+    const b = makeContribution(baseAttrs);
     assert.equal(contributionGroupingKey(a), contributionGroupingKey(b));
   });
 
-  test('returns the same key for identical gitea urls', function(assert) {
-    const a = makeContribution('https://gitea.kosmos.org/kredits/kredits-web/issues/231');
-    const b = makeContribution('https://gitea.kosmos.org/kredits/kredits-web/issues/231');
+  test('returns the same key for identical contributions without url', function(assert) {
+    const attrs = { ...baseAttrs, url: null };
+    const a = makeContribution(attrs);
+    const b = makeContribution(attrs);
     assert.equal(contributionGroupingKey(a), contributionGroupingKey(b));
   });
 
-  test('returns different keys for different urls', function(assert) {
-    const a = makeContribution('https://github.com/67P/kredits-contracts/pull/196');
-    const b = makeContribution('https://github.com/67P/kredits-contracts/pull/197');
+  test('returns different keys when url differs', function(assert) {
+    const a = makeContribution(baseAttrs);
+    const b = makeContribution({ ...baseAttrs, url: 'https://github.com/67P/kredits-contracts/pull/197' });
     assert.notEqual(contributionGroupingKey(a), contributionGroupingKey(b));
   });
 
-  test('returns null when url is empty', function(assert) {
-    const c = makeContribution('');
+  test('returns different keys when amount differs', function(assert) {
+    const a = makeContribution(baseAttrs);
+    const b = makeContribution({ ...baseAttrs, amount: 5000 });
+    assert.notEqual(contributionGroupingKey(a), contributionGroupingKey(b));
+  });
+
+  test('returns different keys when kind differs', function(assert) {
+    const a = makeContribution(baseAttrs);
+    const b = makeContribution({ ...baseAttrs, kind: 'ops' });
+    assert.notEqual(contributionGroupingKey(a), contributionGroupingKey(b));
+  });
+
+  test('returns different keys when description differs', function(assert) {
+    const a = makeContribution(baseAttrs);
+    const b = makeContribution({ ...baseAttrs, description: 'Something else' });
+    assert.notEqual(contributionGroupingKey(a), contributionGroupingKey(b));
+  });
+
+  test('returns different keys when date differs', function(assert) {
+    const a = makeContribution(baseAttrs);
+    const b = makeContribution({ ...baseAttrs, date: '2020-05-28' });
+    assert.notEqual(contributionGroupingKey(a), contributionGroupingKey(b));
+  });
+
+  test('returns different keys for url vs no-url with otherwise identical fields', function(assert) {
+    const withUrl = makeContribution(baseAttrs);
+    const withoutUrl = makeContribution({ ...baseAttrs, url: null });
+    assert.notEqual(contributionGroupingKey(withUrl), contributionGroupingKey(withoutUrl));
+  });
+
+  test('returns null when description is missing', function(assert) {
+    const c = makeContribution({ ...baseAttrs, description: null });
     assert.equal(contributionGroupingKey(c), null);
   });
 
-  test('returns null when url is null/undefined', function(assert) {
-    assert.equal(contributionGroupingKey(makeContribution(null)), null);
-    assert.equal(contributionGroupingKey(makeContribution(undefined)), null);
+  test('returns null when date is missing', function(assert) {
+    const c = makeContribution({ ...baseAttrs, date: null });
+    assert.equal(contributionGroupingKey(c), null);
+  });
+
+  test('returns null when amount is missing', function(assert) {
+    const c = makeContribution({ ...baseAttrs, amount: null });
+    assert.equal(contributionGroupingKey(c), null);
+  });
+
+  test('returns null when kind is missing', function(assert) {
+    const c = makeContribution({ ...baseAttrs, kind: null });
+    assert.equal(contributionGroupingKey(c), null);
   });
 
   test('returns null when contribution is null/undefined', function(assert) {