Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions backend/analytics_server/mhq/api/pull_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,38 @@ def get_team_lead_time_trends(
week.isoformat(): adapt_lead_time_metrics(average_lead_time_metrics)
for week, average_lead_time_metrics in weekly_lead_time_metrics_avg_map.items()
}

@app.route("/teams/<team_id>/prs/merged_without_review", methods={"GET"})
@queryschema(
Schema({
Required("from_time"): All(str, Coerce(datetime.fromisoformat)),
Required("to_time"): All(str, Coerce(datetime.fromisoformat)),
Optional("pr_filter"): All(str, Coerce(json.loads)),
})
)
def get_prs_merged_without_review(
team_id: str,
from_time: datetime,
to_time: datetime,
pr_filter: Dict = None,
) -> List[PullRequest]:
Comment on lines +179 to +184
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix return type annotation and parameter type hint.

The function has two type annotation issues:

  1. The return type is annotated as List[PullRequest], but the function actually returns a dict (the result of get_non_paginated_pr_response).
  2. The pr_filter parameter should have an explicit Optional type hint or use union syntax.

Apply this diff to fix the type annotations:

 def get_prs_merged_without_review(
     team_id: str,
     from_time: datetime,
     to_time: datetime,
-    pr_filter: Dict = None,
-) -> List[PullRequest]:
+    pr_filter: Dict | None = None,
+):

Note: The return type can be inferred from get_non_paginated_pr_response, or you can explicitly type it as dict if preferred.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def get_prs_merged_without_review(
team_id: str,
from_time: datetime,
to_time: datetime,
pr_filter: Dict = None,
) -> List[PullRequest]:
def get_prs_merged_without_review(
team_id: str,
from_time: datetime,
to_time: datetime,
pr_filter: Dict | None = None,
):
🧰 Tools
🪛 Ruff (0.14.2)

183-183: PEP 484 prohibits implicit Optional

Convert to T | None

(RUF013)

🤖 Prompt for AI Agents
In backend/analytics_server/mhq/api/pull_requests.py around lines 179 to 184,
the function signature incorrectly annotates the return type as
List[PullRequest] and leaves pr_filter without an explicit Optional type; update
the signature so the return type matches the actual return value (e.g., dict or
the specific type returned by get_non_paginated_pr_response) and change
pr_filter: Dict = None to pr_filter: Optional[Dict] = None (or pr_filter: Dict |
None = None), and add the necessary typing import (Optional) if not already
present.

query_validator = get_query_validator()

pr_analytics = get_pr_analytics_service()
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove trailing whitespace.

Line 187 has trailing whitespace after the function call.

Apply this diff:

-    pr_analytics = get_pr_analytics_service() 
+    pr_analytics = get_pr_analytics_service()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pr_analytics = get_pr_analytics_service()
pr_analytics = get_pr_analytics_service()
🤖 Prompt for AI Agents
In backend/analytics_server/mhq/api/pull_requests.py around line 187, the line
"pr_analytics = get_pr_analytics_service() " contains trailing whitespace;
remove the extra space at the end of the line so it reads "pr_analytics =
get_pr_analytics_service()" and run a quick linter/formatter to ensure no other
trailing whitespace remains.


interval: Interval = query_validator.interval_validator(from_time, to_time)

pr_filter: PRFilter = apply_pr_filter(
pr_filter, EntityType.TEAM, team_id, [SettingType.EXCLUDED_PRS_SETTING]
)

team_repos = pr_analytics.get_team_repos(team_id)
if not team_repos:
return []
Comment on lines +195 to +197
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Good defensive programming, but clean up trailing whitespace.

The early return when no team repos exist is good practice. However, line 197 has trailing whitespace.

Apply this diff to remove the trailing whitespace:

-        return [] 
+        return []
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
team_repos = pr_analytics.get_team_repos(team_id)
if not team_repos:
return []
team_repos = pr_analytics.get_team_repos(team_id)
if not team_repos:
return []
🤖 Prompt for AI Agents
In backend/analytics_server/mhq/api/pull_requests.py around lines 195 to 197,
the early return is correct but line 197 contains trailing whitespace; remove
the trailing space after the return [] so the line ends exactly with return []
and save the file (run the formatter/linter if you have one to ensure no other
trailing whitespace remains).


repo_ids = [repo.id for repo in team_repos]

prs = pr_analytics.get_prs_merged_without_review(repo_ids, interval, pr_filter)

