Repository: JanDeDobbeleer/oh-my-posh
Pull request #4973 (2024-05-29 06:20:26)

owm: update api call with location provided
### Prerequisites

- [x] I have read and understood the [contributing guide][CONTRIBUTING.md].
- [x] The commit message follows the [conventional commits][cc] guidelines.
- [x] Tests for the changes have been added (for bug fixes / features).
- [x] Docs have been added/updated (for bug fixes / features).

### Description

openweathermap API support use location directly for weather request, thus, use this api to reduce calls from 2 to 1

[CONTRIBUTING.md]: https://github.com/JanDeDobbeleer/oh-my-posh/blob/main/CONTRIBUTING.md
[cc]: https://www.conventionalcommits.org/en/v1.0.0/#summary



Patch:
diff --git a/src/segments/owm.go b/src/segments/owm.go
index bb2b36ac..e614ee03 100644
--- a/src/segments/owm.go
+++ b/src/segments/owm.go
@@ -28,10 +28,6 @@ const (
 	Location properties.Property = "location"
 	// Units openweathermap units
 	Units properties.Property = "units"
-	// Latitude for the location used in place of location
-	Latitude properties.Property = "latitude"
-	// Longitude for the location used in place of location
-	Longitude properties.Property = "longitude"
 	// CacheKeyResponse key used when caching the response
 	CacheKeyResponse string = "owm_response"
 	// CacheKeyURL key used when caching the url responsible for the response
@@ -54,11 +50,6 @@ type owmDataResponse struct {
 	temperature `json:"main"`
 }
 
-type geoLocation struct {
-	Lat float64 `json:"lat"`
-	Lon float64 `json:"lon"`
-}
-
 func (d *Owm) Enabled() bool {
 	err := d.setStatus()
 
@@ -77,65 +68,35 @@ func (d *Owm) Template() string {
 func (d *Owm) getResult() (*owmDataResponse, error) {
 	cacheTimeout := d.props.GetInt(properties.CacheTimeout, properties.DefaultCacheTimeout)
 	response := new(owmDataResponse)
+
 	if cacheTimeout > 0 {
-		// check if data stored in cache
 		val, found := d.env.Cache().Get(CacheKeyResponse)
-		// we got something from te cache
 		if found {
 			err := json.Unmarshal([]byte(val), response)
 			if err != nil {
 				return nil, err
 			}
+
 			d.URL, _ = d.env.Cache().Get(CacheKeyURL)
 			return response, nil
 		}
 	}
 
-	apikey := properties.OneOf[string](d.props, ".", APIKey, "apiKey")
+	apikey := properties.OneOf(d.props, ".", APIKey, "apiKey")
 	if len(apikey) == 0 {
 		apikey = d.env.Getenv(PoshOWMAPIKey)
 	}
 
-	if len(apikey) == 0 {
-		return nil, errors.New("no api key found")
-	}
-
 	location := d.props.GetString(Location, "De Bilt,NL")
-	latitude := d.props.GetFloat64(Latitude, 91)    // This default value is intentionally invalid since there should not be a default for this and 0 is a valid value
-	longitude := d.props.GetFloat64(Longitude, 181) // This default value is intentionally invalid since there should not be a default for this and 0 is a valid value
-	units := d.props.GetString(Units, "standard")
-	httpTimeout := d.props.GetInt(properties.HTTPTimeout, properties.DefaultHTTPTimeout)
 
-	validCoordinates := func(latitude, longitude float64) bool {
-		// Latitude values are only valid if they are between -90 and 90
-		// Longitude values are only valid if they are between -180 and 180
-		// https://gisgeography.com/latitude-longitude-coordinates/
-		return latitude <= 90 && latitude >= -90 && longitude <= 180 && longitude >= -180
+	if len(apikey) == 0 || len(location) == 0 {
+		return nil, errors.New("no api key or location found")
 	}
 
-	if !validCoordinates(latitude, longitude) {
-		var geoResponse []geoLocation
-		geocodingURL := fmt.Sprintf("http://api.openweathermap.org/geo/1.0/direct?q=%s&limit=1&appid=%s", location, apikey)
-
-		body, err := d.env.HTTPRequest(geocodingURL, nil, httpTimeout)
-		if err != nil {
-			return new(owmDataResponse), err
-		}
-
-		err = json.Unmarshal(body, &geoResponse)
-		if err != nil {
-			return new(owmDataResponse), err
-		}
-
-		if len(geoResponse) == 0 {
-			return new(owmDataResponse), fmt.Errorf("no coordinates found for %s", location)
-		}
-
-		latitude = geoResponse[0].Lat
-		longitude = geoResponse[0].Lon
-	}
+	units := d.props.GetString(Units, "standard")
+	httpTimeout := d.props.GetInt(properties.HTTPTimeout, properties.DefaultHTTPTimeout)
 
-	d.URL = fmt.Sprintf("http://api.openweathermap.org/data/2.5/weather?lat=%v&lon=%v&units=%s&appid=%s", latitude, longitude, units, apikey)
+	d.URL = fmt.Sprintf("http://api.openweathermap.org/data/2.5/weather?q=%s&units=%s&appid=%s", location, units, apikey)
 
 	body, err := d.env.HTTPRequest(d.URL, nil, httpTimeout)
 	if err != nil {
diff --git a/themes/schema.json b/themes/schema.json
index 833902be..72342d89 100644
--- a/themes/schema.json
+++ b/themes/schema.json
@@ -2952,18 +2952,6 @@
                     "description": "Location to use for the API call interpreted only if valid coordinates aren't given. Formatted as <City>,<STATE>,<COUNTRY_CODE>. City name, state code and country code divided by comma. Please, refer to ISO 3166 for the state codes or country codes.",
                     "default": "De Bilt,NL"
                   },
-                  "latitude": {
-                    "type": "number",
-                    "title": "Latitude",
-                    "description": "The latitude of the requested location, valid between -90 and 90",
-                    "default": 91
-                  },
-                  "longitude": {
-                    "type": "number",
-                    "title": "Longitude",
-                    "description": "The longitude of the requested location, valid between -180 and 180",
-                    "default": 181
-                  },
                   "units": {
                     "type": "string",
                     "title": "units",
diff --git a/website/docs/segments/owm.mdx b/website/docs/segments/owm.mdx
index f755405f..be9ea8e9 100644
--- a/website/docs/segments/owm.mdx
+++ b/website/docs/segments/owm.mdx
@@ -37,19 +37,13 @@ import Config from "@site/src/components/Config.js";
 
 ## Properties
 
-| Name            | Type      | Default      | Description                                                                                                                                                                                                                                   |
-| --------------- | :-------: | :----------: | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `api_key`       | `string`  | `.`          | Your API key from [Open Weather Map][owm]. Can also be set using the `POSH_OWM_API_KEY` environment variable.                                                                                                                                 |
-| `latitude`      | `float64` | `91`         | The latitude of the requested location, valid between -90 and 90                                                                                                                                                                              |
-| `longitude`     | `float64` | `181`        | The longitude of the requested location, valid between -180 and 180                                                                                                                                                                           |
-| `location`      | `string`  | `De Bilt,NL` | The requested location interpreted only if valid coordinates aren't given. Formatted as \<City,STATE,COUNTRY_CODE\>. City name, state code and country code divided by comma. Please, refer to ISO 3166 for the state codes or country codes |
-| `units`         | `string`  | `standard`   | Units of measurement. Available values are standard (kelvin), metric (celsius), and imperial (fahrenheit)                                                                                                                                     |
-| `http_timeout`  | `int`     | `20`         | in milliseconds, the timeout for http request                                                                                                                                                                                                 |
-| `cache_timeout` | `int`     | `10`         | in minutes, the timeout for request caching. A value of 0 disables the cache.                                                                                                                                                                 |
-
-### Specifying Location
-
-The given location can either be specified through the Latitude and Longitude properties or through the Location property. If both Latitude and Longitude are specified and valid then Location will be ignored if it is included. If Latitude or Longitude are not specified or are invalid then Location will be used.
+| Name            |   Type   |   Default    | Description                                                                                                                                                                                                                                  |
+| --------------- | :------: | :----------: | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `api_key`       | `string` |     `.`      | Your API key from [Open Weather Map][owm]. Can also be set using the `POSH_OWM_API_KEY` environment variable.                                                                                                                                |
+| `location`      | `string` | `De Bilt,NL` | The requested location interpreted only if valid coordinates aren't given. Formatted as \<City,STATE,COUNTRY_CODE\>. City name, state code and country code divided by comma. Please, refer to ISO 3166 for the state codes or country codes |
+| `units`         | `string` |  `standard`  | Units of measurement. Available values are standard (kelvin), metric (celsius), and imperial (fahrenheit)                                                                                                                                    |
+| `http_timeout`  |  `int`   |     `20`     | in milliseconds, the timeout for http request                                                                                                                                                                                                |
+| `cache_timeout` |  `int`   |     `10`     | in minutes, the timeout for request caching. A value of 0 disables the cache.                                                                                                                                                                |
 
 ## Template ([info][templates])
 


Test patch:
diff --git a/src/segments/owm_test.go b/src/segments/owm_test.go
index 6bc85888..01a96051 100644
--- a/src/segments/owm_test.go
+++ b/src/segments/owm_test.go
@@ -13,17 +13,13 @@ import (
 )
 
 const (
-	OWMGEOAPIURL     = "http://api.openweathermap.org/geo/1.0/direct?q=AMSTERDAM,NL&limit=1&appid=key"
-	OWMWEATHERAPIURL = "http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key"
+	OWMWEATHERAPIURL = "http://api.openweathermap.org/data/2.5/weather?q=%s&units=metric&appid=key"
 )
 
 func TestOWMSegmentSingle(t *testing.T) {
 	cases := []struct {
 		Case                string
 		Location            string
-		Latitude            float64
-		Longitude           float64
-		GeoJSONResponse     string
 		WeatherJSONResponse string
 		ExpectedString      string
 		ExpectedEnabled     bool
@@ -33,7 +29,6 @@ func TestOWMSegmentSingle(t *testing.T) {
 		{
 			Case:                "Sunny Display",
 			Location:            "AMSTERDAM,NL",
-			GeoJSONResponse:     `[{"lat": 52.3727598,"lon": 4.8936041}]`,
 			WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`,
 			ExpectedString:      "\ue30d (20°C)",
 			ExpectedEnabled:     true,
@@ -41,7 +36,6 @@ func TestOWMSegmentSingle(t *testing.T) {
 		{
 			Case:                "Sunny Display",
 			Location:            "AMSTERDAM,NL",
-			GeoJSONResponse:     `[{"lat": 52.3727598,"lon": 4.8936041}]`,
 			WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`,
 			ExpectedString:      "\ue30d (20°C)",
 			ExpectedEnabled:     true,
@@ -50,7 +44,6 @@ func TestOWMSegmentSingle(t *testing.T) {
 		{
 			Case:                "Sunny Display",
 			Location:            "AMSTERDAM,NL",
-			GeoJSONResponse:     `[{"lat": 52.3727598,"lon": 4.8936041}]`,
 			WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`,
 			ExpectedString:      "\ue30d",
 			ExpectedEnabled:     true,
@@ -59,24 +52,18 @@ func TestOWMSegmentSingle(t *testing.T) {
 		{
 			Case:                "Config Skip Geocoding Check With Location",
 			Location:            "AMSTERDAM,NL",
-			Latitude:            52.3727598,
-			Longitude:           4.8936041,
 			WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`,
 			ExpectedString:      "\ue30d (20°C)",
 			ExpectedEnabled:     true,
 		},
 		{
 			Case:                "Config Skip Geocoding Check Without Location",
-			Latitude:            52.3727598,
-			Longitude:           4.8936041,
 			WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`,
-			ExpectedString:      "\ue30d (20°C)",
-			ExpectedEnabled:     true,
+			ExpectedEnabled:     false,
 		},
 		{
 			Case:                "Error in retrieving data",
 			Location:            "AMSTERDAM,NL",
-			GeoJSONResponse:     "nonsense",
 			WeatherJSONResponse: "nonsense",
 			Error:               errors.New("Something went wrong"),
 			ExpectedEnabled:     false,
@@ -85,35 +72,15 @@ func TestOWMSegmentSingle(t *testing.T) {
 
 	for _, tc := range cases {
 		env := &mock.MockedEnvironment{}
-		var props properties.Map
-		if tc.Latitude != 0 && tc.Longitude != 0 && tc.Location != "" { //nolint: gocritic
-			props = properties.Map{
-				APIKey:                  "key",
-				Location:                tc.Location,
-				Latitude:                tc.Latitude,
-				Longitude:               tc.Longitude,
-				Units:                   "metric",
-				properties.CacheTimeout: 0,
-			}
-		} else if tc.Location != "" {
-			props = properties.Map{
-				APIKey:                  "key",
-				Location:                tc.Location,
-				Units:                   "metric",
-				properties.CacheTimeout: 0,
-			}
-		} else if tc.Latitude != 0 && tc.Longitude != 0 {
-			props = properties.Map{
-				APIKey:                  "key",
-				Latitude:                tc.Latitude,
-				Longitude:               tc.Longitude,
-				Units:                   "metric",
-				properties.CacheTimeout: 0,
-			}
+		props := properties.Map{
+			APIKey:                  "key",
+			Location:                tc.Location,
+			Units:                   "metric",
+			properties.CacheTimeout: 0,
 		}
 
-		env.On("HTTPRequest", OWMGEOAPIURL).Return([]byte(tc.GeoJSONResponse), tc.Error)
-		env.On("HTTPRequest", OWMWEATHERAPIURL).Return([]byte(tc.WeatherJSONResponse), tc.Error)
+		url := fmt.Sprintf(OWMWEATHERAPIURL, tc.Location)
+		env.On("HTTPRequest", url).Return([]byte(tc.WeatherJSONResponse), tc.Error)
 		env.On("Error", mock2.Anything)
 
 		o := &Owm{
@@ -232,7 +199,8 @@ func TestOWMSegmentIcons(t *testing.T) {
 			ExpectedIconString: "\ue313",
 		},
 	}
-	geoResponse := `[{"lat": 52.3727598,"lon": 4.8936041}]`
+
+	url := fmt.Sprintf(OWMWEATHERAPIURL, "AMSTERDAM,NL")
 
 	for _, tc := range cases {
 		env := &mock.MockedEnvironment{}
@@ -240,8 +208,7 @@ func TestOWMSegmentIcons(t *testing.T) {
 		weatherResponse := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20.3}}`, tc.IconID)
 		expectedString := fmt.Sprintf("%s (20°C)", tc.ExpectedIconString)
 
-		env.On("HTTPRequest", OWMGEOAPIURL).Return([]byte(geoResponse), nil)
-		env.On("HTTPRequest", OWMWEATHERAPIURL).Return([]byte(weatherResponse), nil)
+		env.On("HTTPRequest", url).Return([]byte(weatherResponse), nil)
 
 		o := &Owm{
 			props: properties.Map{
@@ -262,10 +229,9 @@ func TestOWMSegmentIcons(t *testing.T) {
 		env := &mock.MockedEnvironment{}
 
 		weatherResponse := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20.3}}`, tc.IconID)
-		expectedString := fmt.Sprintf("«%s (20°C)»(http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key)", tc.ExpectedIconString)
+		expectedString := fmt.Sprintf("«%s (20°C)»(%s)", tc.ExpectedIconString, url)
 
-		env.On("HTTPRequest", OWMGEOAPIURL).Return([]byte(geoResponse), nil)
-		env.On("HTTPRequest", OWMWEATHERAPIURL).Return([]byte(weatherResponse), nil)
+		env.On("HTTPRequest", url).Return([]byte(weatherResponse), nil)
 
 		o := &Owm{
 			props: properties.Map{
@@ -281,7 +247,8 @@ func TestOWMSegmentIcons(t *testing.T) {
 		assert.Equal(t, expectedString, renderTemplate(env, "«{{.Weather}} ({{.Temperature}}{{.UnitIcon}})»({{.URL}})", o), tc.Case)
 	}
 }
-func TestOWMSegmentFromCache(t *testing.T) {
+
+func TestOWMSegmentFromCacheByGeoName(t *testing.T) {
 	response := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20}}`, "01d")
 	expectedString := fmt.Sprintf("%s (20°C)", "\ue30d")
 
@@ -296,7 +263,7 @@ func TestOWMSegmentFromCache(t *testing.T) {
 		env: env,
 	}
 	cache.On("Get", "owm_response").Return(response, true)
-	cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key", true)
+	cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?q=AMSTERDAM,NL&units=metric&appid=key", true)
 	cache.On("Set").Return()
 	env.On("Cache").Return(cache)
 
@@ -304,9 +271,9 @@ func TestOWMSegmentFromCache(t *testing.T) {
 	assert.Equal(t, expectedString, renderTemplate(env, o.Template(), o), "should return the cached response")
 }
 
-func TestOWMSegmentFromCacheWithHyperlink(t *testing.T) {
+func TestOWMSegmentFromCacheWithHyperlinkByGeoName(t *testing.T) {
 	response := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20}}`, "01d")
-	expectedString := fmt.Sprintf("«%s (20°C)»(http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key)", "\ue30d")
+	expectedString := fmt.Sprintf("«%s (20°C)»(http://api.openweathermap.org/data/2.5/weather?q=AMSTERDAM,NL&units=metric&appid=key)", "\ue30d")
 
 	env := &mock.MockedEnvironment{}
 	cache := &mock.MockedCache{}
@@ -320,7 +287,7 @@ func TestOWMSegmentFromCacheWithHyperlink(t *testing.T) {
 		env: env,
 	}
 	cache.On("Get", "owm_response").Return(response, true)
-	cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key", true)
+	cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?q=AMSTERDAM,NL&units=metric&appid=key", true)
 	cache.On("Set").Return()
 	env.On("Cache").Return(cache)