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.

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

feat(nextcloud): add news endpoints for marking multiple articles as read starred #2324

Merged
merged 2 commits into from
Jul 28, 2024

Conversation

Leptopoda
Copy link
Member

No description provided.

@Leptopoda Leptopoda requested a review from provokateurin July 27, 2024 19:53
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first commit is wrong because booleans in query parameters are represented as 0/1 in Nextcloud. If the parameters were sent in the body then they could need to be booleans indeed.

packages/nextcloud/test/news_test.dart Show resolved Hide resolved
@Leptopoda
Copy link
Member Author

The first commit is wrong because booleans in query parameters are represented as 0/1 in Nextcloud. If the parameters were sent in the body then they could need to be booleans indeed.

Can you please clarify that? The docs say that it's meant to be a boolean:
https://nextcloud.github.io/news/api/api-v1-3/#initial-sync
And judging from the code that's the right thing
https://github.com/nextcloud/news/blob/350eea3aa1c62d0ccc18b522bed27a3bb13b208e/lib/Controller/ItemApiController.php#L61

@provokateurin
Copy link
Member

Yes, but the documentation shows a JSON body as the example which we don't make use of here.
You can see the old quirk the openapi-extractor had when it still used query paramters: nextcloud/openapi-extractor@826eaf2#diff-ab3a3d862e1786c66098dfc9cba0909b72dbdf080660a1e971ac7851157b2bb8L80
I don't know the location where this is implemented on the server side, but my guess would be that it is within Symfony.
I have no better explanation that this, it's just how it works 🤷‍♀️

@Leptopoda
Copy link
Member Author

I don't know where you see a json body

/items?type=3&getRead=false&batchSize=-1

But if you say so ...

@provokateurin
Copy link
Member

provokateurin commented Jul 28, 2024

The docs must be wrong there, only 0/1 work (I previously looked at the wrong part of the docs when I talked about the JSON body).

@Leptopoda Leptopoda force-pushed the feat/nextcloud/news_add_batch_edit_endpoints branch from 544334c to f52bbaf Compare July 28, 2024 08:15
@Leptopoda Leptopoda changed the title fix(nextcloud)!: make news getRead and oldestFirst parameter a boolean feat(nextcloud): add news endpoints for marking multiple articles as read starred Jul 28, 2024
@Leptopoda Leptopoda requested a review from provokateurin July 28, 2024 08:16
.vscode/settings.json Outdated Show resolved Hide resolved
Signed-off-by: Nikolas Rimikis <leptopoda@users.noreply.github.com>
…read/starred

Signed-off-by: Nikolas Rimikis <leptopoda@users.noreply.github.com>
@Leptopoda Leptopoda force-pushed the feat/nextcloud/news_add_batch_edit_endpoints branch from f52bbaf to 07d1eba Compare July 28, 2024 12:27
@Leptopoda Leptopoda enabled auto-merge July 28, 2024 12:27
@Leptopoda Leptopoda merged commit 9b5679a into main Jul 28, 2024
8 checks passed
@Leptopoda Leptopoda deleted the feat/nextcloud/news_add_batch_edit_endpoints branch July 28, 2024 12:41
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.

2 participants