Add feature to Group starred Items per Feed #3148

Juri-w · 2025-04-20T19:06:03Z

Summary

Add Ability to Group Starred Items per Feed. Grouped Feeds are visible under the collapse of starred items. Per Grouped Feed a count of Starred Items is listed.

Checklist

Code is properly formatted
Sign-off message is added to all commits

wofferl

Thank you for your contribution.

Please take a look at the suggested changes and fix the linter errors that npm run lint shows.
A changelog entry should also be added.

src/routes/index.ts

src/components/Sidebar.vue

wofferl

I still see minor problems with the feature.

As I already mentioned, the feed counters are initially not correct and only show the starred items that have already been loaded. With a larger number of starred items, these counters are than counted up while you are going through these feeds.

For the existing routes, the counters are synchronised with the database and are therefore correct from the start, so I don't think this would actual possible for the starred feeds without changes to the backend.

Another problem is that the scroll position is currently not reset, when switching between starred feeds, which is due to the fact that it is not a real route and does not have its own fetchKey.

The open status also cannot be saved, because it is not a real folder.

I think we can live with the latter, I'm not sure whether it would be better to omit the counters, but the scroll reset would be necessary.

@Grotax @SMillerDev your opinions on this?

Juri-w · 2025-04-21T23:27:37Z

For the existing routes, the counters are synchronised with the database and are therefore correct from the start, so I don't think this would actual possible for the starred feeds without changes to the backend.

The number of starred items is loaded as soon as the view opens; once retrieved, they’re grouped by the starred‑item collection from the store, so they display correctly immediately. Feed icons and names load afterward, which means the starred list initially appears without icons or titles until the Feeds data is fully loaded.

I’ve also fixed the scroll‑reset issue.

src/components/Sidebar.vue

src/components/routes/Starred.vue

wofferl · 2025-04-22T14:26:34Z

For the existing routes, the counters are synchronised with the database and are therefore correct from the start, so I don't think this would actual possible for the starred feeds without changes to the backend.

The number of starred items is loaded as soon as the view opens; once retrieved, they’re grouped by the starred‑item collection from the store, so they display correctly immediately. Feed icons and names load afterward, which means the starred list initially appears without icons or titles until the Feeds data is fully loaded.

I’ve also fixed the scroll‑reset issue.

Bildschirmaufnahme_20250422_160104.webm

This clip demonstrates the problem. As you can see, there are a total of 142 starred articles, if you now open the starred feeds, you will see three feeds which, according to the counters, contain exactly 40 articles together. These are the 40 articles that are initially retrieved from the database by default, sorted by date. When you open the first feed and select the only article loaded, 40 more articles are requested from the database, again sorted by date regardless of the feed. Now older articles from the Arch Linux feed also appear. However, since this feed only has a lot of older articles, nothing happens here despite forced triggering of the scroll event using the mouse wheel, because the next batch of articles 81-120 does not contain any of these articles.

I think this clearly shows the problem with this feature. A ‘filter’ feature can actually only work if you make specific requests to the database/backend. Pure filtering in the frontend is not possible.

Grotax · 2025-04-22T16:19:58Z

I agree with @wofferl.

Juri-w · 2025-04-28T00:20:44Z

I have changed it now on the backend, the starredFeed Counter is retrieved with the feeds and the stared items are fetched with the feed id, which returns only the starred items for the selected feed.

Btw. Thank you for constructive the feedback. I clearly hadn't tested it in the full extend before.

wofferl

Actual not working:

Starred feed routes don't fetch more items (see comment src/store/item.ts)
Starred feed counter don't update on change.
In some cases not all items are shown after reload app or press 'r' to refresh

So take al look at the suggestions and as I suggested I would prefer using the existing starred route to decrease complexity.

You also will take a closer look at the unit tests (frontend: make js-test, backend: make unit-test), it is always good idea to check changes with make test and npm run lint
Keep in mind that the api is also used by other clients.

CHANGELOG.md

