From 33a429c4484bed533397fc3fb01eef1950d9bd67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 26 Jul 2026 17:22:53 +0200 Subject: [PATCH] Group same contributions without URL, make grouping more precise --- app/models/contribution.js | 7 +- app/utils/contribution-grouping-key.js | 25 ++++-- tests/fixtures/contributions.js | 20 +++-- .../contribution-list/component-test.js | 15 ++-- tests/unit/services/kredits-test.js | 42 +++++---- .../utils/contribution-grouping-key-test.js | 89 +++++++++++++++---- 6 files changed, 135 insertions(+), 63 deletions(-) diff --git a/app/models/contribution.js b/app/models/contribution.js index 21f862a..75bd7fe 100644 --- a/app/models/contribution.js +++ b/app/models/contribution.js @@ -47,9 +47,10 @@ export default EmberObject.extend({ }), // Stable key shared by all per-contributor contributions created for the - // same issue/pull request (derived from `url`). Manual contributions without - // a URL return null and are treated as singletons (not grouped). - groupId: computed('url', function() { + // same piece of work (derived from `url`, `description`, `date`, `amount`, + // and `kind`). Contributions without enough fields to derive a key return + // null and are treated as singletons (not grouped). + groupId: computed('url', 'description', 'date', 'amount', 'kind', function() { return contributionGroupingKey(this); }), diff --git a/app/utils/contribution-grouping-key.js b/app/utils/contribution-grouping-key.js index e036ab7..92763cc 100644 --- a/app/utils/contribution-grouping-key.js +++ b/app/utils/contribution-grouping-key.js @@ -1,14 +1,29 @@ +import ethers from 'ethers'; import { isEmpty } from '@ember/utils'; // Derives a stable grouping key for a contribution, so that contributions // created for the same issue/pull request (one per contributor) can be // linked together in the UI. // -// Until we have identical IPFS documents for different contributions, we will -// use the URL as grouping key +// The key is a 6-char keccak256 prefix of a combination of fields: +// url (optional), description, date, amount, kind. When the URL is present +// (bot contributions) it is included; when absent (manual contributions) +// the remaining fields still produce a reliable key. All of description, +// date, amount, and kind must be present — if any is missing, the +// contribution is treated as a singleton (null key). export default function contributionGroupingKey (contribution) { - let url = contribution ? contribution.url : null; - if (isEmpty(url)) return null; - return url; + if (!contribution) return null; + + const { url, description, date, amount, kind } = contribution; + + if (isEmpty(description) || isEmpty(date) || isEmpty(amount) || isEmpty(kind)) { + return null; + } + + const input = isEmpty(url) + ? `${description}|${date}|${amount}|${kind}` + : `${url}|${description}|${date}|${amount}|${kind}`; + + return ethers.utils.id(input).slice(2, 8); } diff --git a/tests/fixtures/contributions.js b/tests/fixtures/contributions.js index 3af9620..8a22cac 100644 --- a/tests/fixtures/contributions.js +++ b/tests/fixtures/contributions.js @@ -5,15 +5,17 @@ import processContributionData from 'kredits-web/utils/process-contribution-data const items = []; const data = [ - { id: 1, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: false, amount: 1500, kind: 'dev', url: 'https://github.com/67P/kredits-contracts/pull/196' }, - { id: 2, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: false, amount: 5000, kind: 'ops' }, - { id: 3, contributorId: 2, confirmedAt: 1000, claimed: false, vetoed: false, amount: 1500, kind: 'ops', url: 'https://github.com/67P/kredits-contracts/pull/196' }, - { id: 4, contributorId: 2, confirmedAt: 1000, claimed: false, vetoed: false, amount: 1500, kind: 'docs' }, - { id: 5, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: false, amount: 5000, kind: 'design' }, - { id: 6, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: true, amount: 500, kind: 'dev' }, - { id: 7, contributorId: 3, confirmedAt: 2000, claimed: false, vetoed: false, amount: 5000, kind: 'dev', url: 'https://gitea.kosmos.org/kredits/kredits-web/issues/231' }, - { id: 8, contributorId: 1, confirmedAt: 2000, claimed: false, vetoed: false, amount: 1500, kind: 'community', url: 'https://gitea.kosmos.org/kredits/kredits-web/issues/231' }, - { id: 9, contributorId: 3, confirmedAt: 2000, claimed: false, vetoed: true, amount: 1500, kind: 'docs' }, + { id: 1, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: false, amount: 1500, kind: 'dev', url: 'https://github.com/67P/kredits-contracts/pull/196', description: 'Chore/dependency updates', date: '2020-05-27' }, + { id: 2, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: false, amount: 5000, kind: 'ops', description: 'Server maintenance', date: '2020-05-27' }, + { id: 3, contributorId: 2, confirmedAt: 1000, claimed: false, vetoed: false, amount: 1500, kind: 'dev', url: 'https://github.com/67P/kredits-contracts/pull/196', description: 'Chore/dependency updates', date: '2020-05-27' }, + { id: 4, contributorId: 2, confirmedAt: 1000, claimed: false, vetoed: false, amount: 1500, kind: 'docs', description: 'API documentation', date: '2020-05-28' }, + { id: 5, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: false, amount: 5000, kind: 'design', description: 'UI redesign', date: '2020-05-29' }, + { id: 6, contributorId: 1, confirmedAt: 1000, claimed: false, vetoed: true, amount: 500, kind: 'dev', description: 'Minor fix', date: '2020-05-30' }, + { id: 7, contributorId: 3, confirmedAt: 2000, claimed: false, vetoed: false, amount: 5000, kind: 'dev', url: 'https://gitea.kosmos.org/kredits/kredits-web/issues/231', description: 'Grouped contributions UI', date: '2020-06-01' }, + { id: 8, contributorId: 1, confirmedAt: 2000, claimed: false, vetoed: false, amount: 5000, kind: 'dev', url: 'https://gitea.kosmos.org/kredits/kredits-web/issues/231', description: 'Grouped contributions UI', date: '2020-06-01' }, + { id: 9, contributorId: 3, confirmedAt: 2000, claimed: false, vetoed: true, amount: 1500, kind: 'docs', description: 'Minor docs fix', date: '2020-06-02' }, + { id: 10, contributorId: 2, confirmedAt: 2000, claimed: false, vetoed: false, amount: 1500, kind: 'dev', description: 'Pair programming session', date: '2020-06-03' }, + { id: 11, contributorId: 3, confirmedAt: 2000, claimed: false, vetoed: false, amount: 1500, kind: 'dev', description: 'Pair programming session', date: '2020-06-03' }, ]; data.forEach(attrs => { diff --git a/tests/integration/components/contribution-list/component-test.js b/tests/integration/components/contribution-list/component-test.js index 2bb80b7..febf8e6 100644 --- a/tests/integration/components/contribution-list/component-test.js +++ b/tests/integration/components/contribution-list/component-test.js @@ -15,9 +15,9 @@ module('Integration | Component | contribution-list', function(hooks) { this.set('fixtures', contributions); await render(hbs`{{contribution-list contributions=fixtures}}`); - // 9 contributions: 2 groups (ids 1+3, 7+8) + 5 singletons = 7 top-level rows - assert.equal(this.element.querySelectorAll('li').length, 7, '7 top-level rows when collapsed'); - assert.equal(this.element.querySelectorAll('li.grouped').length, 2, '2 grouped headers'); + // 11 contributions: 3 groups ({1,3}, {7,8}, {10,11}) + 5 singletons = 8 rows + assert.equal(this.element.querySelectorAll('li').length, 8, '8 top-level rows when collapsed'); + assert.equal(this.element.querySelectorAll('li.grouped').length, 3, '3 grouped headers'); assert.equal(this.element.querySelectorAll('li.group-item').length, 0, 'no expanded sub-items'); }); @@ -36,16 +36,15 @@ module('Integration | Component | contribution-list', function(hooks) { await click('.filter-contribution-size input'); // Hide small (<=500): drops id 6 only. Grouped rows stay (matching items - // are 1500/1500). 4 top-level rows. + // are 1500/5000). 4 top-level rows. assert.equal(this.element.querySelectorAll('li').length, 4, 'hide small contributions'); await fillIn('.filter-contribution-kind select', 'dev'); // Kind dev (still filtered to contributor 1, hide small on): only groups // where a single item passes all three filters survive. Group {1,3}: id 1 - // is contributor 1 + dev → included. Group {8,7}: id 8 is contributor 1 but - // community; id 7 is dev but contributor 3 → no item passes both → excluded. - // Singletons id 2 (ops), id 5 (design) → excluded. 1 top-level row. - assert.equal(this.element.querySelectorAll('li').length, 1, 'select kind'); + // is contributor 1 + dev → included. Group {7,8}: id 8 is contributor 1 + + // dev → included. Singletons id 2 (ops), id 5 (design) → excluded. 2 rows. + assert.equal(this.element.querySelectorAll('li').length, 2, 'select kind'); }); test('filtering by contributor keeps grouped rows whole', async function(assert) { diff --git a/tests/unit/services/kredits-test.js b/tests/unit/services/kredits-test.js index 6087acd..276b888 100644 --- a/tests/unit/services/kredits-test.js +++ b/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) { diff --git a/tests/unit/utils/contribution-grouping-key-test.js b/tests/unit/utils/contribution-grouping-key-test.js index 33e720e..45e649e 100644 --- a/tests/unit/utils/contribution-grouping-key-test.js +++ b/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) {