From 1801b9bda2c4fc401dc08b08bd1d93a8cdebedb2 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Fri, 16 Feb 2018 11:16:42 -0700 Subject: [PATCH] Merge pull request #13020 from NejcZdovc/feature/#11238-manual Fixes pinned values when total is not over 100 --- app/browser/api/ledger.js | 109 +++++++++++++----------- app/common/state/ledgerState.js | 2 +- test/unit/app/browser/api/ledgerTest.js | 107 ++++++++++++++++++++++- 3 files changed, 167 insertions(+), 51 deletions(-) diff --git a/app/browser/api/ledger.js b/app/browser/api/ledger.js index 9dd70508446..88da059ce11 100644 --- a/app/browser/api/ledger.js +++ b/app/browser/api/ledger.js @@ -420,70 +420,69 @@ const synopsisNormalizer = (state, changedPublisher, returnState = true, prune = state = module.exports.pruneSynopsis(state) } - let results = [] - let publishers = ledgerState.getPublishers(state) - for (let item of publishers) { - const publisherKey = item[0] - let publisher = item[1] - if (!ledgerUtil.visibleP(state, publisherKey)) { - continue - } - - publisher = publisher.set('publisherKey', publisherKey) - results.push(publisher.toJS()) - } + let publishers = ledgerState.getPublishers(state).toJS() + let pinnedTotal = 0 + let unPinnedTotal = 0 + Object.keys(publishers).forEach(publisherKey => { + let publisher = publishers[publisherKey] - if (results.length === 0) { - if (returnState) { - return ledgerState.saveAboutSynopsis(state, Immutable.List()) + if (!ledgerUtil.visibleP(state, publisherKey)) { + return } - return [] - } - - results = underscore.sortBy(results, (entry) => -entry.scores[scorekeeper]) + publisher.publisherKey = publisherKey - let pinnedTotal = 0 - let unPinnedTotal = 0 - // move publisher to the correct array and get totals - results.forEach((result) => { - if (result.pinPercentage && result.pinPercentage > 0) { + if (publisher.pinPercentage && publisher.pinPercentage > 0) { // pinned - pinnedTotal += result.pinPercentage - dataPinned.push(getPublisherData(result, scorekeeper)) - } else if (ledgerUtil.stickyP(state, result.publisherKey)) { + pinnedTotal += publisher.pinPercentage + dataPinned.push(getPublisherData(publisher, scorekeeper)) + } else if (ledgerUtil.stickyP(state, publisher.publisherKey)) { // unpinned - unPinnedTotal += result.scores[scorekeeper] - dataUnPinned.push(result) + unPinnedTotal += publisher.scores[scorekeeper] + dataUnPinned.push(publisher) } else { // excluded - let publisher = getPublisherData(result, scorekeeper) - publisher.percentage = 0 - publisher.weight = 0 - dataExcluded.push(publisher) + let newPublisher = getPublisherData(publisher, scorekeeper) + newPublisher.percentage = 0 + newPublisher.weight = 0 + dataExcluded.push(newPublisher) } }) + if ( + dataPinned.length === 0 && + dataUnPinned.length === 0 && + dataExcluded.length === 0 + ) { + if (returnState) { + return ledgerState.saveAboutSynopsis(state, Immutable.List()) + } + + return [] + } + // round if over 100% of pinned publishers if (pinnedTotal > 100) { if (changedPublisher) { - let changedObject = dataPinned.filter(publisher => publisher.publisherKey === changedPublisher)[0] // TOOD optimize to find from filter - const setOne = changedObject.pinPercentage > (100 - dataPinned.length - 1) + let changedObject = dataPinned.find(publisher => publisher.publisherKey === changedPublisher) + if (changedObject) { + const setOne = changedObject.pinPercentage > (100 - dataPinned.length - 1) - if (setOne) { - changedObject.pinPercentage = 100 - dataPinned.length + 1 - changedObject.weight = changedObject.pinPercentage - } + if (setOne) { + changedObject.pinPercentage = 100 - dataPinned.length + 1 + changedObject.weight = changedObject.pinPercentage + } - const pinnedRestTotal = pinnedTotal - changedObject.pinPercentage - dataPinned = dataPinned.filter(publisher => publisher.publisherKey !== changedPublisher) - dataPinned = normalizePinned(dataPinned, pinnedRestTotal, (100 - changedObject.pinPercentage), setOne) - dataPinned = roundToTarget(dataPinned, (100 - changedObject.pinPercentage), 'pinPercentage') + const pinnedRestTotal = pinnedTotal - changedObject.pinPercentage + dataPinned = dataPinned.filter(publisher => publisher.publisherKey !== changedPublisher) + dataPinned = module.exports.normalizePinned(dataPinned, pinnedRestTotal, (100 - changedObject.pinPercentage), setOne) + dataPinned = module.exports.roundToTarget(dataPinned, (100 - changedObject.pinPercentage), 'pinPercentage') - dataPinned.push(changedObject) + dataPinned.push(changedObject) + } } else { - dataPinned = normalizePinned(dataPinned, pinnedTotal, 100) - dataPinned = roundToTarget(dataPinned, 100, 'pinPercentage') + dataPinned = module.exports.normalizePinned(dataPinned, pinnedTotal, 100) + dataPinned = module.exports.roundToTarget(dataPinned, 100, 'pinPercentage') } dataUnPinned = dataUnPinned.map((result) => { @@ -497,8 +496,18 @@ const synopsisNormalizer = (state, changedPublisher, returnState = true, prune = state = ledgerState.changePinnedValues(state, dataPinned) } else if (dataUnPinned.length === 0 && pinnedTotal < 100) { // when you don't have any unpinned sites and pinned total is less then 100 % - dataPinned = normalizePinned(dataPinned, pinnedTotal, 100, false) - dataPinned = roundToTarget(dataPinned, 100, 'pinPercentage') + let changedObject = dataPinned.find(publisher => publisher.publisherKey === changedPublisher) + if (changedObject) { + const pinnedRestTotal = pinnedTotal - changedObject.pinPercentage + const restPercentage = 100 - changedObject.pinPercentage + dataPinned = dataPinned.filter(publisher => publisher.publisherKey !== changedPublisher) + dataPinned = module.exports.normalizePinned(dataPinned, pinnedRestTotal, restPercentage) + dataPinned = module.exports.roundToTarget(dataPinned, restPercentage, 'pinPercentage') + dataPinned.push(changedObject) + } else { + dataPinned = module.exports.normalizePinned(dataPinned, pinnedTotal, 100, false) + dataPinned = module.exports.roundToTarget(dataPinned, 100, 'pinPercentage') + } // sync app store state = ledgerState.changePinnedValues(state, dataPinned) @@ -513,7 +522,7 @@ const synopsisNormalizer = (state, changedPublisher, returnState = true, prune = }) // normalize unpinned values - dataUnPinned = roundToTarget(dataUnPinned, (100 - pinnedTotal), 'percentage') + dataUnPinned = module.exports.roundToTarget(dataUnPinned, (100 - pinnedTotal), 'percentage') } const newData = dataPinned.concat(dataUnPinned, dataExcluded) @@ -2833,6 +2842,8 @@ const getMethods = () => { deleteSynopsis, transitionWalletToBat, getNewClient, + normalizePinned, + roundToTarget, savePublisherData, pruneSynopsis, checkBtcBatMigrated, diff --git a/app/common/state/ledgerState.js b/app/common/state/ledgerState.js index 7dcffbccec4..2392ff24b37 100644 --- a/app/common/state/ledgerState.js +++ b/app/common/state/ledgerState.js @@ -359,7 +359,7 @@ const ledgerState = { publishers = makeImmutable(publishers) publishers.forEach((item) => { - const publisherKey = item.get('site') + const publisherKey = item.get('publisherKey') const pattern = urlUtil.getHostPattern(publisherKey) const percentage = item.get('pinPercentage') let newSiteSettings = siteSettings.mergeSiteSetting(state.get('siteSettings'), pattern, 'ledgerPinPercentage', percentage) diff --git a/test/unit/app/browser/api/ledgerTest.js b/test/unit/app/browser/api/ledgerTest.js index adcf604daa5..eb537e9e633 100644 --- a/test/unit/app/browser/api/ledgerTest.js +++ b/test/unit/app/browser/api/ledgerTest.js @@ -19,6 +19,7 @@ describe('ledger api unit tests', function () { let ledgerNotificationsApi let ledgerState let updateState + let ledgerUtil let isBusy = false let ledgerClient let ledgerPublisher @@ -204,6 +205,7 @@ describe('ledger api unit tests', function () { }) ledgerNotificationsApi = require('../../../../../app/browser/api/ledgerNotifications') + ledgerUtil = require('../../../../../app/common/lib/ledgerUtil') ledgerState = require('../../../../../app/common/state/ledgerState') updateState = require('../../../../../app/common/state/updateState') updater = require('../../../../../app/updater') @@ -365,12 +367,17 @@ describe('ledger api unit tests', function () { describe('prune synopsis', function () { let pruneSynopsisSpy, appActionsSpy - beforeEach(function () { + before(function () { pruneSynopsisSpy = sinon.spy(ledgerApi, 'pruneSynopsis') appActionsSpy = sinon.spy(appActions, 'onPruneSynopsis') }) afterEach(function () { + pruneSynopsisSpy.reset() + appActionsSpy.reset() + }) + + after(function () { pruneSynopsisSpy.restore() appActionsSpy.restore() }) @@ -398,6 +405,104 @@ describe('ledger api unit tests', function () { assert(appActionsSpy.calledOnce) }) }) + + describe('only pinned items with total below 100%', function () { + let normalizePinnedSpy, roundToTargetSpy, visiblePStub + + const dataState = defaultAppState + .setIn(['ledger', 'synopsis', 'publishers'], Immutable.fromJS({ + 'site1': { + options: { + exclude: false + }, + pinPercentage: 30, + scores: { + concave: 9.249426617127623, + visits: 3 + }, + weight: 30 + }, + 'site2': { + options: { + exclude: false + }, + pinPercentage: 20, + scores: { + concave: 3.249426617127623, + visits: 3 + }, + weight: 20 + }, + 'site3': { + options: { + exclude: false + }, + pinPercentage: 20, + scores: { + concave: 1.249426617127623, + visits: 3 + }, + weight: 20 + } + })) + + const expectedSate = dataState + .setIn(['ledger', 'about', 'synopsis'], Immutable.Map()) + .setIn(['ledger', 'about', 'synopsisOptions'], Immutable.Map()) + + before(function () { + normalizePinnedSpy = sinon.spy(ledgerApi, 'normalizePinned') + roundToTargetSpy = sinon.spy(ledgerApi, 'roundToTarget') + visiblePStub = sinon.stub(ledgerUtil, 'visibleP', () => true) + }) + + afterEach(function () { + normalizePinnedSpy.reset() + roundToTargetSpy.reset() + }) + + after(function () { + normalizePinnedSpy.restore() + roundToTargetSpy.restore() + visiblePStub.restore() + }) + + it('changed publisher is not known', function () { + let result = ledgerApi.synopsisNormalizer(dataState, null) + assert(normalizePinnedSpy.calledOnce) + assert(roundToTargetSpy.calledOnce) + result = result.setIn(['ledger', 'about', 'synopsis'], {}) + const newState = expectedSate + .setIn(['ledger', 'synopsis', 'publishers', 'site1', 'pinPercentage'], 43) + .setIn(['ledger', 'synopsis', 'publishers', 'site1', 'weight'], 42.857142857142854) + .setIn(['ledger', 'synopsis', 'publishers', 'site2', 'pinPercentage'], 29) + .setIn(['ledger', 'synopsis', 'publishers', 'site2', 'weight'], 28.57142857142857) + .setIn(['ledger', 'synopsis', 'publishers', 'site3', 'pinPercentage'], 28) + .setIn(['ledger', 'synopsis', 'publishers', 'site3', 'weight'], 28.57142857142857) + .setIn(['siteSettings', 'https?://site1', 'ledgerPinPercentage'], 43) + .setIn(['siteSettings', 'https?://site2', 'ledgerPinPercentage'], 29) + .setIn(['siteSettings', 'https?://site3', 'ledgerPinPercentage'], 28) + assert.deepEqual(result.toJS(), newState.toJS()) + }) + + it('changed publisher is known', function () { + let result = ledgerApi.synopsisNormalizer(dataState, 'site2') + assert(normalizePinnedSpy.calledOnce) + assert(roundToTargetSpy.calledOnce) + result = result.setIn(['ledger', 'about', 'synopsis'], {}) + const newState = expectedSate + .setIn(['ledger', 'synopsis', 'publishers', 'site1', 'pinPercentage'], 48) + .setIn(['ledger', 'synopsis', 'publishers', 'site1', 'weight'], 48) + .setIn(['ledger', 'synopsis', 'publishers', 'site2', 'pinPercentage'], 20) + .setIn(['ledger', 'synopsis', 'publishers', 'site2', 'weight'], 20) + .setIn(['ledger', 'synopsis', 'publishers', 'site3', 'pinPercentage'], 32) + .setIn(['ledger', 'synopsis', 'publishers', 'site3', 'weight'], 32) + .setIn(['siteSettings', 'https?://site1', 'ledgerPinPercentage'], 48) + .setIn(['siteSettings', 'https?://site2', 'ledgerPinPercentage'], 20) + .setIn(['siteSettings', 'https?://site3', 'ledgerPinPercentage'], 32) + assert.deepEqual(result.toJS(), newState.toJS()) + }) + }) }) describe('pruneSynopsis', function () {