Repository: stravalib/stravalib (python)

Issue:
Segment Efforts client method uses undocumented API endpoint
There are currently two client methods:

- `get_segment_efforts()`
- `get_segment_effort()`

The first uses an undocumented endpoint `/segments/{segment_id}/all_efforts`. Both should use the documented/supported Strava API endpoint `/segment_efforts`; see https://developers.strava.com/docs/reference/#api-SegmentEfforts.

Pull request description:
fix(endpoint): Update segment_efforts endpoint to match the currently documented endpoint
closes #321

## Description

This pr fixes the segment_efforts api endpoint. I'm struggling a lot with understanding what is broken in the tests and would appreciate any input! This is why I created #603 as I think i generally understand the mock behavior but i clearly don't understand how to use it as these tests don't work as expected. 

## Type of change

Select the statement best describes this pull request.

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] This is a documentation update
- [ ] This is a infrastructure update (docs, ci, etc)
- [ ] Other (please describe)

## Does your PR include tests

If you are fixing a bug or adding a feature, we appreciate (but do not require)
tests to support whatever fix of feature you're implementing.

- [x] Yes
- [ ] No, i'd like some help with tests
- [ ] This change doesn't require tests

## Did you include your contribution to the change log?

- [x] Yes, `changelog.md` is up-to-date.


