Change Autodiscover behaviour

[Grotax]

We often get issue reports because some valid feed url is not working due to the autodiscover feature.
The actual reason is of course that the self-reference is wrong and should be fixed by the author.

But I think for news it's easier to just try to parse the feed and if it fails to check again after autodiscover.

[Grotax]

I tested this manually with one of the reported urls and it worked just as expected.

Providing "https://thomas-leister.de/index.xml" to current master will fail with an exception that it is not a feed.
Unless you disable auto discover.

With my change https://thomas-leister.de/index.xml just works and also https://thomas-leister.de works because the autodiscover jumps in.

In that case the log looks like this:

[news] Warning: No valid feed found at URL, attempting auto discovery
POST /index.php/apps/news/feeds

[news] Warning: https://thomas-leister.de/ read error : No parser can handle this stream
POST /index.php/apps/news/feeds

[news] Error: No parser can handle this stream
POST /index.php/apps/news/feeds

[Grotax]

Seems like with Nextcloud master also some internal stuff changes that affects us, phpunit complains about return types from the query builder

1) OCA\News\Tests\Unit\Db\ItemMapperPaginatedTest::testFindAllFolderIdNull
TypeError: Mock_IExpressionBuilder_c3ef46af::isNull(): Return value must be of type OCP\DB\QueryBuilder\IQueryFunction, string returned

Also there are a lot of deprecation warnings.

[Grotax]

So the bats tests fail now because they used a bug in news to work before ...

The check if a feed already exists is done on the basis of the input url but the feed was then added based on the discovered url which in case of our test feed from nextcloud is different ☠️

I already discovered that during the creation of the api tests, where the same feed was added over and over again without any issues.
I'm not sure why the api tests still work, but I think that might be because the api detects that the feed already exists and just returns the same feed without creating a new one.
Which I think does not reduce the quality of the test.

I think though that the check if the feed already exists should be moved a bit.

[Grotax]

Turns out the tests from the comand line where never running as expected at least regarding the teardown 😃

[Grotax]

Two issues

I'm not sure what exactly the unit test that fails is supposed to do.
It is named testCreateDoesNotFindFeed and expects that the function errors with this feed already exists.

But that doesn't make sense to me.

If you wanted to actually test that you would create a feed and then create the feed again.
And expecting an exception during the second execution.

Second issue is the failing integration test, the cause is that the feed the test is trying to create is already existing, this was never an issue before because the url in the DB was different from the input url, thanks to the feed discovery.
The teardown should actually solve this by deleting all feeds after each test.
That seems not to work in that particular case.
To simplify the code I switched to the assert functions from bats.
No success in debugging it yet

[Grotax]

I will take a break and continue later to rewrite the tests

[Grotax]

looks good :)

[Grotax]

This should be squashed via GitHub :)

[anoymouserver]

Code looks good. I can't judge Bats, but it seems to work. 👍

With this, do we even need the checkbox in the UI anymore, since the discover is just the fallback anyway?

[Grotax]

I think the checkbox is not needed anymore. Can be removed in future UI updates, if we get any 😅