Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

No feedback if OPML import contains no feeds or folders #924

Closed
dbeniamine opened this issue Feb 14, 2016 · 8 comments
Closed

No feedback if OPML import contains no feeds or folders #924

dbeniamine opened this issue Feb 14, 2016 · 8 comments

Comments

@dbeniamine
Copy link

Hi,

I had troubles importing an .opml file into owncloud new application. After a while wondering why nothing was happening I realized that I was importing a wrong file which was not an opml file. Sadly Owncloud didn't tell me about my mistake.

David

@BernhardPosselt
Copy link
Contributor

Method is here: https://github.com/owncloud/news/blob/master/js/service/OPMLParser.js#L60

We should validate it, and if needed throw an exception which is caught here:
https://github.com/owncloud/news/blob/master/js/controller/SettingsController.js#L77

Should we use an alert for showing the error message?

@cosenal @jancborchardt any ideas on how to show proper feedback?

@dbeniamine
Copy link
Author

I haven't looked at the code, but when we provide a badly formatted .opml file Owncloud shows an error message near the import button, I guess it seems reasonable to show a "Bad file format" error at the same place.

@jancborchardt
Copy link
Contributor

Yep, good suggestion. A slightly better error message could be »Wrong file format. Please choose an OPML file.«?

@BernhardPosselt
Copy link
Contributor

Ok, after going through the issues I can not reproduce it. If I upload an invalid OPML file I'm getting: Error when importing: file does not contain valid OPML

@dbeniamine
Copy link
Author

It happened to me while uploading a .gpx file (export.gpx, instead of export.opml) which occurs to be a correct xml file. I guess that New parses the xml correctly but does nothing as the file does not contain any opml entries.

There is an (almost) empty gpx file (renamed export.txt because of github limitations) with which the bug occurs.

I'm running News 6.1.1 over Owncloud 8.2.2 installed from the Owncloud repositories on a Debian stable with Apache 2.4.10, Php 5.6.17-0 and Mysql 5.5.47-0.

I hope this is enough info to reproduce the bug.

@BernhardPosselt
Copy link
Contributor

Ok, so the solution is basically to show an error message if no feeds or folders are found

@BernhardPosselt BernhardPosselt changed the title No feedback on bad file format while importing OPML No feedback if OPML import contains no feeds or folders Feb 22, 2016
@dbeniamine
Copy link
Author

I agree, but I guess there are two distinct errors:

  • Bad file format (valid xml but no tag)
  • No feeds or folder found (valid xml with tag but empty)

I think that those two errors are quite different: the first is a bad manipulation from the user, while the second might come from a BUG on the tool used to generate the OMPL export.

@BernhardPosselt
Copy link
Contributor

Marking this as a junior bug because its very easy to fix and a low priority bug :)

Tips:
The parsedContent variable here: https://github.com/owncloud/news/blob/master/js/controller/SettingsController.js#L42

is a JavaScript object that has a list of feeds and folders:

{feeds: [], folders: []}

All that needs to be done is to test if both of them are of length 0 and then throw an exception (which will be caught here: https://github.com/owncloud/news/blob/master/js/controller/SettingsController.js#L76)

The error message also needs to be improved to reflect the additional erorr state: https://github.com/owncloud/news/blob/master/templates/part.settings.php#L97 by simply adding the additional failure cause or by adding a new type of error on the settings controller.

The JavaScript needs to be compiled using gulp, instructions are here https://github.com/owncloud/news/tree/master/js#javascript-development

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants