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

Episodes endpoint #95

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Episodes endpoint #95

wants to merge 5 commits into from

Conversation

keunes
Copy link
Member

@keunes keunes commented Jun 27, 2024

Copy link

netlify bot commented Jun 27, 2024

Deploy Preview for openpodcastapi ready!

Name Link
🔨 Latest commit 2f74ab1
🔍 Latest deploy log https://app.netlify.com/sites/openpodcastapi/deploys/6727468e694bee0008d36d65
😎 Deploy Preview https://deploy-preview-95--openpodcastapi.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Explain `download_status` and timestamp field & update table formatting on overview page. Add placeholder page for matching & deduplication. Specify GET /episodes.

**Important open discussion point:**

Sending back & forth timestamps is not needed if client always first pulls before push, and assuming that the client stores the timestamps of these changes locally. Maybe we should note this as a requirement, rather than submitting the timestamps.
Copy link
Member Author

Choose a reason for hiding this comment

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

It's hard to wrap my head around this, but I have the impression that we still need to send the timestamps around. Because the last sync date might be in the past, and you have a more recent change, but the server might have received more recent data from another client in the meantime. @mogwa1?


### Episode fields

| Group | Field | Type | Required? | Description |
Copy link
Member Author

Choose a reason for hiding this comment

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

Having this table seems a bit superfluous actually with the 'important fields' table on the Overview page. Do you think it would be safe to drop it here, and add a reference to the other page, instead @Sporiff?


This endpoint enables clients to return all episode information relating to the authenticated user. It returns pagination information and an array of `episodes`.

While supported, this endpoint is expected to be used rarely in practice. More often, episodes are retrieved per queue (TBD) or per subscription (TBD).
Copy link
Member Author

Choose a reason for hiding this comment

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

Did we have an idea yet where/how episodes of a given subscription should be retrieved? /v1/episodes?subscription=x or GET /v1/subscriptions/{guid}/episodes?

Side note: I guess this episode will be used a lot actually, just with the since parameter most of the time.

@@ -202,6 +204,51 @@ paths:
security:
- podcast_auth:
- read:subscriptions
/episodes:
Copy link
Member Author

Choose a reason for hiding this comment

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

@Sporiff Could I ask you to review? I tried to follow the format of the existing file :-)

@@ -599,8 +807,8 @@ components:
implicit:
authorizationUrl: https://test.openpodcastapi.com/oauth/authorize
scopes:
write:subscriptions: modify subscription information for your account
read:subscriptions: read your subscription information
write:subscriptions: modify subscription information & related episodes for your account
Copy link
Member Author

Choose a reason for hiding this comment

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

I think reading & writing subscriptions & episodes can both be in the same scope. Would you agree @Sporiff @JonOfUs?

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.

1 participant