Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Third-Party API for Weather Data #1

Closed
wants to merge 2 commits into from

Conversation

axelerant-hardik
Copy link
Collaborator

No description provided.

use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* Provides a 'Hello' Block.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axelerant-hardik Please provide proper comments for the class or plugin

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some copy and paste stuff here. I will correct this.

return [];
}

// Get location details from session.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axelerant-hardik Why does the comment say "Get location details from the session"? I think the city will be stored in the block configuration, not the session.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're correct. I will update the comment.

Comment on lines 132 to 155

// Endpoint that should be hit.
$request_url = $weather_settings->get('base_url') . '/current.json';
// Make a HTTP GET request to the endpoint.
try {
$request = $this->httpClient->request('GET', $request_url, [
'query' => [
'key' => $weather_settings->get('api_key'),
'q' => $city,
'units' => 'metric',
],
]);

// Parse the response.
$response = Json::decode($request->getBody());
$weather_data = [];
$weather_data['location'] = $response['location']['name'] . ', ' . $response['location']['region'] . ', ' . $response['location']['country'];
$weather_data['temperature'] = $response['current']['temp_c'];
$weather_data['feels_like'] = $response['current']['feelslike_c'];
$weather_data['weather_condition_icon'] = $response['current']['condition']['icon'];
$weather_data['weather_condition_text'] = $response['current']['condition']['text'];
$weather_data['wind'] = $response['current']['wind_kph'];
$weather_data['precipitation'] = $response['current']['precip_mm'];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axelerant-hardik I think it would be better to add this business logic inside the service file. Hence we can call service anywhere if it is required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iyyappan-axelerant I was thinking of having a service file initially then dropped the idea considering it would be used only in this location. But, yes from a wider POV, it makes sense to have this as a service. I can add a service file!

Thank you for reviewing the PR and providing feedback!

Comment on lines +101 to +107
$city = $this->configuration['city'];
if (!$city) {
$city_not_found_message = $this->t('No city entered. Please enter a city to see the weather details');
return [
'#markup' => '<div class="weather--error">' . $city_not_found_message . '</div>',
];
}
Copy link
Contributor

@axelabhay axelabhay May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axelerant-hardik can you help in which situation users will see this message? Asking this as I see this field as required.

$form['city'] = [
'#type' => 'textfield',
'#title' => $this->t('City'),
'#description' => $this->t('Please enter the city to show weather details. Eg: Mumbai, India'),
'#default_value' => $this->configuration['city'],
'#required' => TRUE,
];

@axelabhay
Copy link
Contributor

Closing this PR on behalf of #6.

@axelabhay axelabhay closed this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants