[D7] Pay with a Tweet

fixed.

Hello,

Install File
In Hook_install - "variable_del('pay-with-a-tweet');", this should be deleted in hook_uninstall

changed!

Module File
Line 162 - "hook_entity_entity_delete", should be hook_entity_delete

changed!

Admin INC
Line 135 - "variable_set_value(PAY_WITH_A_TWEET, $settings);" Function definition missing "variable_set_value"

changed!

Library Files
If your Oauth library is not hacked / patched, you can provide the link for it, rather than providing it with the module.
Same goes for twitteroauth library.

I have deleted the oauth library. Now, Pay with a Tweet has oAuth module as dependency (https://drupal.org/project/oauth).

I have not deleted the file lib/twitteroauth.php. I had to remove the require to oauth library and anothers changes to drupal coding standards

correct development version git clone

Hello brice_gato,

changed oauth_common to oauth

thanks!!

I checked your info. The oauth_common module is obsolte, but the dependency name of oauth module (https://drupal.org/project/oauth) is oauth_common, the oauth doesn't exists. So I revert this change.

You can try installing the module pay_with_a_tweet with oauth dependence to check that it does not work. With dependence oauth_common and oauth module (https://drupal.org/project/oauth) installed, the installation is performed without problem.

Changed twitter api version to 1.1

Hello WebEvt,

Good idea for a project. Did you think about an integration with commerce and commerce_file module?

Sounds good. Maybe in a future version. I think it's better than the first version do the basics well and that future versions complicate

pay_with_a_tweet_theme(): there is no need for using drupal_get_path() twice (line 116, 121) - you can use $path variable that comes to the hook_theme() as a fourth argument. See hook_theme()

Great! I don't know it. Checked and changed.

pay_with_a_tweet_render(), line 269: I think you can use url() function here.

I have seen that $base_url and url() were necessary.

pay_with_a_tweet_twitter_connect(), line 52: there's no need for drupal_exit() here, because drupal_goto() does it for you. The same issues occurs in your several other functions.

Checked and changes.

Did you think of participating in a review bonus program? This will speed up the processing of your application.

I know and is my pending action :(

Thank you!

Note: With the changes it's necessary clear cache if you already have installed the module

Hello flebrenn,

thanks for your testing!!

I applied your patch.

I checked your #12 message.

There is a problem with the session that happens when twitter not redirect to callback url. It's a punctual error but can be regular when in dev.twitter.com you have not configured the callback value

I've checked it and I added a error message when this problem appears.

Hi gauravjeet_singh,

thanks a lot for your comments!!

.module file line 333, 338
-You can use require_once() instead of module_load_include()

I had used module_load_include for use the drupal api and include the files with their absolute route. I checked another projects (like coder) and it uses the require_once when the file to include is in the same module. So I changed this lines to:

/**
 * include the block functions file
 */
require_once realpath(__DIR__) . '/pay_with_a_tweet.blocks.inc';

/**
 * include the tokens functions file
 */
require_once realpath(__DIR__) . '/pay_with_a_tweet.tokens.inc';

.install file
- Not all variables have been deleted - pay_with_a_tweet_download, anonymous

The pay_with_a_tweet_download and pay_with_a_tweet_image variables doesn't exist. So It's not necesary delete it. But this variables can't be configured so I removed it from the code (.admin.inc file lines 215 and 223).

The variable anonymous is a drupal core variable. This variable is set when you change the name for anonymous users in http://www.example.com/admin/config/people/accounts. So I can't remove this variable in the uninstall hook ;)

.admin.inc file line 31
- wrong usage of t() function, use placeholders instead that would be helpful in translation.
Instead of :
$table['empty'] = t('There are no downloads available.') . ' ' . l(t('Create a download'), 'admin/pay_with_a_tweet/add');
you can use :
$table['empty'] = t('There are no downloads available. !create', array('!create' => l('Create a download'), 'admin/pay_with_a_tweet/add'));

changed!

.admin.inc file line 33
- please use addTag('node_access') to avoid a security risk. There are some nodes which have restricted access, not using addTag() in the query would bypass them

I don't use addTage('node_access') because the query doesn't call to node data. Pay_with_a_tweet is an own entity (like nodes, taxonomies, users, ...) and doesn't exist any restricted access to the pay_with_a_tweet entity content.

.admin.inc file line 138
- is there really need for intval() ? This is really a 'no problem' but still it's of no use because a checkbox in form API either returns a 0 or a 1.

Yes, the intval isn't necesary. I remove it.

INSTALL.txt file
- There is no need to include an extra descriptive file, you can add this description in README.txt itself

done!

New dependency: OAuth (https://drupal.org/project/oauth)

Reviews of other projects

https://drupal.org/node/2052349#comment-7697195
https://drupal.org/node/2052217#comment-7697381
https://drupal.org/node/2052201#comment-7697313

Hello hapax legomenon,

Your comments are correct!!

• I changed in hook_menu the access arg "create pay_with_a_tweet" to "administer pay_with_a_tweet".
• In .module I removed module_load_include('php', 'oauth_common', 'lib/OAuth'); and I put it on this .pages.inc functions: pay_with_a_tweet_twitter_connect and pay_with_a_tweet_twitter_callback
• In .pages.inc I updated the code comments to inline-comment (the comments inside functions)

thanks!

Hello klausi,

Thanks for your comments. You make me work! ;)

pay_with_a_tweet_uninstall(): Do not use db_select() for simple static queries, use db_query() instead. See http://drupal.org/node/310075 . Same in pay_with_a_tweet_get_all().

fixed!

pay_with_a_tweet_entity_delete(): this looks wrong, because this hook will fire on deletion of any entity. So if i have a profile2 entity with a pid this iwll delete some arbitrary pay_with_a_tweet entuity. Why do you even need this hook? Please add a comment.

You are right. It's my fault. I don't want use hook_entity_delete. I changed the name to pay_with_a_tweet_delete because the function will be called when you confirm deleted a pay_with_a_tweet button.

pay_with_a_tweet_save(): the "&" is useless.

fixed!

pay_with_a_tweet_block_save(): The check_plain() calls are wrong here. "When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database". See https://drupal.org/node/28984

Changed! I apply this comment to all check_plain

PayWithATweetController: why do you need your own controller class and cannot use the DrupalDefaultEntityController? Please add a comment.

You are right. I don't need my own controller. I changed the entity to use the DrupalDefaultEntityController and refactor functions pay_with_a_tweet_save and pay_with_a_tweet_delete to save/delete the entity without the PayWithATweetController.

pay_with_a_tweet_render(): this looks like a preprocess function - so you should implement a theme preprocess hook so that this called automatically whenever the theme function is invoked.

I didn't know this hook. Implemented!!

pay_with_a_tweet_edit(): this can be removed, just use drupal_get_form as page callback in hook_menu() directly.

Fixed. Applied to another functions that do the same.

pay_with_a_tweet_info(): this is where the check_plain() class on title and message should happen, not in pay_with_a_tweet_form_submit().

fixed!

pay_with_a_tweet_info(): duplication of Views? You can list entities already with Views and the Entity API module.

I try to explain me. I did it so because I think that is excessive to force install entity API or views for a single screen. The module has the oauth module like dependency because is really required, but the use of views or entity api to a screen that lists the buttons created does not seem reason enough to force other dependencies. In future versions is a good idea use views and entity api when this modules are installed (but not like dependency), I hope that now the pay_with_a_tweet_info is not a problem.

Thanks!

Add reviews of other projects

Reviews of other projects

https://drupal.org/node/2049679#comment-7703109
https://drupal.org/node/2050019#comment-7703177
https://drupal.org/node/2040297#comment-7703217

Hello klausi,

First at all, thanks you! I learned a lot with your messages.

"'%title' => check_plain($pay_with_a_tweet->title) . ' [' . $pay_with_a_tweet->pid . ']',": no need to run check_plain() here since the "%" placeholder will sanitize the variable for you already. And double escaping is bad. Also elsewhere.

I removed the check_plain when I use the "%" placeholder.

pay_with_a_tweet_delete_form(): If a form has a submit function, then hidden form values are not needed. Instead, any values that you need to pass to $form_state['values'] can be declared in the $form array as '#type' => 'value'. Also elsewhere.

I didn't know that. Changed and working fine!

twitteroauth.php: appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

Sorry. Yes I read the sections that you show me before programming the module. But I understood that the not inclusion of third-party code was more a suggestion than an obligation. Clearly I was wrong for what I have already corrected the error.

I deleted the code and created my own class to take the necessary actions (getting permission to post to twitter and posting a tweet) and only those.

So, I hope that with these changes, the module is as it should be.

Thanks a lot!

Reviews of other projects

https://drupal.org/node/2004572#comment-7709699
https://drupal.org/node/2048779#comment-7709759
https://drupal.org/node/2054613#comment-7709989

add new reviews of other projects

Hello

I fix a bug in hook_uninstall (it generated a notice) and add {} to the table name in the db_select() (in hook_uninstall and pay_with_a_tweet_get_all()

thanks!