Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Hello Ajit,
I took the liberty of making some refactoring / adjustments to your project, which also may help you in your project application:
Corrections
First, I was getting a "Bad Request" error every time the Facebook Open Graph API was invoked.
The reason is that you were using an older version of the API, "https://api.facebook.com/method/fql.query?" is now "https://graph.facebook.com/fql?" and the URLs also needed to be encoded before making the request, in order to avoid that error.
New features
By the other hand, I also added a new parameter (setting) to your admin form: the start date for which the nodes will be retrieved. Since these APIs have quota limits, it's important to give the users the possibility to limit by date the maximum range of requests that will be made to the different social networks. Also, this helps avoid timeout errors on large sites with lots of nodes.
Refactoring
I separated the Facebook and Twitter calls to independent functions. That makes the code more readable and flexible for future changes. Didn't have much time to work on the LinkedIn and Google Plus services though, since they were out of the scope of the requirements I have at this particular moment.
You will find attached the files with the proposed changes. Feel free to review them and add any improvements.
I would like to become a co-maintainer of this project and help whenever I can. If you grant me permission to access your git repo I could push the changes directly to the source code.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 2105121_social_stats_module.patch | 8.44 KB | jcamposanok |
| #3 | 2105121_social_stats_admin.patch | 1.03 KB | jcamposanok |
| #1 | social_stats.module.txt | 10.14 KB | jcamposanok |
| social_stats.admin_.inc_.txt | 1.45 KB | jcamposanok | |
| social_stats.module.txt | 10.07 KB | jcamposanok |
Comments
Comment #1
jcamposanok CreditAttribution: jcamposanok commentedMy apologies,
a required parameter was missing on the new functions of the previous .module file. Consider this new file instead.
Comment #2
AjitS@jcamposanok,
Thank you for reviewing the code. I am glad you want to help. I am currently travelling; so didn't get a chance to test this. I just had a look at the code and it looks fine to me. Could you please submit the changes in the form of patch so that it would be easier for me to review it? I will test this tomorrow and commit the patch. I can also do an accredited commit in your name so that it gets reflected on your d.o profile.
I also welcome your proposal to be a co-maintainer for this project. But, I think I will think about that after the project gets promoted as full project.
I really appreciate your help with this.
For the benefit of others, this project is under review at #2084467: [D7] Social Stats with views and panels integration.
Comment #3
jcamposanok CreditAttribution: jcamposanok commentedAttached both files as patches.
By the way, is it necessary to store the node type in the social_stats tables?
Comment #4
AjitSTesting this. Will report back soon. Thanks.
Comment #5
AjitSThe patches didn't apply cleanly. Got the messages:
and
Applied them manually.
This patches change a hell lot of code, and has moving parts. I will have to break it into smaller tasks.
Will only have Changing the Facebook API to the new Graph API as a part of this issue.
The code:
gives the error
Made the following change to fix it:
Removed all the commented watchdog calls. Fixed the issues reported by Code Sniffer.
Also, moved the "configuration" menu under "services". Not a part of this issue, but felt it was appropriate.
Made an accredited commit https://drupal.org/commitlog/commit/58029/c4d3b13446cc379084ee1360c6a1fc...
Follow up issues:
Comment #6
AjitSNo activity for two weeks after this has been fixed. Closing it.