Patch:
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
new file mode 100644
index 0000000..966b482
--- /dev/null
+++ b/.github/dependabot.yml
@@ -0,0 +1,11 @@
+# To get started with Dependabot version updates, you'll need to specify which
+# package ecosystems to update and where the package manifests are located.
+# Please see the documentation for all configuration options:
+# https://docs.github.com/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file
+
+version: 2
+updates:
+  - package-ecosystem: "pip"
+    directory: "/"
+    schedule:
+      interval: "monthly"
diff --git a/changelog.md b/changelog.md
index 09d05cb..5590d16 100644
--- a/changelog.md
+++ b/changelog.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fix: Update segment_efforts api and return warnings (@lwasser, #321)
+
 ## v2.1.0
 
 ### Added
@@ -31,7 +33,7 @@
 - Change: Strava API update (@github-actions, #592)
 
 ### Contributors to this release
-@yotam5 made their first contribution to stravalib! 
+@yotam5 made their first contribution to stravalib!
 @jsamoocha, @lwasser, @enadeau, @yotam5
 
 ## v2.0.0
diff --git a/src/stravalib/client.py b/src/stravalib/client.py
index d405f9c..c6943d8 100644
--- a/src/stravalib/client.py
+++ b/src/stravalib/client.py
@@ -1250,8 +1250,6 @@ class Client:
     def get_segment_effort(self, effort_id: int) -> model.SegmentEffort:
         """Return a specific segment effort by ID.
 
-        https://developers.strava.com/docs/reference/#api-SegmentEfforts
-
         Parameters
         ----------
         effort_id : int
@@ -1262,6 +1260,10 @@ class Client:
         class:`stravalib.model.SegmentEffort`
             The specified effort on a segment.
 
+        Notes
+        ------
+        https://developers.strava.com/docs/reference/#api-SegmentEfforts
+
         """
         return model.SegmentEffort.model_validate(
             self.protocol.get("/segment_efforts/{id}", id=effort_id)
@@ -1358,6 +1360,7 @@ class Client:
             limit=limit,
         )
 
+    # Next TODO: add tests to deprecated items
     def get_segment_efforts(
         self,
         segment_id: int,
@@ -1369,49 +1372,62 @@ class Client:
         """Gets all efforts on a particular segment sorted by start_date_local
 
         Returns an array of segment effort summary representations sorted by
-        start_date_local ascending or by elapsed_time if an athlete_id is
+        `start_date_local` ascending or by elapsed_time if an athlete_id is
         provided.
 
-        If no filtering parameters is provided all efforts for the segment
+        NOTES:
+        * The `athlete_id` sorting only works if you do NOT provide start and end dates.
+        * If no date filtering parameters are provided, all efforts for the segment
         will be returned.
 
         Date range filtering is accomplished using an inclusive start and end
-        time, thus start_date_local and end_date_local must be sent together.
+        time. `start_date_local` and `end_date_local` must be sent together.
         For open ended ranges pick dates significantly in the past or future.
         The filtering is done over local time for the segment, so there is no
-        need for timezone conversion. For example, all efforts on Jan. 1st, 2014
+        need for timezone conversion. For example, all efforts on Jan. 1st, 2024
         for a segment in San Francisco, CA can be fetched using
-        2014-01-01T00:00:00Z and 2014-01-01T23:59:59Z.
+        `2024-01-01T00:00:00Z` and `2024-01-01T23:59:59Z`.
 
-        https://developers.strava.com/docs/reference/#api-SegmentEfforts-getEffortsBySegmentId
+        This endpoint requires a Strava subscription.
 
         Parameters
         ----------
         segment_id : int
             ID for the segment of interest
-        athlete_id: int, optional
-            ID of athlete.
-        start_date_local : datetime.datetime or str, optional, default=None
+        start_date_local : `datetime.datetime` or `str`, optional, default=None
             Efforts before this date will be excluded.
             Either as ISO8601 or datetime object
-        end_date_local : datetime.datetime or str, optional, default=None
+        end_date_local : `datetime.datetime` or str, optional, default=None
             Efforts after this date will be excluded.
             Either as ISO8601 or datetime object
         limit : int, default=None, optional
             limit number of efforts.
+            This parameter has been removed by Strava and will be removed in
+            stravalib in the near future.
+            .. deprecated::
+                This param is not supported by the Strava API and may be
+                removed in the future.
         athlete_id : int, default=None
-            Strava ID for the athlete
-
+            Strava ID for the athlete. Used to sort efforts by duration.
+            Can only be used if start and end date are not provided.
+            .. deprecated::
+                This param is not supported by the Strava API and may be
+                removed in the future.
         Returns
         -------
         class:`BatchedResultsIterator`
             An iterator of :class:`stravalib.model.SegmentEffort` efforts on
             a segment.
 
+        Notes
+        ------
+        https://developers.strava.com/docs/reference/#api-SegmentEfforts-getEffortsBySegmentId
+
         """
         params: dict[str, Any] = {"segment_id": segment_id}
 
         if athlete_id is not None:
+            warn_param_unsupported("athlete_id")
             params["athlete_id"] = athlete_id
 
         if start_date_local:
@@ -1429,10 +1445,13 @@ class Client:
             )
 
         if limit is not None:
+            warn_param_deprecation("limit", "date ranges")
             params["limit"] = limit
 
         result_fetcher = functools.partial(
-            self.protocol.get, "/segments/{segment_id}/all_efforts", **params
+            self.protocol.get,
+            "/segment_efforts",
+            **params,
         )
 
         return BatchedResultsIterator(


Test patch:
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
index a1e3a11..17251d4 100644
--- a/.github/workflows/build-test.yml
+++ b/.github/workflows/build-test.yml
@@ -58,7 +58,7 @@ jobs:
           nox -s tests-${{ matrix.python-version }}
       - name: Upload coverage to Codecov
         if: ${{ matrix.os == 'ubuntu-latest' &&  matrix.python-version == '3.11'}}
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v5
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           verbose: true
diff --git a/src/stravalib/tests/conftest.py b/src/stravalib/tests/conftest.py
index 9734159..063907b 100644
--- a/src/stravalib/tests/conftest.py
+++ b/src/stravalib/tests/conftest.py
@@ -1,8 +1,12 @@
+import warnings
+
 import pytest
 
 from stravalib import Client
 from stravalib.tests.integration.strava_api_stub import StravaAPIMock
 
+warnings.simplefilter("always")
+
 
 @pytest.fixture
 def mock_strava_api():
diff --git a/src/stravalib/tests/integration/test_client.py b/src/stravalib/tests/integration/test_client.py
index 2babef8..e716f11 100644
--- a/src/stravalib/tests/integration/test_client.py
+++ b/src/stravalib/tests/integration/test_client.py
@@ -1,6 +1,7 @@
 import datetime
 import json
 import os
+import warnings
 from unittest import mock
 
 import pytest
@@ -8,12 +9,17 @@ import responses
 from responses import matchers
 
 from stravalib.client import ActivityUploader
-from stravalib.exc import AccessUnauthorized, ActivityPhotoUploadFailed
+from stravalib.exc import (
+    AccessUnauthorized,
+    ActivityPhotoUploadFailed,
+)
 from stravalib.model import DetailedAthlete, SummaryAthlete, SummarySegment
 from stravalib.strava_model import SummaryActivity, Zones
 from stravalib.tests import RESOURCES_DIR
 from stravalib.unit_helper import miles
 
+warnings.simplefilter("always")
+
 
 @pytest.fixture
 def zone_response():
@@ -885,6 +891,48 @@ def test_get_segment_effort(mock_strava_api, client):
     assert effort.max_heartrate == 170
 
 
+@pytest.mark.parametrize(
+    "params, warning_type, warning_message",
+    [
+        (
+            {"segment_id": 2345, "athlete_id": 12345},
+            DeprecationWarning,
+            'The "athlete_id" parameter is unsupported',
+        ),
+        (
+            {"segment_id": 2345, "limit": 10},
+            DeprecationWarning,
+            'The "limit" parameter is deprecated',
+        ),
+    ],
+)
+def test_get_segment_efforts_warnings(
+    params, warning_type, warning_message, mock_strava_api, client
+):
+    """Test that if provided with deprecated params, the user receives a
+    warning."""
+
+    mock_strava_api.get(
+        "/segment_efforts",
+        response_update={"name": "Best Run Ever"},
+        n_results=1,
+    )
+
+    with pytest.warns(warning_type, match=warning_message):
+        a = client.get_segment_efforts(**params)
+        next(a)
+
+
+def test_get_segment_efforts(client, mock_strava_api):
+    """Test that endpoint returns data as expected."""
+    mock_strava_api.get("/segment_efforts", n_results=4)
+
+    efforts = list(client.get_segment_efforts(segment_id=2345, athlete_id=12345))
+
+    assert len(efforts) == 4
+    assert efforts[0].name == "Alpe d'Huez"
+
+
 def test_get_activities_paged(mock_strava_api, client):
     for i in range(1, 4):
         params = {"page": i, "per_page": 200}