-
Notifications
You must be signed in to change notification settings - Fork 17
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: Add sanitizer for tiktok #310
Conversation
b7d379f
to
b43e3d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your pull request 😃 I have a few remarks.
...main/src/main/kotlin/com/svenjacobs/app/leon/core/domain/sanitizer/tiktok/TiktokSanitizer.kt
Outdated
Show resolved
Hide resolved
...main/src/main/kotlin/com/svenjacobs/app/leon/core/domain/sanitizer/tiktok/TiktokSanitizer.kt
Outdated
Show resolved
Hide resolved
.../src/test/kotlin/com/svenjacobs/app/leon/core/domain/sanitizer/tiktok/TiktokSanitizerTest.kt
Outdated
Show resolved
Hide resolved
That is correct. Leon would have to perform network requests to unwrap these short URLs which is not desired. |
Note some sharing interfaces in the app give you shortened links (https://vt.tiktok.com/fr54gr6rf3e) that expand to the links matched here, but there isn't infrastructure in the app currently to deal with them.
b43e3d8
to
d7600f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you very much!
@all-contributors please add @yedayak for code |
I've put up a pull request to add @yedayak! 🎉 |
Note some sharing interfaces in the app give you shortened links (like https://vt.tiktok.com/fr54gr6rf3e) that expand to the links matched here, but there isn't infrastructure in the app currently to deal with them.