Fix #924

4 files changed, 108 insertions, 88 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 37a113c6f..28f59ad43 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
 owncloud-news (8.2.1)
 * **Bugfix**: Do not abort full OPML import if one feed failed, #843
+* **Bugfix**: Show error message when empty OPML file was imported, #924
 
 owncloud-news (8.2.0)
 * **Backwards incompatible change**: Move updater into separate repository at https://github.com/owncloud/news-updater and publish it on pypi
diff --git a/js/controller/SettingsController.js b/js/controller/SettingsController.js
index e9d93a1eb..2bb932aa8 100644
--- a/js/controller/SettingsController.js
+++ b/js/controller/SettingsController.js
@@ -8,76 +8,83 @@
  * @copyright Bernhard Posselt 2014
  */
 app.controller('SettingsController',
-function ($route, $q, SettingsResource, ItemResource, OPMLParser,
-          OPMLImporter, Publisher) {
-    'use strict';
+    function ($route, $q, SettingsResource, ItemResource, OPMLParser,
+              OPMLImporter, Publisher) {
+        'use strict';
+        this.isOPMLImporting = false;
+        this.isArticlesImporting = false;
+        this.opmlImportError = false;
+        this.articleImportError = false;
+        this.opmlImportEmptyError = false;
+        var self = this;
 
-    this.isOPMLImporting = false;
-    this.isArticlesImporting = false;
-    this.opmlImportError = false;
-    this.articleImportError = false;
+        var set = function (key, value) {
+            SettingsResource.set(key, value);
 
-    var set = function (key, value) {
-        SettingsResource.set(key, value);
+            if (['showAll', 'oldestFirst', 'compact'].indexOf(key) >= 0) {
+                $route.reload();
+            }
+        };
 
-        if (['showAll', 'oldestFirst', 'compact'].indexOf(key) >= 0) {
-            $route.reload();
-        }
-    };
+        this.toggleSetting = function (key) {
+            set(key, !this.getSetting(key));
+        };
 
-    this.toggleSetting = function (key) {
-        set(key, !this.getSetting(key));
-    };
+        this.getSetting = function (key) {
+            return SettingsResource.get(key);
+        };
 
-    this.getSetting = function (key) {
-        return SettingsResource.get(key);
-    };
+        this.importOPML = function (content) {
+            self.opmlImportError = false;
+            self.opmlImportEmptyError = false;
+            self.articleImportError = false;
 
-    this.importOPML = function (content) {
-        this.opmlImportError = false;
-        this.articleImportError = false;
+            try {
+                this.isOPMLImporting = false;
+                var parsedContent = OPMLParser.parse(content);
 
-        try {
-            this.isOPMLImporting = false;
-            var parsedContent = OPMLParser.parse(content);
+                var jobSize = 5;
 
-            var self = this;
-            var jobSize = 5;
+                if (parsedContent.folders.length === 0 &&
+                    parsedContent.feeds.length === 0) {
+                    self.opmlImportEmptyError = true;
+                } else {
+                    OPMLImporter.importFolders(parsedContent)
+                        .then(function (feedQueue) {
+                            return OPMLImporter.importFeedQueue(feedQueue,
+                                jobSize);
+                        }).finally(function () {
+                        self.isOPMLImporting = false;
+                    });
+                }
 
-            OPMLImporter.importFolders(parsedContent)
-            .then(function (feedQueue) {
-                return OPMLImporter.importFeedQueue(feedQueue, jobSize);
-            }).finally(function () {
-                self.isOPMLImporting = false;
-            });
+            } catch (error) {
+                this.opmlImportError = true;
+                console.error(error);
+                this.isOPMLImporting = false;
+            }
+        };
 
-        } catch (error) {
-            console.error(error);
-            this.isOPMLImporting = false;
-            this.opmlImportError = true;
-        }
-    };
-
-    this.importArticles = function (content) {
-        this.opmlImportError = false;
-        this.articleImportError = false;
+        this.importArticles = function (content) {
+            this.opmlImportError = false;
+            this.articleImportError = false;
 
-        try {
-            this.isArticlesImporting = true;
-            var articles = JSON.parse(content);
+            try {
+                this.isArticlesImporting = true;
+                var articles = JSON.parse(content);
 
-            var self = this;
-            ItemResource.importArticles(articles).success(function (data) {
-                Publisher.publishAll(data);
-            }).finally(function () {
-                self.isArticlesImporting = false;
-            });
+                var self = this;
+                ItemResource.importArticles(articles).success(function (data) {
+                    Publisher.publishAll(data);
+                }).finally(function () {
+                    self.isArticlesImporting = false;
+                });
 
-        } catch (error) {
-            console.error(error);
-            this.articleImportError = true;
-            this.isArticlesImporting = false;
-        }
-    };
+            } catch (error) {
+                console.error(error);
+                this.articleImportError = true;
+                this.isArticlesImporting = false;
+            }
+        };
 
-});
\ No newline at end of file
+    });
\ No newline at end of file
diff --git a/js/tests/unit/controller/SettingsControllerSpec.js b/js/tests/unit/controller/SettingsControllerSpec.js
index 4214f9d81..19973860e 100644
--- a/js/tests/unit/controller/SettingsControllerSpec.js
+++ b/js/tests/unit/controller/SettingsControllerSpec.js
@@ -112,7 +112,13 @@ describe('SettingsController', function () {
     OPMLParser) {
         var queue = 4;
 
-        OPMLParser.parse = jasmine.createSpy('parse').and.returnValue(2);
+        var opml = {
+            feeds: [
+                {name: 'hi'}
+            ],
+            folders: []
+        };
+        OPMLParser.parse = jasmine.createSpy('parse').and.returnValue(opml);
 
         OPMLImporter.importFolders = jasmine.createSpy('importFolders')
         .and.returnValue({
@@ -133,12 +139,12 @@ describe('SettingsController', function () {
             OPMLParser: OPMLParser
         });
 
-        var content = '{"test":1}';
+        var content = '{"folders":[{name:"b"}]}';
 
         ctrl.importOPML(content);
 
         expect(OPMLParser.parse).toHaveBeenCalledWith(content);
-        expect(OPMLImporter.importFolders).toHaveBeenCalledWith(2);
+        expect(OPMLImporter.importFolders).toHaveBeenCalledWith(opml);
         expect(OPMLImporter.importFeedQueue).toHaveBeenCalledWith(4, 5);
         expect(ctrl.isOPMLImporting).toBe(false);
         expect(ctrl.opmlImportError).toBe(false);
diff --git a/templates/part.settings.php b/templates/part.settings.php
index a85413fa1..24300eea6 100644
--- a/templates/part.settings.php
+++ b/templates/part.settings.php
@@ -1,7 +1,7 @@
 <div id="app-settings-header">
-<button name="app settings"
-    class="settings-button"
-    data-apps-slide-toggle="#app-settings-content"></button>
+    <button name="app settings"
+            class="settings-button"
+            data-apps-slide-toggle="#app-settings-content"></button>
 </div>
 
 <div id="app-settings-content">
@@ -65,33 +65,35 @@
     </p>
 
     <p>
-        <a target="_blank" href="https://github.com/owncloud/news/wiki"><?php p($l->t('Documentation')); ?></a>
+        <a target="_blank"
+           href="https://github.com/owncloud/news/wiki"><?php p($l->t('Documentation')); ?></a>
     </p>
     <p>
-        <a target="_blank" href="https://github.com/owncloud/news/issues/new"><?php p($l->t('Report a bug')); ?></a>
+        <a target="_blank"
+           href="https://github.com/owncloud/news/issues/new"><?php p($l->t('Report a bug')); ?></a>
     </p>
 
     <div class="import-export">
         <h3><?php p($l->t('Subscriptions (OPML)')); ?></h3>
 
         <input type="file"
-            id="opml-upload"
-            name="import"
-            news-read-file="Settings.importOPML($fileContent)"/>
+               id="opml-upload"
+               name="import"
+               news-read-file="Settings.importOPML($fileContent)"/>
 
         <button title="<?php p($l->t('Import')); ?>"
-            class="icon-upload svg button-icon-label"
-            news-trigger-click="#opml-upload"
-            ng-class="{'entry-loading': Settings.isOPMLImporting}"
-            ng-disabled=
+                class="icon-upload svg button-icon-label"
+                news-trigger-click="#opml-upload"
+                ng-class="{'entry-loading': Settings.isOPMLImporting}"
+                ng-disabled=
                 "Settings.isOPMLImporting || Settings.isArticlesImporting">
         </button>
 
         <a title="<?php p($l->t('Export')); ?>"
-            class="button icon-download svg button-icon-label"
-            href="<?php p(\OCP\Util::linkToRoute('news.export.opml')); ?>"
-            target="_blank"
-            ng-hide="App.isFirstRun()">
+           class="button icon-download svg button-icon-label"
+           href="<?php p(\OCP\Util::linkToRoute('news.export.opml')); ?>"
+           target="_blank"
+           ng-hide="App.isFirstRun()">
         </a>
 
         <button
@@ -106,7 +108,12 @@
                 $l->t('Error when importing: file does not contain valid OPML')
             ); ?>
         </p>
-
+        <p class="error" ng-show="Settings.opmlImportEmptyError">
+            <?php p(
+                $l->t('Error when importing: OPML is does neither contain ' .
+                      'feeds nor folders')
+            ); ?>
+        </p>
 
         <h3><?php p($l->t('Unread/Starred Articles')); ?></h3>
 
@@ -117,18 +124,17 @@
             news-read-file="Settings.importArticles($fileContent)"/>
 
         <button title="<?php p($l->t('Import')); ?>"
-            class="icon-upload svg button-icon-label"
-            ng-class="{'entry-loading': Settings.isArticlesImporting}"
-            ng-disabled=
-                "Settings.isOPMLImporting || Settings.isArticlesImporting"
-            news-trigger-click="#article-upload">
+                class="icon-upload svg button-icon-label"
+                ng-class="{'entry-loading': Settings.isArticlesImporting}"
+                ng-disabled="Settings.isOPMLImporting || Settings.isArticlesImporting"
+                news-trigger-click="#article-upload">
         </button>
 
         <a title="<?php p($l->t('Export')); ?>"
-            class="button icon-download svg button-icon-label"
-            href="<?php p(\OCP\Util::linkToRoute('news.export.articles')); ?>"
-            target="_blank"
-            ng-hide="App.isFirstRun()">
+           class="button icon-download svg button-icon-label"
+           href="<?php p(\OCP\Util::linkToRoute('news.export.articles')); ?>"
+           target="_blank"
+           ng-hide="App.isFirstRun()">
         </a>
         <button
             class="icon-download svg button-icon-label"