[D8] Open Weather

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

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.

Module is not working for me

I am getting "GuzzleHttp\Exception\ClientException: Client error: `GET http://api.openweathermap.org/data/2.5/weather?appid=15893c78d7f435f4908...` resulted in a `401 Unauthorized` response: {"cod":401, "message": "Invalid API key. Please see http://openweathermap.org/faq#error401 for more info."} in GuzzleHttp\Exception\RequestException::create() (line 107 of /var/www/html/drupal-8/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php)." Error.

Please see the screenshot

File : WeatherBlock.php

76
'#default_value' => $config['input_value'],
This will throw an undefined error as you have not given the element a default value, if is not set.

Line 130, 131

'#collapsible' => FALSE,
      '#collapsed' => FALSE,

This makes no sense, i am sure why it's working though :P Refer to: https://api.drupal.org/api/drupal/developer!topics!forms_api_reference.h...

Line 155
$message = $token_service->replace($form_state->getValue('input_value'), array('user' => $user));

$user is undefined.

$token_service = \Drupal::token();
    $message = $token_service->replace($form_state->getValue('input_value'), array('user' => $user));

You may these lines to only execute only when module token exists.

Line 190

case 'forecast_daily':
        $build = [
          '#markup' => $this->t('Working on this functionality'),
        ];
        break;
    }

You can work to extend this functionality later.

File: openweather.routing.yml

_permission: 'administer module openweather'
You may want add a permissions.yml file :) Refer to: #2311427: hook_permission() replaced with permissions defined in a $module.permissions.yml file

File: openweather.libraries.yml

It will look good if you can introduce some spacing b/w two declaration :)

Hi prashant114606,
Wheather block shown successfully based on the city but am not able to see proper timings as per the city given. Is there anything am missing?

one suggestion:
It would be good if we have the hook_help

https://www.drupal.org/docs/develop/documenting-your-project/module-docu...

otherwise its good

Hi @prashant114606,

Automated Review

FILE: ...oot/repos/pareviewsh/pareview_temp/src/Plugin/Block/WeatherBlock.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
170 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
--------------------------------------------------------------------------

Recommendation: 'token' service should be injected.

  /**
   * Returns the token service.
   *
   * @return \Drupal\Core\Utility\Token
   *   The token service.
   */
  public static function token() {
    return static::getContainer()->get('token');
  }

Manual Review

Individual user account

[Yes: Follows / No: Does not follow] the guidelines for individual user accounts.

No duplication

No duplication found.

Master Branch

[Yes: Follows] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

3rd party assets/code

[Yes: Follows] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.]

Coding style & Drupal API usage

No major issues found except the one identified above regarding dependency injection of token service. Otherwise code looks good to me both functionality wise and code vise.

This review uses the Project Application Review Template.

Also @prashant114606, kindly get Review Bonus to speed up your own project through the review process and you will learn a lot.
https://www.drupal.org/node/1975228

Best,
PK

This is clear to go from my end :) Issuing RTBC

project page is empty: https://www.drupal.org/sandbox/prashant114606/2839644

Can you describe what your module does, how it works etc.? See also https://www.drupal.org/node/997024

manual review:

1. git branches: which one should be reviewed, 8.x-1.x or 8.x-2.x? Please delete all git branches that might not be needed anymore. I'm reviewing the 8.x-1.x branch because the most recent git commits are there.
2. class WeatherService: do not use \Drupal in service classes, use dependency injection instead by specifying all services as parameters in your *.services.yml file.
3. WeatherService::createRequest(): why do you call Html::escape() here? You are not printing anything to HTML here, so there is no XSS threat? I think those calls should be removed in this method.
4. getCurrentWeatherInformation(): why is the cache max age set to 0 here? Please add a comment. Does it make sense to cache the information for a short period of time? Same in the other methods on WeatherService.
5. openweather-dailyforecast.html.twig: all user facing text must run through the translation system. For example "Geo coords ..." here. See https://www.drupal.org/docs/8/api/translation-api/overview for translation in Twig files.

But that are not application blockers, otherwise looks good to me.

Thanks for your contribution, Prashant!

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:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.