Merge pull request #12706 from NejcZdovc/hotfix/#12591-error

Fixes undefined synopsis for media publishers

Author: bsclifton

app/browser/api/ledger.js

@@ -2394,7 +2394,7 @@ const onMediaRequest = (state, xhr, type, tabId) => {
   if (!cache.isEmpty()) {
     const publisherKey = cache.get('publisher')
     const publisher = ledgerState.getPublisher(state, publisherKey)
-    if (!publisher.isEmpty()) {
+    if (!publisher.isEmpty() && publisher.has('providerName')) {
       return module.exports.saveVisit(state, publisherKey, {
         duration,
         revisited,
@@ -2443,19 +2443,13 @@ const onMediaPublisher = (state, mediaKey, response, duration, revisited) => {
   const publisherURL = response.get('publisherURL')
   const providerName = response.get('providerName')
 
-  if (publisher.isEmpty()) {
-    revisited = false
+  if (!synopsis.publishers[publisherKey] || publisher.isEmpty()) {
     synopsis.initPublisher(publisherKey)
 
     if (!synopsis.publishers[publisherKey]) {
       return state
     }
 
-    synopsis.publishers[publisherKey].faviconName = faviconName
-    synopsis.publishers[publisherKey].faviconURL = faviconURL
-    synopsis.publishers[publisherKey].publisherURL = publisherURL
-    synopsis.publishers[publisherKey].providerName = providerName
-
     state = ledgerState.setPublisher(state, publisherKey, synopsis.publishers[publisherKey])
 
     if (!getSetting(settings.PAYMENTS_SITES_AUTO_SUGGEST)) {
@@ -2467,15 +2461,19 @@ const onMediaPublisher = (state, mediaKey, response, duration, revisited) => {
         savePublisherOption(publisherKey, 'exclude', exclude)
       })
     }
-  } else {
-    synopsis.publishers[publisherKey].faviconName = faviconName
-    synopsis.publishers[publisherKey].faviconURL = faviconURL
-    synopsis.publishers[publisherKey].publisherURL = publisherURL
-    synopsis.publishers[publisherKey].providerName = providerName
-    state = ledgerState.setPublishersProp(state, publisherKey, 'faviconName', faviconName)
-    state = ledgerState.setPublishersProp(state, publisherKey, 'faviconURL', faviconURL)
-    state = ledgerState.setPublishersProp(state, publisherKey, 'publisherURL', publisherURL)
-    state = ledgerState.setPublishersProp(state, publisherKey, 'providerName', providerName)
+  }
+
+  synopsis.publishers[publisherKey].faviconName = faviconName
+  synopsis.publishers[publisherKey].faviconURL = faviconURL
+  synopsis.publishers[publisherKey].publisherURL = publisherURL
+  synopsis.publishers[publisherKey].providerName = providerName
+  state = ledgerState.setPublishersProp(state, publisherKey, 'faviconName', faviconName)
+  state = ledgerState.setPublishersProp(state, publisherKey, 'faviconURL', faviconURL)
+  state = ledgerState.setPublishersProp(state, publisherKey, 'publisherURL', publisherURL)
+  state = ledgerState.setPublishersProp(state, publisherKey, 'providerName', providerName)
+
+  if (publisher.isEmpty()) {
+    revisited = false
   }
 
   const cacheObject = Immutable.Map()

test/unit/app/browser/api/ledgerTest.js

@@ -591,7 +591,8 @@ describe('ledger api unit tests', function () {
         }))
         .setIn(['ledger', 'synopsis', 'publishers', publisherKey], Immutable.fromJS({
           visits: 1,
-          duration: 1000
+          duration: 1000,
+          providerName: 'YouTube'
         }))
 
       beforeEach(function () {
@@ -698,10 +699,26 @@ describe('ledger api unit tests', function () {
           ignoreMinTime: true
         }).calledOnce)
       })
+
+      it('state for this publisher is corrupted, so we need to fetch it again', function () {
+        const badState = defaultAppState
+          .setIn(['cache', 'ledgerVideos', videoId], Immutable.fromJS({
+            publisher: publisherKey
+          }))
+          .setIn(['ledger', 'synopsis', 'publishers', publisherKey], Immutable.fromJS({
+            options: {
+              excluded: false
+            }
+          }))
+
+        ledgerApi.onMediaRequest(badState, xhr, ledgerMediaProviders.YOUTUBE, 10)
+        assert(mediaSpy.calledOnce)
+        assert(saveVisitSpy.notCalled)
+      })
     })
 
     describe('onMediaPublisher', function () {
-      let saveVisitSpy, verifiedPStub
+      let saveVisitSpy, verifiedPStub, setPublisherSpy
 
       const expectedState = Immutable.fromJS({
         cache: {
@@ -730,12 +747,18 @@ describe('ledger api unit tests', function () {
         migrations: {}
       })
 
-      before(function () {
-        verifiedPStub = sinon.stub(ledgerApi, 'verifiedP', (state, publisherKey, fn) => state)
+      const response = Immutable.fromJS({
+        publisher: publisherKey,
+        faviconName: 'Brave',
+        faviconURL: 'data:image/jpeg;base64,...',
+        publisherURL: 'https://brave.com',
+        providerName: 'Youtube'
       })
 
-      after(function () {
-        verifiedPStub.restore()
+      before(function () {
+        verifiedPStub = sinon.stub(ledgerApi, 'verifiedP', (state, publisherKey, fn) => state)
+        saveVisitSpy = sinon.spy(ledgerApi, 'saveVisit')
+        setPublisherSpy = sinon.spy(ledgerState, 'setPublisher')
       })
 
       beforeEach(function () {
@@ -748,33 +771,92 @@ describe('ledger api unit tests', function () {
               options: {
                 exclude: true
               },
-              providerName: 'Youtube'
+              providerName: 'YouTube'
             }
           }
         })
-        saveVisitSpy = sinon.spy(ledgerApi, 'saveVisit')
+      })
+
+      after(function () {
+        verifiedPStub.restore()
+        saveVisitSpy.restore()
+        setPublisherSpy.restore()
       })
 
       afterEach(function () {
         ledgerApi.setSynopsis(undefined)
-        saveVisitSpy.restore()
+        verifiedPStub.reset()
+        saveVisitSpy.reset()
+        setPublisherSpy.reset()
       })
 
       it('null case', function () {
         const result = ledgerApi.onMediaPublisher(defaultAppState)
         assert.deepEqual(result.toJS(), defaultAppState.toJS())
+        assert(setPublisherSpy.notCalled)
+        assert(saveVisitSpy.notCalled)
       })
 
-      it('create publisher if new and add cache', function () {
-        const response = Immutable.fromJS({
-          publisher: publisherKey,
-          faviconName: 'Brave',
-          faviconURL: 'data:image/jpeg;base64,...',
-          publisherURL: 'https://brave.com',
-          providerName: 'Youtube'
+      it('we do not have publisher in the synopsis', function () {
+        ledgerApi.setSynopsis({
+          initPublisher: () => {
+            ledgerApi.setSynopsis({
+              initPublisher: () => {},
+              addPublisher: () => {},
+              publishers: {
+                [publisherKey]: {
+                  exclude: false,
+                  options: {
+                    exclude: true
+                  },
+                  providerName: 'YouTube'
+                }
+              }
+            })
+          },
+          addPublisher: () => {},
+          publishers: { }
         })
 
+        const newState = Immutable.fromJS({
+          cache: {
+            ledgerVideos: {
+              'youtube_kLiLOkzLetE': {
+                publisher: 'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg',
+                faviconName: 'Brave',
+                providerName: 'Youtube',
+                faviconURL: 'data:image/jpeg;base64,...',
+                publisherURL: 'https://brave.com'
+              }
+            }
+          },
+          ledger: {
+            synopsis: {
+              publishers: {
+                'youtube#channel:UCFNTTISby1c_H-rm5Ww5rZg': {
+                  options: {
+                    exclude: true
+                  },
+                  faviconName: 'old Brave',
+                  faviconURL: 'data:image/jpeg;base64,...',
+                  publisherURL: 'https://brave.io',
+                  providerName: 'Youtube'
+                }
+              }
+            }
+          },
+          migrations: {}
+        })
+
+        const state = ledgerApi.onMediaPublisher(newState, videoId, response, 1000, false)
+        assert(saveVisitSpy.calledOnce)
+        assert(setPublisherSpy.calledTwice)
+        assert.deepEqual(state.toJS(), expectedState.toJS())
+      })
+
+      it('create publisher if new and add cache', function () {
         const state = ledgerApi.onMediaPublisher(defaultAppState, videoId, response, 1000, false)
+        assert(setPublisherSpy.calledTwice)
         assert(saveVisitSpy.calledOnce)
         assert.deepEqual(state.toJS(), expectedState.toJS())
       })
@@ -810,15 +892,8 @@ describe('ledger api unit tests', function () {
           migrations: {}
         })
 
-        const response = Immutable.fromJS({
-          publisher: publisherKey,
-          faviconName: 'Brave',
-          faviconURL: 'data:image/jpeg;base64,...',
-          publisherURL: 'https://brave.com',
-          providerName: 'Youtube'
-        })
-
         const state = ledgerApi.onMediaPublisher(newState, videoId, response, 1000, false)
+        assert(setPublisherSpy.calledOnce)
         assert(saveVisitSpy.calledOnce)
         assert.deepEqual(state.toJS(), expectedState.toJS())
       })