[2.0.x] Social Feeds Block

Hello @Jitendra verma
I think the module looks fine,
Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

Thanks

I have reviewed the project code. It looks fine.

Hello @vishal.kadam,
Check the version we wish to evaluate [2.0.x] and you are evaluate [8.x-1.x],
Read the issue careful about jumping to conclusions.

OPTIONAL

Not an issue, just an observation. Pintrest is the wrong spelling. Use Pinterest.

Manual Findings:

1. Fix these typos Cridential,Pintrest

2.   public function __construct(ConfigFactoryInterface $config) {
   
       parent::__construct($config);
       $this->config = $config->get('config.facebook_social_feeds_block');
   
     }

   you don't need to override the function, as you will get the $this->config('') in the build form itself. Please remove it from all the config forms.

3. $fbfeeds = "https://graph.facebook.com/" . $fb_app_name . "/feed?access_token=" . $fb_app_id . "|" . $fb_secret_id . '&fields=link,message,description&limit=' . $fb_no_feeds;
   $response = $this->httpClient->get($fbfeeds, ['headers' => ['Accept' => 'text/plain']]);
   $url = "https://api.instagram.com/v1/users/self/media/recent/?access_token=" . $insta_access_token . '&count=' . $insta_pic_counts;
   
   $response = $this->httpClient->get($url, ['headers' => ['Accept' => 'text/json']]);
   $request = $response->getBody()->getContents();
   
   $url = "https://api.pinterest.com/v3/pidgets/users/" . $pintrest_user_name . "/pins";
   $response = $this->httpClient->get($url, ['headers' => ['Accept' => 'text/json']]);
   $feeds = $response->getBody()->getContents();
   

   all these client requests must be in the try catch blocks and all the exceptions must be logged so that admin can know what is broken and a message for the end user.

4. remove # from social_feeds_block.routing.yml as they seems odd.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

src/Form/FacebookSocialFeedsBlockForm.php

class FacebookSocialFeedsBlockForm extends ConfigFormBase {

  /**
   * Constructor for SocialFeedsBlockForm.
   *
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   *   The factory for configuration objects.
   */
  public function __construct(ConfigFactoryInterface $config_factory) {

    parent::__construct($config_factory);

  }

The documentation comment for the constructor is not for that class.

  /**
   * Returns a unique string identifying the form.
   *
   * @return string
   *   The unique string identifying the form.
   */
  public function getFormId() {
    return 'facebook_social_feeds_admin_form';
  }

Documentation of methods defined in an interface or the parent class needs to just contain {@inheritdoc}.

    $form['social_feeds_block_fb'] = [
      '#type' => 'fieldset',
      '#title' => $this->t('Facebook Cridential'),
      '#weight' => 50,
      '#collapsible' => TRUE,
      '#collapsed' => TRUE,
    ];

It is Credential, not Cridential. Form element tiles and descriptions need to use sentence case.

src/Form/InstagramSocialFeedsBlockForm.php

/**
 * Configure custom settings for this site.
 */
class InstagramSocialFeedsBlockForm extends ConfigFormBase {

  /**
   * Constructor for SocialFeedsBlockForm.
   *
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config
   *   The factory for configuration objects.
   */
  public function __construct(ConfigFactoryInterface $config) {

    parent::__construct($config);
    $this->config = $config->get('config.pintrest_social_feeds');

  }

  /**
   * Returns a unique string identifying the form.
   *
   * @return string
   *   The unique string identifying the form.
   */
  public function getFormId() {
    return 'instagram_social_feeds_admin_form';
  }

  /**
   * Gets the configuration names that will be editable.
   *
   * @return array
   *   An array of configuration object names that are editable if called in
   *   conjunction with the trait's config() method.
   */
  protected function getEditableConfigNames() {
    return ['config.instagram_social_feeds_block'];
  }

  /**
   * Form constructor.
   *
   * @param array $form
   *   An associative array containing the structure of the form.
   * @param \Drupal\Core\Form\FormStateInterface $form_state
   *   The current state of the form.
   *
   * @return array
   *   The form structure.
   */
  public function buildForm(array $form, FormStateInterface $form_state) {
    $instagram_social_feeds = $this->config('config.instagram_social_feeds_block');

    // Instagram fieldset.
    $form['instagram_social_feeds'] = [
      '#type' => 'fieldset',
      '#title' => $this->t('Instagram Cridential'),
      '#weight' => 50,
      '#collapsible' => TRUE,
      '#collapsed' => TRUE,
    ];

    $form['instagram_social_feeds']['insta_client_id'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Client ID'),
      '#default_value' => $instagram_social_feeds->get('insta_client_id'),
      '#maxlength' => 255,
      '#required' => TRUE,
    ];

    $form['instagram_social_feeds']['insta_redirec_uri'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Redirect URI'),
      '#default_value' => $instagram_social_feeds->get('insta_redirec_uri'),
      '#maxlength' => 255,
      '#required' => TRUE,
    ];

    $form['instagram_social_feeds']['insta_access_token'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Access Token'),
      '#default_value' => $instagram_social_feeds->get('insta_access_token'),
      '#maxlength' => 255,
      '#required' => TRUE,
    ];

    $form['instagram_social_feeds']['insta_image_resolution'] = [
      '#type' => 'select',
      '#title' => $this->t('Image Resoluction'),
      '#options' => [
        'thumbnail' => $this->t('Thumbnail'),
        'low_resolution' => $this->t('Low Resolution'),
        'standard_resolution' => $this->t('Standard Resolution'),
      ],
      '#default_value' => $instagram_social_feeds->get('insta_image_resolution'),
      // '#maxlength' => 255,.
      '#required' => TRUE,
    ];

    $form['instagram_social_feeds']['insta_likes'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('Likes Count'),
      '#default_value' => $instagram_social_feeds->get('insta_likes'),
    // '#maxlength' => 255,
    // '#required' => TRUE,.
    ];

    $form['instagram_social_feeds']['insta_pic_counts'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Picture Count'),
      '#default_value' => $instagram_social_feeds->get('insta_pic_counts'),
      '#maxlength' => 255,
      '#required' => TRUE,
    ];

    return parent::buildForm($form, $form_state);

  }

