Poll Twitter for tweets made containing a given hashtag #12
Conversation
For any found tweets, add them as a new unflagged topic against the owner user record, so that they can be reviewed.
|
Quick note:
Nope. https://github.com/mattstauffer/suggestive/blob/master/tests/AdminTopicTest.php#L74 You can pass in parameters; haven't finalized what they'll be but right now it would be |
|
I saw that part. I meant to say that the topics list currently shows topics whether they're flagged as accepted or not. The dashboard currently hits |
|
@deringer Aha, got it :) 👍 will review PR later, busy day w the fam today :) |
resources/assets/js/app.js
Outdated
There was a problem hiding this comment.
Nope, we actually want to list out all proposed topics for the purpose of voting.
There was a problem hiding this comment.
Ahhh, my bad - I misunderstood the purpose of the accepted status.
|
Looks good. Architecturally, I wonder if it wouldn't be better to have something of a "machine" user that could be the one that gets assigned these topics? |
I misinterpreted the purpose of the topic status, which is being used to flag topics that were accepted on the podcast, rather than accepted for voting.
|
Some kind of system user was my first thought, but I don't know what the multi-tenant vision is for Suggestive, so wasn't sure how a polled status would then be assigned back to the appropriate owner. Happy to discuss and implement! |
Limit duplication of incoming topics by rejecting statuses which have been either quoted or retweeted.
app/Console/Commands/PollTwitter.php
Outdated
There was a problem hiding this comment.
Opening parenthesis of a multi-line function call must be the last content on the line
app/Console/Commands/PollTwitter.php
Outdated
There was a problem hiding this comment.
Oh man, I commented on your comments above but GitHub has hidden them all now 😁
There's a couple of others where I asked for feedback on, too. Should have waited before pushing new commits. I'm not a GitHub noob >_>
There was a problem hiding this comment.
No, no need to wait; usually it won't wipe the old comments. I'll look through them.
On this one, your comment was:
It's not an interface, it's an (I don't know what the proper name is) alias to the underlying return, given the TwitterOAuth client doesn't implement an interface.
Would you suggest that in this instance we would create an app-specific wrapper for it that implements an interface so that we can bind to it?
This particular solution has always seemed hacky to me (I use the same method in another app I have). The alternative would be to wrap it in a facade?
I'm not sure what you mean for the Interface; why don't you just typehint Abraham\TwitterOAuth\TwitterOAuth? If you want TwitterClient as a shortcut like you have it here, you can bind one of them like you are right now, and then have the other one alias to it. Ping me here if you want me to write out the code for what that would look like.
There was a problem hiding this comment.
Yeah, for some reason I had trouble getting this to work in another project use used TwitterOAuth with, but that's because I was passing the additional parameters to it, so needed to use app()->make() to pass the extra parameters in.
I'll push an update shortly that fixes this and pushes the functionality off to a Job 👍
Add a bit of logging around Twitter API rate limits so it's obvious if things "just stop working"
There was a problem hiding this comment.
Each class must be in a namespace of at least one level (a top-level vendor name)
|
@deringer I OWE YOU A PR REVIEW OF THIS. I still remember! |
|
In your own time my friend, though I’ll need to leverage some of the code I wrote here on another project at some point so another set of eyes over it will be handy (no rush!) Michael Dyrynda Sent from On Wed, 02 Mar 2016 at 3:12 PM Matt Stauffer <
https://github.com/deringer — Reply to this email directly or |
|
@michaeldyrynda holy crap i never reviewed this :/ // / / / /// // // // / |
|
I did look at this yesterday. Now there are lots merge conflicts with compiled assets lol |
3 participants
Addresses #5.
For any found tweets, add them as a new unflagged topic against the owner user record, so that they can be reviewed.
I've done some preliminary work on this and opened a PR so I can get some feedback on the direction I've taken so far. The basic theory is that a user will specify a hashtag that Suggestive looks for periodicially (scheduling the
suggestive:poll-twittercommand), and creates unflaggedTopicrecords for the owner to review.I assume as well that the
/topicsAPI endpoint should return only accepted topics (it currently returns everything).I haven't yet made any interface to set a hashtag, though that's only because there's no user profile at this stage.
This will also use the application Twitter credentials, which is 150 requests every ~15 minutes. This may be ok initially, but it is trivial to make it 150 requests per user - you just need to capture the Twitter token and secret when the user signs up. Of course, that's not much good with a GitHub authenticated user, so you might want to lock it down to just Twitter users.