repo_id_repo_map = {repo.id: repo for repo in team_repos}
return get_non_paginated_pr_response(prs, repo_id_repo_map, len(prs))
6 changes: 5 additions & 1 deletion backend/analytics_server/mhq/service/code/pr_analytics.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from mhq.store.models.code.filter import PRFilter
from mhq.store.models.core.teams import Team
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused import.

The Team import is not used anywhere in this file.

Apply this diff to remove the unused import:

-from mhq.store.models.core.teams import Team
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from mhq.store.models.core.teams import Team
🧰 Tools
🪛 Flake8 (7.3.0)

[error] 2-2: 'mhq.store.models.core.teams.Team' imported but unused

(F401)

🤖 Prompt for AI Agents
In backend/analytics_server/mhq/service/code/pr_analytics.py around line 2, the
imported symbol Team from mhq.store.models.core.teams is unused; remove the line
"from mhq.store.models.core.teams import Team" to eliminate the unused import
and keep the file imports minimal, then run the linter/tests to confirm no other
references break.

from mhq.utils.time import Interval
from mhq.store.models.code import OrgRepo, PullRequest
from mhq.store.repos.code import CodeRepoService

from typing import List, Optional


Expand All @@ -17,6 +19,8 @@ def get_team_repos(self, team_id: str) -> List[OrgRepo]:
def get_repo_by_id(self, repo_id: str) -> Optional[OrgRepo]:
return self.code_repo_service.get_repo_by_id(repo_id)

def get_prs_merged_without_review(self, repo_ids: List[str], interval: Interval, pr_filter: PRFilter) -> List[PullRequest]:
return self.code_repo_service.get_prs_merged_without_review(repo_ids, interval, pr_filter)

def get_pr_analytics_service():
return PullRequestAnalyticsService(CodeRepoService())
33 changes: 31 additions & 2 deletions backend/analytics_server/mhq/store/repos/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from operator import and_
from typing import Optional, List

from mhq.store.models.code.enums import CodeProvider
from sqlalchemy import or_
from mhq.store.models.code.enums import CodeProvider, PullRequestEventType
from sqlalchemy import or_, not_, exists
from sqlalchemy.orm import defer
from mhq.store.models.core import Team

Expand Down Expand Up @@ -283,6 +283,35 @@ def get_active_org_repos_by_ids(self, repo_ids: List[str]) -> List[OrgRepo]:
.all()
)

@rollback_on_exc
def get_prs_merged_without_review(
self,
repo_ids: List[str],
interval: Interval,
pr_filter: PRFilter = None,
) -> List[PullRequest]:
query = self._db.session.query(PullRequest).options(defer(PullRequest.data))

query = self._filter_prs_by_repo_ids(query, repo_ids)
query = self._filter_prs_merged_in_interval(query, interval)

query = self._filter_prs(query, pr_filter)

query = query.filter(
not_(
exists().where(
and_(
PullRequestEvent.pull_request_id == PullRequest.id,
PullRequestEvent.type == PullRequestEventType.REVIEW,
)
)
)
)

query = query.order_by(PullRequest.state_changed_at.asc())

return query.all()
Comment on lines +286 to +313
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Query logic is correct, but clean up trailing whitespace.

The implementation correctly identifies PRs merged without review using a NOT EXISTS subquery to exclude PRs with any REVIEW events. The approach is sound and follows established patterns in the codebase.

However, there are trailing spaces on lines 288 and 308 that should be removed.

Apply this diff to remove trailing whitespace:

     def get_prs_merged_without_review(
-        self, 
+        self,
         repo_ids: List[str],
         interval: Interval,
         pr_filter: PRFilter = None,
     ) -> List[PullRequest]:
         query = self._db.session.query(PullRequest).options(defer(PullRequest.data))
 
         query = self._filter_prs_by_repo_ids(query, repo_ids)
         query = self._filter_prs_merged_in_interval(query, interval)
 
         query = self._filter_prs(query, pr_filter)
         
         query = query.filter(
             not_(
                 exists().where(
                     and_(
                         PullRequestEvent.pull_request_id == PullRequest.id,
                         PullRequestEvent.type == PullRequestEventType.REVIEW,
                     )
                 )
-            )   
+            )
         )
 
         query = query.order_by(PullRequest.state_changed_at.asc())
 
         return query.all()
🤖 Prompt for AI Agents
In backend/analytics_server/mhq/store/repos/code.py around lines 286 to 313,
there are trailing whitespace characters on lines 288 and 308; remove those
trailing spaces so the lines end cleanly (no extra spaces) and save the file to
eliminate the whitespace-only diffs.


@rollback_on_exc
def get_prs_merged_in_interval(
self,
Expand Down