  /**
   * Form submission handler.
   *
   * @param array $form
   *   An associative array containing the structure of the form.
   * @param \Drupal\Core\Form\FormStateInterface $form_state
   *   The current state of the form.
   */
  public function submitForm(array &$form, FormStateInterface $form_state) {

    $this->config('config.instagram_social_feeds_block')
      ->set('insta_client_id', $form_state->getValue('insta_client_id'))
      ->set('insta_redirec_uri', $form_state->getValue('insta_redirec_uri'))
      ->set('insta_access_token', $form_state->getValue('insta_access_token'))
      ->set('insta_pic_counts', $form_state->getValue('insta_pic_counts'))
      ->set('insta_image_resolution', $form_state->getValue('insta_image_resolution'))
      ->set('insta_likes', $form_state->getValue('insta_likes'))
      ->save();

    parent::submitForm($form, $form_state);

  }

}

The form to configure a plugin is returned by its buildConfigurationForm() method. There is no need to use a separate form class.
#maxlength is not used for a checkbox form element. The line setting it should not be there, even if commented out.

src/Plugin/Block/FacebookFeedsBlock.php

    $values = $this->configfactory->getEditable('config.facebook_social_feeds_block');

    $fb_app_name = $values->get('fb_app_name');
    $fb_app_id = $values->get('fb_app_id');
    $fb_secret_id = $values->get('fb_secret_id');
    $fb_no_feeds = $values->get('fb_no_feeds');

A plugin's configuration is passed in $configuration to its constructor; there should not be any reason to use a configuration object. getEditable() is not necessary, when the values in the configuration object are not changed nor is the configuration object saved.

    return [
      '#theme' => 'fb_social_feeds_block',
      '#data' => $fb_feeds,
      '#error_message' => $error_message,
    ];

Strings shown to the users need to be translatable.

social_feeds_block.info.yml

core: 8.x
core_version_requirement: ^8 || ^9

Now that Drupal 8 is no longer supported, core is not necessary for new projects and core_version_requirement needs to be updated.

Thank you for the feedback.

All the code review comments have been addressed and committed into the branch - 2.0.x . Please validate the same and let me know please if there is any observation.

Git repo URL - https://git.drupalcode.org/project/social_feeds_block/-/tree/2.0.x
Branch - 2.0.x

The form to set a block is returned by its blockForm() method. There is no need to create a separate form class. See BookNavigationBlock::blockForm() as example.

src/Controller/InstagramAuthController.php

    return [
      '#markup' => $this->t('@message @token', [
        '@message' => $message,
        '@token' => $token,
      ]),
    ];

The content of $message is not translated, in that way. The only way to make it translatable is to write the message in the literal string passed as first parameter to t().

src/Plugin/Block/FacebookFeedsBlock.php

  /**
   * The field names to retrieve from Facebook.
   *
   * @var array
   */
  protected array $fields = [
    'picture',
    'permalink_url',
    'status_type',
    'message',
  ];

Drupal 9 still requires PHP 7.3. A module that uses PHP 7.4 or PHP 8 features should explicitly require that PHP version, or require Drupal 10.x, which requires PHP 8.1 or higher versions.

      catch (\Exception $exception) {
        $this->logger->error($this->t('Exception: @exception', [
          '@exception' => $exception->getMessage(),
        ]));

The $message parameter passed to the LoggerInterface methods must be a literal string, not a translatable string.
For exceptions, the function used to log them should be watchdog_exception(), or code similar to that should be used.

    else {
      $error_message = 'API Credentials are Missing.';
    }

    return [
      '#theme' => 'fb_social_feeds_block',
      '#data' => $fb_feeds,
      '#error_message' => $error_message,
    ];

Strings shown to users need to be translatable.

$config = $this->configfactory->getEditable('config.facebook_social_feeds_block');
$permanent_token = $config->get('page_permanent_token');

If the configuration object is not changed, there is no need to call getEditable().

social_feeds_block.info.yml

core: 8.x
core_version_requirement: ^8 || ^9

Now that Drupal 8 is no longer supported, core is not necessary for new projects and core_version_requirement needs to be updated.

Hello @apaderno sir,
I think you are checking version 1.0.x as the core in social_feeds_block.info.yml has already been removed, and I've included a picture to this comment.

The rest is still to be changed, though.

src/Form/TwitterSocialFeedsBlockForm.php

    $form['twitter_social_feeds'] = [
      '#type' => 'fieldset',
      '#title' => $this->t('Twitter Credential'),
      '#weight' => 50,
      '#collapsible' => TRUE,
      '#collapsed' => TRUE,
    ];

    $form['twitter_social_feeds']['tw_api_key'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Twitter API Key'),
      '#default_value' => $config->get('tw_api_key'),
      '#maxlength' => 255,
      '#required' => TRUE,
    ];

As a side note, we do not use capital case for all the words in a form element title or description.

@apaderno
All the code review comments have been addressed and committed into the branch - 2.0.x . Please review.

Git repo URL - https://git.drupalcode.org/project/social_feeds_block/-/tree/2.0.x
Branch - 2.0.x

Automatically closed - issue fixed for 2 weeks with no activity.