lib/Db/ItemMapperV2.php

src/components/Sidebar.vue

src/routes/index.ts

src/store/item.ts

src/dataservices/item.service.ts

Juri-w · 2025-05-05T18:19:57Z

I've added the counter and the fetching works according to my tests properly as well. I couldn't recreate the problem with the refresh with 'r', but it could be that I solved it during solving the other problems.
I ran all JS (Unit-)Test, but couldn't get the make unit-test and make test to get running, because the bootstrap.php trys to use a bootstrap file three folders above require_once __DIR__ . '/../../../tests/bootstrap.php'; where logicly none exists. Maybe I'm missing something.

wofferl · 2025-05-06T06:09:05Z

I've added the counter and the fetching works according to my tests properly as well. I couldn't recreate the problem with the refresh with 'r', but it could be that I solved it during solving the other problems. I ran all JS (Unit-)Test, but couldn't get the make unit-test and make test to get running, because the bootstrap.php trys to use a bootstrap file three folders above require_once __DIR__ . '/../../../tests/bootstrap.php'; where logicly none exists. Maybe I'm missing something.

You need the test folder from the nextcloud/server repository which contains the bootstrap.php.

Now you have set :exact="true" you don't need the extra starredfeed route and all the checks when to use starred and when to use starredfeed. I have attached a patch how I would implement this, to have a consistent feedKey for starred through the app.
I would also not loading the lastViewedFeedId for the last starred feed, as long the open state for starred is not saved.

starred-route.patch.txt

Also the header in the item list for starred feeds don't show the feed name and the counter is not in sync.

You should then also consider squashing the commits, so that we have a clean commit history.

Juri-w · 2025-11-24T19:32:44Z

I think I have fix all errors now and all the unit-tests should succeed.

codecov · 2025-11-25T17:57:43Z

Codecov Report

❌ Patch coverage is 71.87500% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/components/routes/Starred.vue	33.33%	2 Missing and 6 partials ⚠️
src/components/Sidebar.vue	75.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Juri-w · 2025-11-27T07:57:13Z

I have resolved the errors, the tests should work now and added the missing tests.

src/components/routes/Starred.vue

Signed-off-by: Juri-w <[email protected]>

wofferl · 2025-11-30T09:07:50Z

Unselect starred element when starred feed is selected
I checked, but I found nothing else which would replace the previous :exact flag

Yes, it seems so, NcNavigationItem has an active prop, but it is only usable when not using vue-router.
I think that should be handled in the library. The easiest workaround I would see on our side would be to use a route like starred/0 or maybe better starred/all for the main route.

* [x]  Save open state for the starred menu entry
  I added it by checking the route and - collapsing or uncollapsing

This works for the moment, but the goal should be to save the open state permanently as with folders.

* [ ]  Handle lastitemviewed for starred feeds
  What do you mean with this? Because when selecting an item it shows in the last selected items as well.

Sorry, I meant lastViewedFeedId. This and lastViewedFeedType is used to get the last viewed route as initial route when the app is loaded with the default route apps/news.
It is set by the backend and loaded in src/route/index.ts. Here, adjustments must be made for both cases.

wofferl

For me this looks good now, maybe @Grotax or @SMillerDev also want to have a look here.

src/components/Sidebar.vue

Juri-w · 2025-12-30T10:30:17Z

Hey,
Hope you're doing well! I wanted to check in on this PR - it's been waiting for the two additional reviews for about a month now.
Is there anything I can do to help move this along? I'm available to walk through the changes or address any concerns.
Would appreciate any updates when you get a chance!

Copilot

Pull request overview

This pull request adds functionality to group starred items per feed in the Nextcloud News app. Users can now view starred items filtered by individual feeds, with each feed group showing a count of starred items. The grouped feeds appear as collapsible sub-items under the main "Starred" navigation item in the sidebar.

Key Changes

Extended the backend to track and return starred counts per feed, including database schema changes and new filtering capabilities
Updated the frontend to display grouped starred feeds with counts, including modifications to the Starred route component to accept an optional feedId parameter
Added comprehensive test coverage for the new grouped starred items functionality across both frontend and backend

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
lib/Db/Feed.php	Added starredCount property with setter method to track starred items per feed
lib/Db/FeedMapperV2.php	Modified query to include starred count via LEFT JOIN with distinct count aggregation
lib/Db/ItemMapperV2.php	Added feedId parameter to findAllItems to filter starred items by specific feed
lib/Service/ItemServiceV2.php	Updated findAllWithFilters signature to accept and pass feedId parameter
lib/Controller/ItemController.php	Passed id parameter to service layer for starred item filtering
src/types/Feed.ts	Added starredCount field to Feed type definition
src/types/MutationTypes.ts	Added MODIFY_STARRED_COUNT mutation type for updating feed starred counts
src/store/feed.ts	Implemented MODIFY_STARRED_COUNT mutation to increment/decrement starred counts
src/store/item.ts	Updated FETCH_STARRED action to accept feedId parameter and use feed-specific fetch keys
src/dataservices/item.service.ts	Modified fetchStarred to accept feedId and conditionally include it in API request
src/components/routes/Starred.vue	Added feedId prop and logic to filter/display starred items for specific feeds
src/components/Sidebar.vue	Added collapsible GroupedStars section showing feeds with starred items
src/routes/index.ts	Updated starred route to accept optional feedId parameter
appinfo/routes.php	Added new route for starred items with feedId parameter
tests/javascript/unit/store/item.spec.ts	Added tests for FETCH_STARRED with feedId parameter and starred count handling
tests/javascript/unit/store/feed.spec.ts	Added tests for MODIFY_STARRED_COUNT mutation
tests/javascript/unit/services/item.service.spec.ts	Added tests for fetchStarred with feedId parameter
tests/javascript/unit/components/routes/Starred.spec.ts	Updated tests to handle new feedId prop and fetch key logic
tests/javascript/unit/components/Sidebar.spec.ts	Added tests for GroupedStars computed property and collapsed state
tests/Unit/Service/ItemServiceTest.php	Updated test expectations to include feedId parameter in mapper calls
tests/Unit/Db/ItemMapperPaginatedTest.php	Updated all findAllItems test calls to include feedId parameter
tests/Unit/Db/FeedTest.php	Updated serialization test to include starredCount field and corrected null value expectations
tests/Unit/Db/FeedMapperTest.php	Updated to test both unread and starred count queries with proper aliasing
CHANGELOG.md	Added entry documenting the new feature

tests/javascript/unit/store/item.spec.ts

src/components/routes/Starred.vue

src/components/Sidebar.vue

lib/Db/Feed.php

Grotax · 2026-01-05T09:27:00Z

Basically looks ok to me, but copilot had some valid comments

Remove duplicate tests Co-authored-by: Copilot <[email protected]> Signed-off-by: Juri <[email protected]>

Signed-off-by: Juri-w <[email protected]>

Change second if to else Co-authored-by: Copilot <[email protected]> Signed-off-by: Juri <[email protected]>

Signed-off-by: Juri-w <[email protected]>

Juri-w · 2026-01-05T19:29:37Z

@Grotax I have made the suggested changes, I think it should be ready now.

wofferl · 2026-01-06T09:36:05Z

@Juri-w
Please fix the recently introduced lint errors

/home/build/server/apps/news/src/components/Sidebar.vue
  374:4  error  Keys starting with '_' are reserved in '_wasStarredVisited' group  vue/no-reserved-keys
  536:5  error  Unexpected side effect in "isStarredCollapsed" computed property   vue/no-side-effects-in-computed-properties

/home/build/server/apps/news/src/components/routes/Starred.vue
  13:1  error  Expected "../../types/Feed.ts" to come before "../../types/FeedItem.ts"  perfectionist/sort-imports

/home/build/server/apps/news/tests/javascript/unit/store/item.spec.ts
  383:4  error  Block must not be padded by blank lines  @stylistic/padded-blocks

