At present, the twitter_pull get_items method wraps the twitter_get_items method. If the latter returns a false value, it falls back to an unauthenticated request, logging the message Twitter Pull is using an Unauthenticated request to twitter apis. Download, enable and configure the twitter module to allow for authenticated requests. on the assumption that twitter_get_items returned false owing to the twitter module not being present/configured. However, twitter_get_items will also return false if it makes a successful request which returns no tweets. This behaviour is spurious, as it gives a false impression in the logs that there is a problem with the twitter module when invoked with arguments that results in no tweets being found.

CommentFileSizeAuthor
#8 twitter_pull-no_tweets_bug-2021591-6.patch865 bytesscuba_fly
#5 twitter_pull-no_tweets_bug-2021591-5.patch865 bytespjcdawkins
#2 get_items_unauthenticated_request-2021591-2-D7.patch917 bytesgausie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins commented
Issue summary: View changes

I've had this issue too

gausie’s picture

gausie CreditAttribution: gausie commented
FileSize
get_items_unauthenticated_request-2021591-2-D7.patch917 bytes

This patch fixes the issue.

It changes the output of twitter_get_items to the number of tweets received, provided an authenticated request can be made. Unavoidably, I have had to change the output of a function in the class, so this could be a breaking change for a few implementations.

pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins commented
Status: Active » Needs review

Setting to 'Needs review' (IMO the patch works, but I work with @gausie so somebody less biased needs to review it)

pstewart’s picture

pstewart CreditAttribution: pstewart commented

Patch works as expected for me - zero tweet results demonstrate correct behaviour.

pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins commented
FileSize
twitter_pull-no_tweets_bug-2021591-5.patch865 bytes

I've re-rolled gausie's patch in #2 so that it is relative to the module's directory - no other changes.

I think this is RTBC.

pstewart’s picture

pstewart CreditAttribution: pstewart commented
Status: Needs review » Reviewed & tested by the community

Agreed.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Reviewed & tested by the community » Needs work

I don't really understand what the change is here, so setting back to needs work I'm afraid.

  1. +++ b/twitter_pull.class.inc
@@ -43,11 +43,12 @@ class twitter_puller {
-    /* First try to use the twitter module to get the tweets
+    /**
+     * First try to use the twitter module to get the tweets

    Unrelated change. 

  2. +++ b/twitter_pull.class.inc
@@ -43,11 +43,12 @@ class twitter_puller {
-    if ($this->twitter_get_items()) {
+    if ($this->twitter_get_items() !== false) {

    Constants should be in caps.

    I also don't really follow the change here. 

  3. +++ b/twitter_pull.class.inc
@@ -188,7 +189,7 @@ class twitter_puller {
-    return (count($this->tweets) > 0);
+    return count($this->tweets);

    That is changing the return value of this method from a boolean to an integer!

scuba_fly’s picture

scuba_fly
Location Netherlands, Vinkeveen
CreditAttribution: scuba_fly at LimoenGroen commented
Status: Needs work » Needs review
FileSize
twitter_pull-no_tweets_bug-2021591-6.patch865 bytes

Not sure why 3. is needed from joachim's comment.
I used the patch above and now I have tweets again.

I've updated the patch to be capital letters.
left part 1 in because it is a easy codestyle fix.

Here's a new patch.

Can somebody explain part 3 from joachim's comment above? Is that really needed and why?

Hmm I somehow created the patch with comment #6.

scuba_fly’s picture

scuba_fly
Location Netherlands, Vinkeveen
CreditAttribution: scuba_fly at LimoenGroen commented

Hmm I think I see why nr3 is needed.

If the count of the tweets is 0 it will return FALSE.

Then in the method get_items() it will result in a return:

 function get_items() {

    /**
     * First try to use the twitter module to get the tweets
     * At some point, authentication will be required and if you have the
     * twitter module set up properly, it will handle the authenticated session.
     */
    if ($this->twitter_get_items() !== FALSE) {
      return;
    }
pjcdawkins’s picture

pjcdawkins CreditAttribution: pjcdawkins as a volunteer commented

joachim's comment 3 suggests he hasn't read the comment in #2 - changing the return value of the function was intentional. The caller needs to know whether no value (NULL/void) is returned (because the Twitter integration isn't working or isn't configured) or just no tweets (0).