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

Fix the "Show the Local Weather" project #15598

Closed
QuincyLarson opened this issue Jul 5, 2017 · 22 comments
Closed

Fix the "Show the Local Weather" project #15598

QuincyLarson opened this issue Jul 5, 2017 · 22 comments
Labels
help wanted Open for all. You do not need permission to work on these.

Comments

@QuincyLarson
Copy link
Contributor

QuincyLarson commented Jul 5, 2017

Challenge Name

Show the local weather

Issue Description

Currently this project has at least two major problems:

  1. It recommends more than one API. We should just have a single API that reliably works on CodePen
  2. It suggests putting private keys in a visible place on Codepen. This is an anti-pattern.

Here's what I propose we do. We build a Glitch.com app that serves as an API. The Glitch App can then reach out to the Dark Sky (or whatever API works best) on the user's behalf and pass back the data.

This way, the keys are obscured, and the user has a simplified API they can interact with. There's no need for them to shop around for APIs or worry about security issues with CodePen.

We have already done something like this with our Twitch.tv challenge.

@erictleung
Copy link
Member

Possibly related issues:

@erictleung erictleung added codepen help wanted Open for all. You do not need permission to work on these. labels Jul 5, 2017
@MiloATH
Copy link
Contributor

MiloATH commented Jul 5, 2017

The issue with weather APIs is that most (and maybe all) have a max number of free calls per day. Dark Sky has a max at 1000 calls per 24 hours. We set the API to default to using the longitude and latitude requested and store all the calls of the last 24 hours. Then, if more than 1000 calls are requested in 24 hours, the API sends the data from the closest stored longitude and latitude to the requested longitude and latitude.

@MiloATH
Copy link
Contributor

MiloATH commented Jul 5, 2017

Something like this: https://fcc-weather-api.glitch.me? Uses similar index.html as the Twitch API. Right now, it just uses the OpenWeatherMap which allows for 60 requests per minute. Right now, only geographic coordinate requests are supported. I am willing to work on this more.

@QuincyLarson
Copy link
Contributor Author

@MiloATH Wow - this is perfect! Excellent work. I think we just need to handle the case where we've exceeded the API limits and show a message. If this is caching results, we should be fine.

Would you be willing to get this working with the existing example app? https://codepen.io/freeCodeCamp/full/bELRjV

If you can give me the updated JavaScript for that app, I can use it to update the example app.

Note that we have moved the weather app out of the core curriculum and over to supplemental challenges on https://beta.freecodecamp.com, so soon the rate limit shouldn't be that big of a deal.

@MiloATH
Copy link
Contributor

MiloATH commented Jul 7, 2017

I forked the freeCodeCamp codepen and updated the code. Here is the forked codepen. For the icons, I provide the link to the images on the glitch cdn. I will add caching results next. Then, I will update the instructions in the challenge and make a PR.

Right now, weather can only be requested by longitude and latitude. City or zip code searches code be added fairly easily. Here is all that is currently working.

API requests:

  • /api/current
    • Required parameters:
      • lon is longitude
      • lat is latitude
    • Optional parameters:
      • callback can be set to JSON_CALLBACK

The temperature is returned in celsius. Do we need to add other units options as optional parameters?

Do any other types of API requests or request options need to be handled?

Examples:
https://fcc-weather-api.glitch.me/api/current?lat=35&lon=139
https://fcc-weather-api.glitch.me/api/current?lat=35&lon=139&callback=JSON_CALLBACK

Note: my codepen linked above uses a request to ipinfo.io for the longitude and latitude. However, it could be implemented to just use HTML5 Geolocation.

@MiloATH
Copy link
Contributor

MiloATH commented Jul 7, 2017

I read through OpenWeatherMap's faq, and open source projects can get the access limits lifted.

Here is from http://openweathermap.org/faq

For FOSS developers: we welcome free and open source software and are willing to help you. If you want to use OWM data in your free software application please register an API key and file a ticket describing your application and API key registered. OWM will review your request lift access limits for your key if used in open source application.

@QuincyLarson I can file a ticket for FreeCodeCamp to OpenWeatherMap if this is something FreeCodeCamp believes this is a good idea.

This means we wouldn't have to cache results.

I would have to make the code on glitch open source, but that really wouldn't change much.

This would be nice, but may not be worth the time since this challenge will become optional. Also, the free API key allows for 60 requests every minute which is fast enough. I don't think we are going to need caching very much using the current free API plan.

@MiloATH
Copy link
Contributor

MiloATH commented Jul 9, 2017

The freeCodeCamp codepen javascript still needs to be updated. Here is the javascript:

var app = angular.module('Weather', []);

app.factory('WeatherApi', function($http) {
  var obj = {};
  
  obj.getLoc = function() {
    return $http.jsonp("https://ipinfo.io/json?callback=JSON_CALLBACK");
  };
  obj.getCurrent = function(loc) {
    var api = "https://fcc-weather-api.glitch.me/api/current?";
    loc = loc.split(",");
    var lat = "lat=" + loc[0];
    var lon = "&lon=" + loc[1];
    var cb = "&callback=JSON_CALLBACK";
    return $http.jsonp(api + lat + lon + cb);
  };
  return obj
});

