[D7] Telegram Bot notification

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2699013

Project 2: https://www.drupal.org/node/2697809

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxVikkyShostak2698997git

Fixed the git clone URL in the issue summary for non-maintainer users.

We 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.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

You can help your module gain more priority and get reviewed by manually reviewing at least three modules.

Hi. Thanks for the application. Here is my review.

Info file:

package = Telegram

Why? Are there some other modules in this package? I would recommend leave this so it falls into default 'Other' group.

The file lacks configure link.

Module file:

// Settings page.

The comment is not very helpful in this case.

if (isset($form['type']) && arg(1) !== 'add') {

What is this about? I guess you need more specific form alter hook like hook_form_FORM_ID_alter() or hook_form_BASE_ID_alter().

$users_array = entity_load('user');

Doesn't it load all site users? This will cause high memory consumption if the site has a good number of registered accounts.

$text = $form['#node']->title

I don't know how telegram will handle this. Maybe you need to sanitize the title using check_plain().

$GLOBALS['base_url'] . '/node/' . $form['nid']['#value']

I would use url() function with absolute option.

file_get_contents('https://api.telegram.org/bot' . variable_get('telegram_bot_notification_access_token') . '/sendMessage?chat_id=' . $users_array[$key]->field_telegram_chat_id[LANGUAGE_NONE][0]['value'] . '&text=' . urlencode($text));

Usually CURL or drupal_http_request() are more preferable for making http requests. I also recommend you check telegram response here and log any errors to watchdog.

unset($_GET['destination']);

Why do you remove the destination?

drupal_redirect_form($form_state);

You've set redirect destination twice before. Doesn't it work without this? Btw, why do you need the redirect at all?

Admin file

'#weight' => 0

It's default weight. If you configure form elements sequentially you don't have to specify their weights.

// How to create Telegram bot.

Same as about settings page. The comment duplicates the element title.

t('<p>To create your Telegram Bot, follow this instructions:</p>
                      <ol>
                        <li>Message <a href="https://telegram.me/botfather" target="_blank">@BotFather</a> with the following text: /newbot;</li>
                        <li>Type whatever name you want for your Bot;</li>
                        <li>Type whatever username you want for your bot, minimum 5 characters, may use «_» symbol, and must end with bot For example: mysample_bot or MySampleBot;</li>
                        <li>Copy token to access the HTTP API, paste to «Specify access token» field and save this form;</li>
                        <li>Type /setprivacy and choose your bot;</li>
                        <li>Choose Enable, if your bot will only receive messages that either start with the «/» symbol or mention the bot by username. Choose Disable, if your bot will receive all messages that people send to groups;</li>
                        <li>To change additional parameters — type /help to @BotFather.</li>
                      </ol>
                      <p>If you looking for screencast, see <a href="http://blizzy.ru/node/11">this blog post</a>.</p>'),

The text is too long. It's usually recommended to split such texts into small translatable strings as follows:

$description[] = t('Sentence one.');
$description[] = t('Sentence two.');
$description[] = t('Sentence three.');

Then you can implode it or output as a list using theme_item_list(). Also try to avoid using markup inside t().

If you looking for screencast, see this blog post

Don't put urls into translatable strings but use placeholder instead like this: t('If you looking for screencast, see <a href="!url">this blogpost</a>.', array('!url' => 'http://blizzy.ru/node/11'); In this particular case I would move this to README.txt.

'#title' => 'Enter your Telegram Bot access token',
'#title' => 'Enter machine name of node type',

These strings need to be translatable.

'#default_value' => variable_get('telegram_bot_notification_node_type', ''),

'' can be omitted.

'#description' => t('<p>Single or multiple (comma separated). For example: <em>article, page, blog_post</em>.</p>'),

Keep <p/> outside t() as follows:

'#description' => '<p>' . t('Single or multiple (comma separated). For example: <em>article</em>, <em>page</em>, <em>blog_post</em>.') . '</p>';

The content type form element obviously needs validation handler.

Install file

field_create_field(array(
    'field_name' => 'field_telegram_chat_id',
    'type' => 'text',
    'cardinality' => 1,
  ));

There is a small chance that the field with a given name already exists. I would check it first.

All variables declared in module should be deleted in hook_uninstall().

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.