Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improvements for API v1.2 #1727

Merged
merged 10 commits into from
Apr 30, 2022
Merged

Improvements for API v1.2 #1727

merged 10 commits into from
Apr 30, 2022

Conversation

powerpaul17
Copy link
Contributor

No description provided.

@powerpaul17
add routes for starring/unstarring items by id
f77d4a4 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@SMillerDev
Copy link
Contributor

Please make sure to add tests for this too.

@powerpaul17
Copy link
Contributor Author

Please make sure to add tests for this too.

Yeah, sure, just have to re-activate my development setup 😉 . I'll also add the method for getting only IDs too. Just wanted to set up the PR (also marked it as a Draft)

@SMillerDev
Copy link
Contributor

Can you make the ID-only change a new pull request?

@powerpaul17
Copy link
Contributor Author

Can you make the ID-only change a new pull request?

Sure. Then I'll only add tests for this one.

@powerpaul17
add tests for starring/unstarring items by id
c8fec7c 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17
document new starring api methods in changelog
2e5dcf2 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17 powerpaul17 marked this pull request as ready for review April 11, 2022 15:15
SMillerDev
SMillerDev reviewed Apr 11, 2022
View reviewed changes
lib/Controller/ItemApiController.php Outdated Show resolved Hide resolved
lib/Service/ItemServiceV2.php Outdated Show resolved Hide resolved
@powerpaul17
fix indentation of function description
f4917ed 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17
remove duplicate function
35d921a 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@SMillerDev
Copy link
Contributor

Php changes look good now, do you want to add this to the docs in this PR or the next?

@powerpaul17
Copy link
Contributor Author

I will add the documentation of the 2 new calls so this PR is complete.

@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2022 •
edited
Loading

Codecov Report

Merging #1727 (b8bfbd3) into master (681bef3) will decrease coverage by 0.08%.
The diff coverage is 75.00%.

❗ Current head b8bfbd3 differs from pull request most recent head da637b1. Consider uploading reports for the commit da637b1 to get more accurate results

@@             Coverage Diff              @@
##             master    #1727      +/-   ##
============================================
- Coverage     91.86%   91.77%   -0.09%     
- Complexity      766      775       +9     
============================================
  Files            65       65              
  Lines          2692     2724      +32     
============================================
+ Hits           2473     2500      +27     
- Misses          219      224       +5
Impacted Files Coverage Δ
lib/Controller/ApiController.php 72.72% <0.00%> (ø)
lib/Controller/ItemApiController.php 95.60% <78.94%> (-4.40%) ⬇️
lib/Command/ExploreGenerator.php 100.00% <0.00%> (ø)
lib/Explore/RecommendedSites.php 0.00% <0.00%> (ø)
lib/Controller/FeedController.php 100.00% <0.00%> (ø)
lib/Controller/PageController.php 100.00% <0.00%> (ø)
lib/Fetcher/Client/FeedIoClient.php 100.00% <0.00%> (ø)
lib/Controller/FeedApiController.php 100.00% <0.00%> (ø)
lib/Db/Feed.php 99.05% <0.00%> (+<0.01%) ⬆️
lib/Fetcher/FeedFetcher.php 80.27% <0.00%> (+0.13%) ⬆️
... and 1 more

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 681bef3...da637b1. Read the comment docs.

@powerpaul17
rename documentation for api v1.2
498a934 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17
create copies of v1.2 api routes for v1.3 api
39306e2 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17
also allow starring/unstarring multiple items by id
f901d28 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17
add documentation for API v1.3 & fix parameter description
da637b1 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@powerpaul17
Copy link
Contributor Author

One more thing: I noticed that the read/multiple and unread/multiple calls use items as the function parameter but actually expect itemIds. Should I also copy the function definition and chain it through?

@powerpaul17
rename parameter for read/unread api call
545a210 
Signed-off-by: Paul Tirk <paultirk@paultirk.com>
@Grotax Grotax mentioned this pull request Apr 14, 2022
Merged
@powerpaul17 powerpaul17 requested a review from SMillerDev April 15, 2022 07:46
@Grotax
Copy link
Member

Grotax commented Apr 30, 2022

I think if itemIds are used it should also be labeled like that and not as items

@Grotax Grotax merged commit da5e749 into nextcloud:master Apr 30, 2022
@Grotax
Copy link
Member

Grotax commented Apr 30, 2022

Thanks a lot @powerpaul17

@powerpaul17 powerpaul17 deleted the api_v1_3 branch May 3, 2022 08:59
Grotax added a commit that referenced this pull request May 29, 2022
@Grotax
Release 18.1.0-beta1
858ce5c 
Changed
- Add API v1.3 adding routes for starring/unstarring items by id and general fixes (#1727)
  https://nextcloud.github.io/news/api/api-v1-3/
- Improve styling of tables in articles (#1779)
- Allow fetching feeds that omit guid by using link as stand-in (#1785)

Fixed
- Fix updated api not returning any item after marking item as read (#1713)
- Fix deprecation warning for strip_tags() on a null value (#1766)
- Fix selected item being set incorrectly when using default ordering or newest first ordering (#1324)
- Fix doubling the height of the content area (#1796)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request May 29, 2022
Merged
Grotax added a commit that referenced this pull request May 29, 2022
@Grotax
Release 18.1.0-beta1
86e01a9 
Changed
- Add API v1.3 adding routes for starring/unstarring items by id and general fixes (#1727)
  https://nextcloud.github.io/news/api/api-v1-3/
- Improve styling of tables in articles (#1779)
- Allow fetching feeds that omit guid by using link as stand-in (#1785)

Fixed
- Fix updated api not returning any item after marking item as read (#1713)
- Fix deprecation warning for strip_tags() on a null value (#1766)
- Fix selected item being set incorrectly when using default ordering or newest first ordering (#1324)
- Fix doubling the height of the content area (#1796)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SMillerDev SMillerDev SMillerDev approved these changes

Assignees
No one assigned
Labels
3. to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@powerpaul17 @SMillerDev @codecov-commenter @Grotax