I would remove the isStarredCollapsed function completely for the moment, as it currently has little use and will be replaced anyway when the load/save state for this would be implemented.

Signed-off-by: Juri-w <[email protected]>

Juri-w · 2026-01-06T16:52:58Z

@wofferl
I fix the linters, forgot to run those before.
I kept wasStarredVisited but replaced isStarredCollapsed. Until the load/save state is implemented, this is a useful interim solution, because when the page is reloaded on a starred route, the Starred section is otherwise collapsed.

wofferl · 2026-01-06T18:11:29Z

@wofferl I fix the linters, forgot to run those before. I kept wasStarredVisited but replaced isStarredCollapsed. Until the load/save state is implemented, this is a useful interim solution, because when the page is reloaded on a starred route, the Starred section is otherwise collapsed.

The tests need to be updated for this change also.

 FAIL  tests/javascript/unit/components/Sidebar.spec.ts > Sidebar.vue > SideBarState > should mark Starred collapsed when route is STARRED with feedId param
TypeError: Cannot read properties of undefined (reading 'call')
 ❯ tests/javascript/unit/components/Sidebar.spec.ts:216:54
    214| 
    215|   it('should mark Starred collapsed when route is STARRED with feedId param', () => {
    216|    const isCollapsed = wrapper.vm.$options.computed?.isStarredCollapsed.call({
       |                                                      ^
    217|     $route: { name: 'starred', params: { feedId: '123' } },
    218|     ROUTES: { STARRED: 'starred' },

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯

 FAIL  tests/javascript/unit/components/Sidebar.spec.ts > Sidebar.vue > SideBarState > should NOT mark Starred collapsed when route is STARRED without feedId param
TypeError: Cannot read properties of undefined (reading 'call')
 ❯ tests/javascript/unit/components/Sidebar.spec.ts:224:54
    222| 
    223|   it('should NOT mark Starred collapsed when route is STARRED without feedId param', () => {
    224|    const isCollapsed = wrapper.vm.$options.computed?.isStarredCollapsed.call({
       |                                                      ^
    225|     $route: { name: 'starred' },
    226|     ROUTES: { STARRED: 'starred' },

Signed-off-by: Juri-w <[email protected]>

Juri-w · 2026-01-06T22:43:00Z

@wofferl
Oh sorry, updated those now.

Juri-w · 2026-01-08T09:00:25Z

should I fix those errors? These are not related to this pull request.

wofferl · 2026-01-08T10:29:51Z

should I fix those errors? These are not related to this pull request.

No, these are warnings for the next Nextcloud version and will be fixed with #3485.

wofferl requested changes Apr 21, 2025

View reviewed changes

src/routes/index.ts Outdated Show resolved Hide resolved

src/components/Sidebar.vue Outdated Show resolved Hide resolved

wofferl added enhancement frontend impact Javascript/Frontend code labels Apr 21, 2025

Juri-w requested a review from wofferl April 21, 2025 16:41

wofferl reviewed Apr 21, 2025

View reviewed changes

Juri-w requested a review from wofferl April 21, 2025 23:27

wofferl requested changes Apr 22, 2025

View reviewed changes

src/components/Sidebar.vue Show resolved Hide resolved

src/components/routes/Starred.vue Outdated Show resolved Hide resolved

Juri-w requested a review from wofferl April 28, 2025 00:20

wofferl requested changes May 2, 2025

View reviewed changes

wofferl added the API Impact API/Backend code label May 2, 2025

Juri-w force-pushed the master branch from 117cd95 to f75bdd4 Compare May 5, 2025 18:50

Juri-w requested a review from wofferl May 5, 2025 18:55

Grotax marked this pull request as draft August 13, 2025 11:20

Juri-w marked this pull request as ready for review November 24, 2025 00:29

wofferl reviewed Nov 27, 2025

View reviewed changes

src/components/routes/Starred.vue Outdated Show resolved Hide resolved

Add feature to Group starred Items per Feed

4701189

Signed-off-by: Juri-w <[email protected]>

Juri-w force-pushed the master branch from f1e1d53 to 4701189 Compare November 27, 2025 21:29

readd list name and count

f526238

Signed-off-by: Juri-w <[email protected]>

Juri-w requested a review from wofferl November 27, 2025 21:34

Juri-w added 2 commits November 27, 2025 22:57

fix linters

4d8cfbb

Signed-off-by: Juri-w <[email protected]>

fix php unit tests

40c5208

Signed-off-by: Juri-w <[email protected]>

wofferl approved these changes Nov 30, 2025

View reviewed changes

src/components/Sidebar.vue Outdated Show resolved Hide resolved

wofferl requested review from Grotax and SMillerDev November 30, 2025 09:12

wofferl mentioned this pull request Dec 1, 2025

feat(NcAppNavigationItem): use exact route matching nextcloud-libraries/nextcloud-vue#7939

Open

2 tasks

Grotax requested a review from Copilot January 5, 2026 09:09

Copilot started reviewing on behalf of Grotax January 5, 2026 09:09 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

Juri-w and others added 6 commits January 5, 2026 17:19

Update tests/javascript/unit/store/item.spec.ts

8c11a6d

Remove duplicate tests Co-authored-by: Copilot <[email protected]> Signed-off-by: Juri <[email protected]>

Update tests/javascript/unit/store/item.spec.ts

3f936d7

Remove duplicate tests Co-authored-by: Copilot <[email protected]> Signed-off-by: Juri <[email protected]>

add missing type import

79af9de

Signed-off-by: Juri-w <[email protected]>

Update src/components/Sidebar.vue

8d30516

Change second if to else Co-authored-by: Copilot <[email protected]> Signed-off-by: Juri <[email protected]>

only open starred when called by url

2373591

Signed-off-by: Juri-w <[email protected]>

remove not neccessary setter

7f13ed3

Signed-off-by: Juri-w <[email protected]>

Juri-w added 2 commits January 6, 2026 17:08

Merge branch 'master' of https://github.com/nextcloud/news

89aa9e9

fix linters

837e2d8

Signed-off-by: Juri-w <[email protected]>

update StarredCollapse tests

ddedb25

Signed-off-by: Juri-w <[email protected]>

wofferl approved these changes Jan 7, 2026

View reviewed changes

Grotax approved these changes Jan 8, 2026

View reviewed changes

Grotax merged commit 4d7737b into nextcloud:master Jan 8, 2026
26 of 28 checks passed

wofferl mentioned this pull request Jan 8, 2026

feat: save starred navigation item open state #3499

Merged

Add feature to Group starred Items per Feed #3148

Add feature to Group starred Items per Feed #3148

Conversation

Juri-w commented Apr 20, 2025

Summary

Checklist

Uh oh!

wofferl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wofferl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juri-w commented Apr 21, 2025

Uh oh!

Uh oh!

Uh oh!

wofferl commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Grotax commented Apr 22, 2025

Uh oh!

Juri-w commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wofferl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Juri-w commented May 5, 2025

Uh oh!

wofferl commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Juri-w commented Nov 24, 2025

Uh oh!

codecov bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Juri-w commented Nov 27, 2025

Uh oh!

Uh oh!

wofferl commented Nov 30, 2025

Uh oh!

wofferl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Juri-w commented Dec 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Grotax commented Jan 5, 2026

Uh oh!

Juri-w commented Jan 5, 2026

Uh oh!

wofferl commented Jan 6, 2026

Uh oh!

wofferl left a comment •

edited

Loading

wofferl commented Apr 22, 2025 •

edited

Loading

Juri-w commented Apr 28, 2025 •

edited

Loading

wofferl left a comment •

edited

Loading

wofferl commented May 6, 2025 •

edited

Loading

codecov bot commented Nov 25, 2025 •

edited

Loading

wofferl left a comment •

edited

Loading