Skip to content
This repository has been archived by the owner on Dec 27, 2018. It is now read-only.

Added flattr to the feed #17

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Added flattr to the feed #17

wants to merge 5 commits into from

Conversation

klaustopher
Copy link
Contributor

This adds flattr to the feed ... The necessary infos for the auto URL stuff need to be added to the podcast.yml

---
flattr:
  user_id: klaustopher
  language: de_DE
  tags:
    - podcast
    - nerdkunde
    - technology
    # See https://api.flattr.com/rest/v2/categories.txt
  category: audio

Still to do

  • Add tests for the Auto-Submit-URL generation to Podcast and Episode
  • Refactor the duplication into a flattr helper class
  • Fix Podcast#deep_link_url since it only handles GST-Kitchen URLs (@tisba, please let's talk about this)

@klaustopher klaustopher mentioned this pull request Aug 4, 2013
Closed
@tisba
Copy link
Owner

tisba commented Aug 4, 2013

Cool thing! I'll think about what we can do to make Podcast#deep_link_url customizable.

tisba
tisba reviewed Aug 4, 2013
View reviewed changes
lib/gst-kitchen/episode.rb
@@ -86,6 +88,10 @@ def rfc_2822_date
self.published_at.rfc2822
end

def flattr_auto_submit_link
"https://flattr.com/submit/auto?user_id=#{podcast.flattr["user_id"]}&url=#{CGI.escape podcast.deep_link_url(self)}&title=#{CGI.escape self.title}&description=#{CGI.escape Sanitize.clean(self.subtitle)}&language=#{podcast.flattr["language"]}&tags=#{podcast.flattr["tags"].join(',')}&category=#{podcast.flattr["category"]}"
Copy link
Owner

Choose a reason for hiding this comment

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

If I see this correctly, this is almost the same as in Podcast#flattr_auto_submit_link. Maybe we can factor this out into a helper, or something. I'm thinking about including ActiveSupport to get Hash#to_param to make this readable.

@klaustopher
Copy link
Contributor Author

Well, it's a string with 7 params... So tje duplication isn't hurting so much in my eyes. But moving this out to a new class is fine with me as well... I can extract this out tomorrow when I add the tests

@klaustopher
Copy link
Contributor Author

I removed the duplication and added the tests ... Any ideas about the deep link thingy? I have fixed it in my fork for our urls but I think forking the kitchen is not the way to go

@klaustopher
Copy link
Contributor Author

Just noticed... We can remove the sanitize gem... I'll do that later

@dennisreimann dennisreimann mentioned this pull request Aug 10, 2013
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@klaustopher @tisba