Refactored BlockedNumberController.

js/background.js

@@ -432,7 +432,7 @@
 
     try {
       await ConversationController.load();
-      BlockedNumberController.load();
+      BlockedNumberController.refresh();
     } catch (error) {
       window.log.error(
         'background.js: ConversationController failed to load:',

js/blocked_number_controller.js

@@ -1,4 +1,4 @@
-/* global , Whisper, storage, ConversationController */
+/* global , Whisper, storage */
 /* global textsecure: false */
 
 /* eslint-disable more/no-then */
@@ -13,34 +13,20 @@
   window.getBlockedNumbers = () => blockedNumbers;
 
   window.BlockedNumberController = {
-    getAll() {
-      try {
-        this.load();
-      } catch (e) {
-        window.log.warn(e);
-      }
-      return blockedNumbers;
-    },
     reset() {
       this.unblockAll();
       blockedNumbers.reset([]);
     },
-    load() {
+    refresh() {
       window.log.info('BlockedNumberController: starting initial fetch');
 
-      if (blockedNumbers.length) {
-        throw new Error('BlockedNumberController: Already loaded!');
-      }
-
       if (!storage) {
         throw new Error('BlockedNumberController: Could not load blocked numbers');
       }
 
       // Add the numbers to the collection
       const numbers = storage.getBlockedNumbers();
-      blockedNumbers.add(
-        numbers.map(number => ({ number }))
-      );
+      blockedNumbers.reset(numbers.map(number => ({ number })));
     },
     block(number) {
       const ourNumber = textsecure.storage.user.getNumber();
@@ -54,7 +40,7 @@
       storage.addBlockedNumber(number);
 
       // Make sure we don't add duplicates
-      if (blockedNumbers.getNumber(number))
+      if (blockedNumbers.getModel(number))
         return;
 
       blockedNumbers.add({ number });
@@ -63,7 +49,7 @@
       storage.removeBlockedNumber(number);
 
       // Remove the model from our collection
-      const model = blockedNumbers.getNumber(number);
+      const model = blockedNumbers.getModel(number);
       if (model) {
         blockedNumbers.remove(model);
       }

js/models/blockedNumbers.js

@@ -85,7 +85,7 @@
     comparator(m) {
       return m.get('number');
     },
-    getNumber(number) {
+    getModel(number) {
       return this.models.find(m => m.get('number') === number);
     },
   });

js/signal_protocol_store.js

@@ -864,7 +864,7 @@
       ConversationController.reset();
       BlockedNumberController.reset();
       await ConversationController.load();
-      BlockedNumberController.load();
+      BlockedNumberController.refresh();
     },
     async removeAllConfiguration() {
       await window.Signal.Data.removeAllConfiguration();

js/views/blocked_number_view.js

@@ -1,4 +1,5 @@
 /* global BlockedNumberController: false */
+/* global getBlockedNumbers: false */
 /* global Whisper: false */
 /* global storage: false */
 /* global i18n: false */
@@ -19,7 +20,8 @@
       },
       initialize() {
         storage.onready(() => {
-          this.collection = BlockedNumberController.getAll();
+          BlockedNumberController.refresh();
+          this.collection = getBlockedNumbers();
           this.listView = new Whisper.BlockedNumberListView({
             collection: this.collection,
           });

test/blocked_number_controller_test.js

@@ -5,68 +5,68 @@
 describe('Blocked Number Controller', () => {
 
   beforeEach(async () => {
+    // Purge everything manually
     const numbers = storage.getBlockedNumbers();
     numbers.forEach(storage.removeBlockedNumber);
     window.getBlockedNumbers().reset([]);
   });
 
-  describe('getAll', () => {
-    it('returns our blocked numbers', () => {
-      assert.isEmpty(BlockedNumberController.getAll().models);
-
-      BlockedNumberController.block('1');
-      BlockedNumberController.block('2');
-      assert.lengthOf(BlockedNumberController.getAll().models, 2);
-    });
-  });
-
   describe('reset', () => {
     it('clears blocked numbers', () => {
       BlockedNumberController.block('1');
-      assert.isNotEmpty(BlockedNumberController.getAll().models);
+      assert.isNotEmpty(storage.getBlockedNumbers());
+      assert.isNotEmpty(window.getBlockedNumbers().models);
 
       BlockedNumberController.reset();
-      assert.isEmpty(BlockedNumberController.getAll().models);
+      assert.isEmpty(storage.getBlockedNumbers());
+      assert.isEmpty(window.getBlockedNumbers().models);
     });
   });
 
-  describe('load', () => {
+  describe('refresh', () => {
     it('loads blocked numbers from storage', () => {
-      BlockedNumberController.load();
+      BlockedNumberController.refresh();
       assert.isEmpty(window.getBlockedNumbers().models);
 
       storage.addBlockedNumber('1');
       storage.addBlockedNumber('2');
-      BlockedNumberController.load();
+      BlockedNumberController.refresh();
       assert.lengthOf(window.getBlockedNumbers().models, 2);
     });
 
-    it('throws if we have already loaded numbers', () => {
+    it('overrides old numbers if we refresh again', () => {
+      BlockedNumberController.refresh();
+      assert.isEmpty(window.getBlockedNumbers().models);
+
+      storage.addBlockedNumber('1');
+      BlockedNumberController.refresh();
+      assert.isNotEmpty(window.getBlockedNumbers().find(m => m.get('number') === '1'));
+
+      storage.removeBlockedNumber('1');
       storage.addBlockedNumber('2');
-      BlockedNumberController.load();
-      assert.throws(() => BlockedNumberController.load(), 'BlockedNumberController: Already loaded!');
+      BlockedNumberController.refresh();
+      assert.isNotEmpty(window.getBlockedNumbers().find(m => m.get('number') === '2'));
     });
 
     it('throws if storage is invalid', () => {
       const _storage = window.storage;
       window.storage = null;
-      assert.throws(() => BlockedNumberController.load(), 'BlockedNumberController: Could not load blocked numbers');
+      assert.throws(() => BlockedNumberController.refresh(), 'BlockedNumberController: Could not load blocked numbers');
       window.storage = _storage;
     });
   });
 
   describe('block', () => {
     beforeEach(() => {
-      BlockedNumberController.load();
-      assert.isEmpty(BlockedNumberController.getAll().models);
+      BlockedNumberController.refresh();
+      assert.isEmpty(storage.getBlockedNumbers());
+      assert.isEmpty(window.getBlockedNumbers().models);
     });
 
     it('adds number to the blocked list', () => {
-      assert.isEmpty(storage.getBlockedNumbers());
-
       BlockedNumberController.block('1');
 
-      const numbers = BlockedNumberController.getAll().models;
+      const numbers = window.getBlockedNumbers().models;
       assert.lengthOf(numbers, 1);
       assert.strictEqual('1', numbers[0].get('number'))
       assert.deepEqual(['1'], storage.getBlockedNumbers());
@@ -75,33 +75,34 @@ describe('Blocked Number Controller', () => {
     it('only blocks the same number once', () => {
       BlockedNumberController.block('2');
       BlockedNumberController.block('2');
-      assert.lengthOf(BlockedNumberController.getAll().models, 1);
+      assert.lengthOf(window.getBlockedNumbers().models, 1);
       assert.deepEqual(['2'], storage.getBlockedNumbers());
     });
 
     it('does not block our own number', () => {
       BlockedNumberController.block(textsecure.storage.user.getNumber());
-      assert.isEmpty(BlockedNumberController.getAll().models);
+      assert.isEmpty(window.getBlockedNumbers().models);
       assert.isEmpty(storage.getBlockedNumbers());
     });
   });
 
   describe('unblock', () => {
     beforeEach(() => {
-      BlockedNumberController.load();
-      assert.isEmpty(BlockedNumberController.getAll().models);
+      BlockedNumberController.refresh();
+      assert.isEmpty(storage.getBlockedNumbers());
+      assert.isEmpty(window.getBlockedNumbers().models);
     });
 
     it('removes number from the blocked list', () => {
       BlockedNumberController.block('1');
       BlockedNumberController.block('2');
 
-      assert.lengthOf(BlockedNumberController.getAll().models, 2);
+      assert.lengthOf(window.getBlockedNumbers().models, 2);
       assert.lengthOf(storage.getBlockedNumbers(), 2);
 
       BlockedNumberController.unblock('1');
 
-      const numbers = BlockedNumberController.getAll().models;
+      const numbers = window.getBlockedNumbers().models;
       assert.lengthOf(numbers, 1);
       assert.isEmpty(numbers.filter(n => n.get('number') === '1'));
       assert.deepEqual(['2'], storage.getBlockedNumbers());
@@ -122,25 +123,25 @@ describe('Blocked Number Controller', () => {
 
   describe('unblockAll', () => {
     it('removes all our blocked numbers', () => {
-      BlockedNumberController.load();
+      BlockedNumberController.refresh();
 
       BlockedNumberController.block('1');
       BlockedNumberController.block('2');
       BlockedNumberController.block('3');
 
-      assert.lengthOf(BlockedNumberController.getAll().models, 3);
+      assert.lengthOf(window.getBlockedNumbers().models, 3);
       assert.lengthOf(storage.getBlockedNumbers(), 3);
 
       BlockedNumberController.unblockAll();
 
-      assert.lengthOf(BlockedNumberController.getAll().models, 0);
+      assert.lengthOf(window.getBlockedNumbers().models, 0);
       assert.lengthOf(storage.getBlockedNumbers(), 0);
     });
   });
 
   describe('isBlocked', () => {
     it('returns whether a number is blocked', () => {
-      BlockedNumberController.load();
+      BlockedNumberController.refresh();
 
       BlockedNumberController.block('1');
       assert.isOk(BlockedNumberController.isBlocked('1'));