Fix unread feed selection in FoldersAndFeedsQueryBuilder

[christophehenry]

Fixes #255.

[codecov]

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes missing coverage. Please review.

Project coverage is 18.71%. Comparing base (b12d675) to head (614839b).
Report is 42 commits behind head on develop.

Files with missing lines	Patch %	Lines
...readrops/db/queries/FoldersAndFeedsQueryBuilder.kt	64.70%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop     #276      +/-   ##
=============================================
- Coverage      19.61%   18.71%   -0.91%     
- Complexity       452      458       +6     
=============================================
  Files            190      189       -1     
  Lines          10089    10234     +145     
  Branches        1577     1595      +18     
=============================================
- Hits            1979     1915      -64     
- Misses          7992     8212     +220     
+ Partials         118      107      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Shinokuni]

So I tested your solution and it didn't work for me, I still had some feeds and folders with 0 unread items displayed in the drawer.

And the catch is here. ItemState table isn't a perfect representation of the remote database. During synchronization only a limited read/unread ids are fetched. That's why when no state for an item is available in ItemState, we consider this item read. So in AND (ItemState.read = 0 OR ItemState.read IS NULL), ItemState.read IS NULL shouldn't be here to get only feeds with unread items whose read state is available in ItemState.

[Shinokuni]

I'm adding this in addition to the comment just above. I had written a test in GetFoldersWithFeedsTest, could you add some to cover hideReadFeeds and useSeparateState cases? Currently there is only one test which covers useSeparateState=false and hideReadFeeds=false.

Lastly, I haven't forgotten about #263. To answer to your last comment, please know I'm open to optimization and simplification. It's just that when you opened the PR, as stated in here, your solution wasn't satisfying enough. But now you tell me that you have iterated on it and consider meeting my requirements, I will take a look at it.

Please also note that as Readrops 2.1 is around the corner, it is currently in feature freeze state. I would like to wait for the release before merging any work of this kind.

[christophehenry]

So I tested your solution and it didn't work for me, I still had some feeds and folders with 0 unread items displayed in the drawer.

I'm sorry to read that. Unfortunately, in the current state of the code, I can't progres any further. The creation of this SQL query is too complex for me. I really encourage you to look at #263 to simplify this part. You may not need the database view. That was just me experimenting with that solution. But at least look at that SQL query which I belive is the simpler and more optimized way to select things related to feeds which have unread items. Parts of this SQL query may search in a template string for other queries.

[Shinokuni]

I'm sorry to read that. Unfortunately, in the current state of the code, I can't progres any further. The creation of this SQL query is too complex for me.

Oh, I should have been more clearer. Your solution works if you remove ItemState.read IS NULL from if(useSeparateState) append("AND (ItemState.read = 0 OR ItemState.read IS NULL)") in FoldersAndFeedsQueryBuilder. That's why I made a review on that specific line. Could you just modify this in order to merge?

Otherwise I understand if you don't want to work on this subject anymore, I will handle tests myself.

[christophehenry]

Ah right, sorry. Done. I added a test too.

[Shinokuni]

Alright, thanks for this!