Skip to content

Conversation

@maxolasersquad
Copy link
Contributor

Closes #1359

This is not at all tested. I'd like feedback about if this feature is desired and if my approach here is the correct one.

@SMillerDev
Copy link
Contributor

I think the feature is fine, but I would like to see this as a separate command.

@maxolasersquad
Copy link
Contributor Author

A command configured like this?

        $this->setName('news:updater:update-feeds')
            ->addArgument(
                'user-id',
                InputArgument::OPTIONAL,
                'user id of a user, string'
            )
            ->setDescription('Console API for updating multiple feeds');

@SMillerDev
Copy link
Contributor

I would call it update-user to avoid confusion but yeah

@Grotax
Copy link
Member

Grotax commented Jun 1, 2021

@maxolasersquad can you implement the change request so that we can maybe still merge it into the 16.0.0 release?

@maxolasersquad
Copy link
Contributor Author

Yes, I'll work on it right now.

@maxolasersquad maxolasersquad marked this pull request as ready for review June 2, 2021 03:53
@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2021

Codecov Report

Merging #1360 (43d48d6) into master (65c15da) will increase coverage by 0.09%.
The diff coverage is 100.00%.

❗ Current head 43d48d6 differs from pull request most recent head e330d24. Consider uploading reports for the commit e330d24 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1360      +/-   ##
============================================
+ Coverage     91.93%   92.02%   +0.09%     
- Complexity      759      766       +7     
============================================
  Files            64       65       +1     
  Lines          2776     2796      +20     
============================================
+ Hits           2552     2573      +21     
+ Misses          224      223       -1     
Impacted Files Coverage Δ
lib/Command/Updater/UpdateUser.php 100.00% <100.00%> (ø)
lib/Service/FeedServiceV2.php 100.00% <0.00%> (ø)
lib/Command/Debug/ItemList.php 100.00% <0.00%> (ø)
lib/Command/Debug/FeedItemList.php 100.00% <0.00%> (ø)
lib/Command/Debug/FolderItemList.php 100.00% <0.00%> (ø)
lib/Fetcher/FeedFetcher.php 89.11% <0.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65c15da...e330d24. Read the comment docs.

@maxolasersquad
Copy link
Contributor Author

FYI, I'll be leaving town tomorrow morning and will not have much Internet access until I return Monday evening. I'm not sure what the timeline for the 16.0 release is. Hopefully I can get all suggestion resolved on time.

@Grotax
Copy link
Member

Grotax commented Jun 4, 2021

I plan the release latest when NC 22 final is released.
That will still take some weeks.

And we could also move this to 16.1.0

Closes #1359

Signed-off-by: David Baucum <[email protected]>
@Grotax Grotax merged commit 21203db into nextcloud:master Jul 24, 2021
Grotax added a commit that referenced this pull request Jul 24, 2021
Changed
- Added vue and ng-vue packages (#1421)
- Reimplemented relative time formatting as a filter (#1450)
- Added new `news:updater:update-user` command to update the feeds of a single user (#1360).

Signed-off-by: Benjamin Brahmer <[email protected]>
@Grotax Grotax mentioned this pull request Jul 24, 2021
Grotax added a commit that referenced this pull request Sep 2, 2021
Changed
- Added new `news:updater:update-user` command to update the feeds of a single user (#1360).

Fixed
- Removed spurious requests for `.../apps/news/%7B%7B%20::Content.getFeed(item.feedId).faviconLink%20%7D%7D` (#1488)

Signed-off-by: Benjamin Brahmer <[email protected]>
@Grotax Grotax mentioned this pull request Sep 2, 2021
Grotax added a commit that referenced this pull request Sep 2, 2021
Changed
- Added new `news:updater:update-user` command to update the feeds of a single user (#1360).

Fixed
- Removed spurious requests for `.../apps/news/%7B%7B%20::Content.getFeed(item.feedId).faviconLink%20%7D%7D` (#1488)

Signed-off-by: Benjamin Brahmer <[email protected]>
Neo11 pushed a commit to Neo11/news that referenced this pull request May 28, 2022
Changed
- Added new `news:updater:update-user` command to update the feeds of a single user (nextcloud#1360).

Fixed
- Removed spurious requests for `.../apps/news/%7B%7B%20::Content.getFeed(item.feedId).faviconLink%20%7D%7D` (nextcloud#1488)

Signed-off-by: Benjamin Brahmer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants