Problem/Motivation
I had a client who asked me to have an option of sorting a list of documents (view of content) with the help of their popularity on social media sites. For example, he wished to be able to sort some documents on basis of the number of facebook likes, comments, and shares; ascending or descending. I had to write a views plugin for the same. On searching I found no other module doing this. Hence, the contribution.
Description
This is a statistics module. It provides data from various social media sites. The data which is saved per node includes data from:
- Facebook : likes count, shares count, comments count & total count.
- Twitter : tweets count.
- LinkedIn : share count.
- Google Plus : plus one count
- Total Shares : Facebook total count + tweets count + LinkedIn share count + Google plus one count
The module has integration with Views and Panels (please refer the README.txt).
Project Page
https://drupal.org/sandbox/ajgit/2080003
Project issue queue
https://drupal.org/project/issues/2080003?status=All&categories=All
GIT Repository
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/ajgit/2080003.git
pareview.sh
http://pareview.sh/pareview/httpgitdrupalorgsandboxajgit2080003git (no issues reported).
Reviews of other projects
- https://drupal.org/node/2084273#comment-7844671
- https://drupal.org/node/2104683#comment-7940573
- https://drupal.org/node/2104683#comment-7940739
- https://drupal.org/node/2104683#comment-7941271
- https://drupal.org/node/2104683#comment-7943507
- https://drupal.org/node/2108495#comment-7950237
- https://drupal.org/node/2107725#comment-7959221
- https://drupal.org/node/2080721#comment-7959243
- https://drupal.org/node/2093037#comment-7959825
More reviews
Comment | File | Size | Author |
---|---|---|---|
#11 | Social Statistics.png | 30.4 KB | AjitS |
#11 | Share count.png | 16.79 KB | AjitS |
Comments
Comment #1
PA robot CreditAttribution: PA robot commentedWe are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #2
littledynamo CreditAttribution: littledynamo commentedSteps of review:
Cloned module into clean install of Drupal 7.23 and enabled.
Automated Tests
Ran module through pareview.sh - no issues found
Duplication
There is another module called Share Count Statistics which seems to be doing a similar thing - creating stats on how many times certain pieces of content have been shared etc. It has views integration and an API for adding new types.
Were you aware of this module? Can you explain how your module is different?
Security
No issues found.
No user input in this module, so no danger of XSS.
Database API used always so no danger of SQL Injection.
Manual Code Review
I noticed a DSM call on line 99 of social_stats.module:
No other issues found - t() used consistently for all UI facing text, variables deleted in .install etc
Comment #3
littledynamo CreditAttribution: littledynamo commentedComment #4
AjitS@littledynamo : Thanks a lot for the review. I couldn't believe that I was not able to search the module that implemented that functionality. Nevertheless, it was a good learning experience going through the review process. I understand Having too many projects doing the same thing is confusing for developers and site builders. Hence, closing this issue myself as duplicate (feeling pity about myself). My module has some extra features, but I guess I'll talk about them being integrated to the existing module, and see if the maintainer accepts it. Will be back with a new module to get the complete GIT access soon :)
Comment #5
littledynamo CreditAttribution: littledynamo commentedIts a pity that your module is a duplicate after all your hard work. All the best for your future Drupal efforts.
Comment #6
jcamposanok CreditAttribution: jcamposanok commented@littledynamo
After reviewing both the Share Count Statistics source code and this one, I think this one is actually better implemented and has more features. Share Count Statistics has many unsolved issues, lacks documentation / install / uninstall support, and does not look like it's actively maintained. I also haven't found a way to actually add new services using the API the module says it provides.
Furthermore:
- This project offers Panels integration while the Share Count Statistics module does not
- Views filters are more fine-grained, allowing to obtain results per service and total
- The statistics are obtained during cron runs instead of every time the node is accessed (allowing the user to configure via cron how often the social services API are accesed/queried, and reducing performance impact on large sites)
- This project allows the user to obtain the count of Facebook likes, comments and shares. The Share Count module only provides the latter
- It also allows to configure the services per node type
I also think merging both projects it's the best way to go, or even start a new branch of the mentioned module using this sandbox project as a base. However, it's important to acknowledge this project does provide different functionalities on its own and offers a greater benefit for the users in general who are looking for this kind of solution.
So in my opinion it shouldn't be considered as a duplicate yet, since they offer similar but not the same functionality.
@AjitS
I recommend you to get in contact with the module maintainer and let him know of this proposal. I would also gladly be interested in cooperating with this project as a co-maintainer.
Please let me know any way I can help, since I'm very interested in the functionality this project provides.
Comment #7
AjitS@jcamposanok:
Thank you for the interest in my work.
Please see the issue created at : #2103551: Offering to maintain Share Count Statistics module
Comment #8
AjitSEven if I become the maintainer of the module, does that mean I have to go through the review process again to get the full GIT access?
I have received some nice reviews on this module from my colleagues as well. I am hoping to get full GIT access with this. Can this be done? If yes, anything that I should do? I am ready to continue receiving the comments for improvements for the module; and I will happily work through them.
I have experience with maintaining the Soundslides module. Willing to contribute more :)
Comment #9
jcamposanok CreditAttribution: jcamposanok commentedGlad to know you want to keep improving the module and contributing to the community. I think at this moment you basically have two options:
1. Going through the entire review process detailed here in order to get your project promoted from sandbox to full module. You may also want to apply for a review bonus to speed things up
2. Wait for an answer to the possible duplication / fork / branch / integration scenarios with the author of Share Count Statistics (by the way, I already subscribed to that thread to see how things develop)
Even though I really want this module to be further improved and developed, I cannot grant you by myself the permission to promote this to a full module, since I may lose my "vetted user" git status because of the possible duplicity (see "Can I fork any project, even if I don't own or co-maintain it?" at the Drupal.org Git FAQ. The objective of the Drupal community is to encourage collaboration over competition, so I think going for option #2 may be the best way to go.
Meanwhile, you may get more people interested in this Project application and reach a general agreement that this is not a duplicate (as is my opinion). If the author of the other module does not provide you with an answer in about two weeks, we may look also for other options because that would mean the other project is either abandoned or unsupported.
Hope this helps.
Cheers!
Comment #10
dipen chaudhary CreditAttribution: dipen chaudhary commentedDisclaimer : I work with Ajit so I'm not sure how much my opinion will be weighed in seeing this through.
@jcamposanok has already mentioned detailed differences between the two modules in #6 above.
I just wish to add that I reviewed the initial version of this module and Ajit fixed all the issues in a day which says a lot about his enthusiasm for this module and his willingness to maintain it.
Comment #11
AjitSI'm glad to be a part of the community which always encourages to do more :-)
I think I will see this project review through, and see if I get the full GIT access. As of #7, I am planning to take over that module too (in case the module maintainer doesn't reply; marking the project as abandoned). I will do only bug fixes in that module, as I see there are many patches / suggestions which needs review in the module's issue queue.
Here are some more points (advantages) that makes social_stats different the other module (share_count):
Social Share Statistics collects the data on
hook_entity_view()
, which can affect the performance adversely.There is already and issue for this : #2083119: Performance problem with share_count_entity_load
as compared to share count module:
My plans maintaining this:
- This project will be actively maintained.
- Will fix whatever bugs I can in share_count module (if I get access to do so). And mention on its project page that there will only be bug fixes made and users should use this(social_stats) module instead. This way will be much better than just taking over the module and creating a new branch with my code. If there is a new branch created in the existing module, the upgrade path would be very confusing for most of the users. Users will get notified that there is an update, and if they update the module the site will brake as the schema of the modules is different. Moreover, the code style is completely different, and users (developers) who read the code would find it confusing while comparing the two branches.
- If I don't get access to the module, then I will be happy to maintain this with full commitment.
- Also, I worked through some issues in D8. So, #2107321: [meta] Drupal 8 port of Social Stats
Comment #12
Enxebre CreditAttribution: Enxebre commentedHi AjitS,
Becouse you are using function "node_type_get_types()" in social_stats.admin.inc file you should add "node module" dependecy in .info file.
In line 47:
'#default_value' => $variable ? $variable : NULL.
In my opinion you can achieve the same result in a cleaner way with:
'#default_value' => $variable_get($types->type, NULL)
Just an opinion :)
As an improvement it would be interesteing that the module would work with entities, not just nodes.
Hope this helps!
Regards!
Comment #13
AjitS@Enxebre : Thank you for the review.
node is a required module for Drupal. You can check the node.info file to check that it contains
required = TRUE
So, it is not necessary to add it separately as a dependency.
I've made change to line 47, to accommodate the change you've suggested.
I am aware that it will be helpful if this was entity based. However, I'm restricting the first version of the module to node_types. Maybe I'll support entity based statistics in future versions.
Thanks :)
Comment #13.0
AjitSUpdated issue summary.
Comment #14
Enxebre CreditAttribution: Enxebre commentedHi AjitS,
thanks for the information about the required attribute.
regards!
Comment #14.0
Enxebre CreditAttribution: Enxebre commentedUpdated issue summary.
Comment #14.1
AjitSUpdated issue summary.
Comment #15
AjitSAdding review bonus.
Comment #15.0
AjitSUpdated issue summary.
Comment #16
saheel.sikilkar CreditAttribution: saheel.sikilkar commentedWe will be using this in a major media channel's website (10,000 daily authenticated traffic). Site is in testing mode. No known issues till now. Works like charm.
The most fascinating thing is the granular level of control provided by this module.
Comment #16.0
saheel.sikilkar CreditAttribution: saheel.sikilkar commentedissue summary updated
Comment #17
AjitS@saheel.sikilkar : Thank you for the review. All the best for your project :-)
Comment #17.0
AjitSUpdated issue summary.
Comment #17.1
AjitSupdated issue summary
Comment #18
klausimanual review:
So the variable names is a blocker right now, and also the hook_cron() scalability issue is a t least major. Otherwise this looks pretty good :-)
Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #19
AjitSThank you for reviewing the module, and providing the insightful comments.
I check the time from which to select the nodes to update the stats.
social_stats_
, and in social_stats_uninstall I only delete the variables that begin with the module name.nid
as a primary key. Didn't needed to have a separatenode_type
column, so removed it.And I had also mentioned Elysia Cron, as a recommended module on the project page, which would allow the user to change when the cron can be run, even on a production site.
format_plural()
, andt()
wherever required for user facing text.db_query()
now instead ofdb_select()
.I had reviewed 6 projects. So, adding the review bonus back :-)
Comment #19.0
AjitSUpdated issue summary.
Comment #19.1
AjitSUpdated issue summary.
Comment #20
klausiLooks good to me now! You still don't need to set the variable in hook_install(), because variable_get() will just take the default value (the second parameter) if the variable does not exist in the DB.
Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Assigning to jthorson as he might have time to take a final look at this.
Comment #20.0
klausiUpdated issue summary.
Comment #21
AjitSThank you for the review :-)
I have removed the hook_install, and used the default value in variable_get.
Added reviews of three more projects in the issue summary.
Comment #22
klausino objections for more than a week, so ...
Thanks for your contribution, AjitS!
I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.
Here are some recommended readings to help with excellent maintainership:
You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!
Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
Thanks to the dedicated reviewer(s) as well.
Comment #22.0
klausiUpdated issue summary.
Comment #24
kandrupaler CreditAttribution: kandrupaler commentedHi, this module promises exactly what I want, so I installed it. However, I only see zeros for all shares, which I know is incorrect. I've run cron several times after installing, but no change. What am I doing wrong?
social_stats version: 7.x-1.7
drupal version: 7.38
social sharing buttons: Ridiculosusly Responsive Social Sharing Buttons (does it matter?)