app.controller('MainCtrl', function($scope, WeatherApi) {
  $scope.Data = {};
  $scope.Data.unit ='C';
  $scope.Data.sysChange = false;
  WeatherApi.getLoc().success(function(data) {
    var city = data.loc + ',' + data.country;
    $scope.Data.city = data.city;
    $scope.Data.country = data.country;
    WeatherApi.getCurrent(data.loc).success(function(data) {
      CurrentWeather(data)
    });
  });

  function CurrentWeather(data) {
    $scope.Data.temp = Math.round(data.main.temp);
    $scope.Data.Cel = Math.round(data.main.temp);
    $scope.Data.des = data.weather[0].main;
    $scope.Data.Fah = Math.round( ($scope.Data.temp * 9)/5 + 32 );
    return IconGen($scope.Data.des);
  }

  function IconGen(city) {
    var city = city.toLowerCase()
    switch (city) {
      case 'drizzle':
        addIcon(city)
        break;
      case 'clouds':
        addIcon(city)
        break;
      case 'rain':
        addIcon(city)
        break;
      case 'snow':
        addIcon(city)
        break;
      case 'clear':
        addIcon(city)
        break;
      case 'thunderstom':
        addIcon(city)
        break;
      default:
    $('div.clouds').removeClass('hide');
    }
  }

  function addIcon(city) {
    $('div.' + city).removeClass('hide');
  }
  
  $scope.Data.sys= function(){
   if($scope.Data.sysChange){
     $scope.Data.unit ='C';
     $scope.Data.temp = $scope.Data.Cel;
     return $scope.Data.sysChange = false;
     }
    $scope.Data.unit ='F';
    $scope.Data.temp = $scope.Data.Fah;
    return $scope.Data.sysChange = true;
  }
  
  
});

Also, the challenge is still on the master branch with old instructions for using API keys.

@john8kelvin
Copy link

nice

@aw1231
Copy link

aw1231 commented Jul 14, 2017

Per the discussion on freeCodeCamp/testable-projects-fcc#133 I think this issue should reopen.

@raisedadead
Copy link
Member

@aw1231 thanks for letting us know.
@MiloATH I have updated the official codepen.

Potential contributors:
I am re-opening this, and tagging this for updating the instructions. Can we please branch off backup/master update the instructions and make a pull request against the same?

Let us know in the Contributors Chat room if you need assistance.

@raisedadead raisedadead reopened this Jul 15, 2017
@raisedadead
Copy link
Member

Also it would be awesome if you could reference any related duplicate issues that you find on the tracker.

@MiloATH
Copy link
Contributor

MiloATH commented Jul 15, 2017

@raisedadead I made a PR that updated the instructions on backup/master. See #15619.
And thanks for updating the codepen! 😄

@marigerr
Copy link
Contributor

@MiloATH one other thing I noticed, is that my ad-blocker uBlock Origin blocks ipinfo.io , so that the weather doesn't come back unless I disable uBlock temporarily. uBlock is a pretty popular extension that many campers might use, so maybe would be useful to add a try-catch block in the codepen so that if if the ipinfo.io call is blocked it will display a some sort of message to user that they need to allow ipinfo.io or temporarily disable their ad-blocker to view the app.

with uBlock:
$http.jsonp("https://ipinfo.io/json?callback=JSON_CALLBACK");
returns
Failed to load resource: net::ERR_BLOCKED_BY_CLIENT

@AdelMahjoub
Copy link

AdelMahjoub commented Jul 15, 2017

Weather forecast proxy helper

Usage

replace https://api.darksky.net

by https://proxy-forecast.glitch.me

The query params and the querystring are the same as the darksky docs

expect the user should not provide his/her dark sky key

Only works for forecast not the time machine

Caching

Responses are cached for one hour, except the ones with errors.

If the user change the query options, the cached data is updated

Important

The Api only accept requests from codepen.

Improvements

Refactoring the code responsible for caching

Links

Glitch Code

Api

Github repo

@MiloATH
Copy link
Contributor

MiloATH commented Jul 15, 2017

@marigerr That is interesting. It is hard for me to test that error because I don't have that ad blocker. Do you have suggested code?

We could default to use HTML5 Geolocation, but the we wouldn't have the city.

@raisedadead
Copy link
Member

raisedadead commented Jul 15, 2017

@marigerr @MiloATH we will not be fixing any pens or code etc, to specifically check for ad-blockers, it's beyond the scope of the assistance that we can provide.

It's up to users to be aware of such caveats

@MiloATH
Copy link
Contributor

MiloATH commented Jul 15, 2017

@raisedadead can this issue be closed or is something still missing? backup/master has been updated.

@raisedadead
Copy link
Member

@MiloATH oh yes, thanks. I missed your comment earlier.

@marigerr
Copy link
Contributor

@MiloATH @raisedadead I was looking at this a little bit closer. The instructions on the weather app challenge recommend for the user to use HTML 5 geolocation to get user's location. But the example codepen app uses ipinfo.io to get the user's Lat Long. I wonder if it would be better to have the example project use the HTML 5 geolocation that is recommended in the instructions. Ipinfo.io, gets a user's ip info without their consent which is a little sketchy imo and maybe something that shouldn't be encouraged at FCC in an example project? While HTML 5 geolocation, will ask for the user's permission. I might try to join in the contributors room and see if there are more opinions on this.

@marigerr
Copy link
Contributor

Here is a altered version of the example pen that uses HTML5 geolocation instead of ipinfo.io
It uses the new api at fcc-weather-api.glitch.me
https://codepen.io/marigerr/full/mwgVoJ/

@raisedadead
Copy link
Member

@marigerr I have updated the pen. Thanks for working on this.

@adeluccar
Copy link

adeluccar commented Jul 28, 2017

Hey everyone, I built a proxy server that works quite well for this purpose. It's for the Dark Sky API. You can self host it on a heroku instance and then use a GET request to:

https://[your-heroku-app].herokuapp.com/?lat=xxx&lon=yyy

Response is the same JSON by Dark Sky (piped).

https://github.com/adeluccar/fcc-show-the-local-weather-proxy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Open for all. You do not need permission to work on these.
Projects
None yet
Development

No branches or pull requests

9 participants