Skip to content

Add AudioBlock implementation with tests #187

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aydinomer00
Copy link
Contributor

Fixes #183

This PR adds AudioBlock implementation with the following changes:

  • Added BlockTypeAudio to block types
  • Implemented DownloadableFileBlock interface for AudioBlock
    • Implemented GetURL() method for retrieving internal/external audio URLs
    • Implemented GetExpiryTime() method for handling file expiry
  • Added comprehensive test coverage
    • Tests for internal and external URLs
    • Tests for expiry time functionality
    • Interface compliance checks

All tests are passing. The implementation follows the same pattern as other media blocks (Image, Video) in the codebase.

This change:
- Adds new interface to unify common functionality
- Implements interface for PdfBlock, FileBlock, and ImageBlock
- Adds comprehensive test coverage
- Implemented GetURL() and GetExpiryTime() methods for AudioBlock
- Added comprehensive tests for AudioBlock functionality
- Added interface compliance checks
const.go Outdated
@@ -218,6 +218,8 @@ const (
BlockTypeChildPage BlockType = "child_page"
BlockTypeChildDatabase BlockType = "child_database"

BlockTypeAudio BlockType = "audio"

Choose a reason for hiding this comment

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

Other constants are grouped. Maybe then put this BlockTypeAudo after image and before video so it's grouped as well?

@aydinomer00
Copy link
Contributor Author

Thanks for the feedback! I'll move the BlockTypeAudio constant to be grouped with other media types, placing it between image and video blocks for better organization.

@aydinomer00
Copy link
Contributor Author

aydinomer00 commented Dec 23, 2024 via email

@amberpixels
Copy link

@aydinomer00 sync please your branch, so it doesn't have conflicts for merging. I guess then it will be sooner to be merged by maintainer.

P.S. I'm ok sitting with local branch-on-top-of-branches, but i'm looking forward to getting this merged as well :)

@aydinomer00
Copy link
Contributor Author

@amberpixels Thank you so much for your patience and guidance! I’m still learning the ropes with Git and doing my best to follow your instructions. I’ve tried to sync my branch and resolve the conflicts, but I’m not entirely sure if everything was done correctly. If you notice any mistakes or have additional suggestions, I’d really appreciate your feedback. Thanks again for all your help!

@amberpixels
Copy link

hey @aydinomer00. There are several ways of resolving the issue. I don't claim my approach is the most comfortable, but here what you can do:

  1. Your fork aydinomer00/notionapi is 1 commit behind from jomei/notionapi:main. You'll see the button Sync there on the main page of your fork.
  2. after your main branch is up-to-date you can either merge or rebase. In this case i personally prefer merging. So you checkout your feature branch and do git merge main. After resolving conflicts and commiting you can push to your branch again.

@aydinomer00
Copy link
Contributor Author

@amberpixels I just used the “Update branch” button to sync my branch with the upstream repository. It looks like there aren’t any conflicts so far. Could you please take a look and let me know if you spot any issues or have any suggestions? Thanks!

@jomei
Copy link
Owner

jomei commented Feb 19, 2025

@aydinomer00 could you please resolve conflicts in this PR?
I can easily merge it then

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.

Need an BlockTypeAudio type
3 participants