refactor(issue-sync): streamline issue closing logic and enhance error handling
This commit is contained in:
null
parent
1076ca27bb
commit
11d950a13a
|
|
@ -694,7 +694,10 @@ async def close_issue(
|
|||
if auth.user is None:
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED)
|
||||
|
||||
# Get boards linked to this issue's repository for this organization.
|
||||
# Check board links — used for authorization and activity logging.
|
||||
# If the repository is not linked to any board the user can still close
|
||||
# the issue (org membership is sufficient); we just skip the board-scoped
|
||||
# activity event.
|
||||
links = (
|
||||
await session.exec(
|
||||
select(BoardRepositoryLink).where(
|
||||
|
|
@ -703,22 +706,19 @@ async def close_issue(
|
|||
)
|
||||
)
|
||||
).all()
|
||||
if not links:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail="Issue repository is not linked to any board",
|
||||
)
|
||||
|
||||
allowed_board_ids = set(await list_accessible_board_ids(session, member=ctx.member, write=True))
|
||||
authorized_board_id = next(
|
||||
(link.board_id for link in links if link.board_id in allowed_board_ids),
|
||||
None,
|
||||
)
|
||||
if authorized_board_id is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="Board access denied",
|
||||
authorized_board_id: object = None
|
||||
if links:
|
||||
allowed_board_ids = set(await list_accessible_board_ids(session, member=ctx.member, write=True))
|
||||
authorized_board_id = next(
|
||||
(link.board_id for link in links if link.board_id in allowed_board_ids),
|
||||
None,
|
||||
)
|
||||
if authorized_board_id is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="Board access denied",
|
||||
)
|
||||
|
||||
# Close the issue using the service.
|
||||
try:
|
||||
|
|
@ -735,15 +735,16 @@ async def close_issue(
|
|||
raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(e))
|
||||
|
||||
repository_full_name = str(result.get("repository_full_name") or "unknown/unknown")
|
||||
record_activity(
|
||||
session,
|
||||
event_type="forgejo.issue.closed",
|
||||
message=(
|
||||
"Forgejo issue closed by user "
|
||||
f"{auth.user.id}: {repository_full_name}#{result['forgejo_issue_number']}"
|
||||
),
|
||||
board_id=authorized_board_id,
|
||||
)
|
||||
if authorized_board_id is not None:
|
||||
record_activity(
|
||||
session,
|
||||
event_type="forgejo.issue.closed",
|
||||
message=(
|
||||
"Forgejo issue closed by user "
|
||||
f"{auth.user.id}: {repository_full_name}#{result['forgejo_issue_number']}"
|
||||
),
|
||||
board_id=authorized_board_id,
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
return CloseIssueResponse(
|
||||
|
|
|
|||
|
|
@ -85,209 +85,209 @@ class IssueSyncService:
|
|||
limit=limit,
|
||||
)
|
||||
|
||||
# Forgejo returns issues as a JSON array, not wrapped in "items"
|
||||
issues = (
|
||||
response
|
||||
if isinstance(response, list)
|
||||
else response.get("items", response.get("data", []))
|
||||
)
|
||||
if not isinstance(issues, list) or len(issues) == 0:
|
||||
break
|
||||
# Forgejo returns issues as a JSON array, not wrapped in "items"
|
||||
issues = (
|
||||
response
|
||||
if isinstance(response, list)
|
||||
else response.get("items", response.get("data", []))
|
||||
)
|
||||
if not isinstance(issues, list) or len(issues) == 0:
|
||||
break
|
||||
|
||||
for issue_data in issues:
|
||||
# Skip pull requests
|
||||
if issue_data.get("pull_request") is not None:
|
||||
continue
|
||||
for issue_data in issues:
|
||||
# Skip pull requests
|
||||
if issue_data.get("pull_request") is not None:
|
||||
continue
|
||||
|
||||
raw_number = issue_data.get("number", 0)
|
||||
try:
|
||||
forgejo_number = int(raw_number)
|
||||
except (TypeError, ValueError):
|
||||
forgejo_number = 0
|
||||
state = issue_data.get("state", "open")
|
||||
issue_payload = dict(issue_data)
|
||||
comments_payload: list[dict[str, object]] = []
|
||||
timeline_payload: list[dict[str, object]] = []
|
||||
reactions_payload: list[dict[str, object]] = []
|
||||
raw_number = issue_data.get("number", 0)
|
||||
try:
|
||||
forgejo_number = int(raw_number)
|
||||
except (TypeError, ValueError):
|
||||
forgejo_number = 0
|
||||
state = issue_data.get("state", "open")
|
||||
issue_payload = dict(issue_data)
|
||||
comments_payload: list[dict[str, object]] = []
|
||||
timeline_payload: list[dict[str, object]] = []
|
||||
reactions_payload: list[dict[str, object]] = []
|
||||
|
||||
# Enrich each issue with full detail and exhaustive nested data.
|
||||
try:
|
||||
full_issue = await client.get_issue(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
)
|
||||
maybe_full = _as_dict(full_issue)
|
||||
if maybe_full is not None:
|
||||
issue_payload = maybe_full
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_detail_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
comments_payload = _as_dict_list(
|
||||
await client.list_issue_comments(
|
||||
# Enrich each issue with full detail and exhaustive nested data.
|
||||
try:
|
||||
full_issue = await client.get_issue(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
)
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_comments_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
timeline_payload = _as_dict_list(
|
||||
await client.list_issue_timeline(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
maybe_full = _as_dict(full_issue)
|
||||
if maybe_full is not None:
|
||||
issue_payload = maybe_full
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_detail_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_timeline_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
reactions_payload = _as_dict_list(
|
||||
await client.list_issue_reactions(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
try:
|
||||
comments_payload = _as_dict_list(
|
||||
await client.list_issue_comments(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
)
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_comments_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_reactions_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
|
||||
# Parse labels
|
||||
labels_data = []
|
||||
for label in issue_payload.get("labels") or []:
|
||||
labels_data.append(
|
||||
{
|
||||
"id": label.get("id"),
|
||||
"name": label.get("name", ""),
|
||||
"color": label.get("color", ""),
|
||||
"description": label.get("description", ""),
|
||||
try:
|
||||
timeline_payload = _as_dict_list(
|
||||
await client.list_issue_timeline(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
)
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_timeline_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
|
||||
try:
|
||||
reactions_payload = _as_dict_list(
|
||||
await client.list_issue_reactions(
|
||||
owner=repository.owner,
|
||||
repo=repository.repo,
|
||||
issue_number=forgejo_number,
|
||||
)
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"issue_reactions_sync_failed",
|
||||
extra={
|
||||
"repository_id": str(repository_id),
|
||||
"issue_number": forgejo_number,
|
||||
"error": str(exc),
|
||||
},
|
||||
)
|
||||
|
||||
# Parse labels
|
||||
labels_data = []
|
||||
for label in issue_payload.get("labels") or []:
|
||||
labels_data.append(
|
||||
{
|
||||
"id": label.get("id"),
|
||||
"name": label.get("name", ""),
|
||||
"color": label.get("color", ""),
|
||||
"description": label.get("description", ""),
|
||||
}
|
||||
)
|
||||
|
||||
# Parse assignees
|
||||
assignees_data = []
|
||||
for assignee in issue_payload.get("assignees") or []:
|
||||
assignees_data.append(
|
||||
{
|
||||
"login": assignee.get("login", ""),
|
||||
"id": assignee.get("id", 0),
|
||||
"avatar_url": assignee.get("avatar_url", ""),
|
||||
}
|
||||
)
|
||||
|
||||
# Parse milestone
|
||||
milestone_data = None
|
||||
raw_milestone = issue_payload.get("milestone")
|
||||
if raw_milestone and isinstance(raw_milestone, dict):
|
||||
milestone_data = {
|
||||
"id": raw_milestone.get("id"),
|
||||
"title": raw_milestone.get("title", ""),
|
||||
"state": raw_milestone.get("state", "open"),
|
||||
"description": raw_milestone.get("description") or None,
|
||||
"due_on": raw_milestone.get("due_on") or None,
|
||||
"closed_at": raw_milestone.get("closed_at") or None,
|
||||
}
|
||||
)
|
||||
|
||||
# Parse assignees
|
||||
assignees_data = []
|
||||
for assignee in issue_payload.get("assignees") or []:
|
||||
assignees_data.append(
|
||||
{
|
||||
"login": assignee.get("login", ""),
|
||||
"id": assignee.get("id", 0),
|
||||
"avatar_url": assignee.get("avatar_url", ""),
|
||||
}
|
||||
)
|
||||
# Full body and preview
|
||||
raw_body = issue_payload.get("body") or ""
|
||||
body_full = raw_body if raw_body else None
|
||||
body_preview = raw_body[:1000] if raw_body else None
|
||||
|
||||
# Parse milestone
|
||||
milestone_data = None
|
||||
raw_milestone = issue_payload.get("milestone")
|
||||
if raw_milestone and isinstance(raw_milestone, dict):
|
||||
milestone_data = {
|
||||
"id": raw_milestone.get("id"),
|
||||
"title": raw_milestone.get("title", ""),
|
||||
"state": raw_milestone.get("state", "open"),
|
||||
"description": raw_milestone.get("description") or None,
|
||||
"due_on": raw_milestone.get("due_on") or None,
|
||||
"closed_at": raw_milestone.get("closed_at") or None,
|
||||
}
|
||||
# Parse dates — required fields fall back to utcnow(), optional closed_at stays None
|
||||
created_at = self._parse_iso_date(issue_payload.get("created_at")) or utcnow()
|
||||
updated_at = self._parse_iso_date(issue_payload.get("updated_at")) or utcnow()
|
||||
closed_at = self._parse_iso_date(issue_payload.get("closed_at"))
|
||||
|
||||
# Full body and preview
|
||||
raw_body = issue_payload.get("body") or ""
|
||||
body_full = raw_body if raw_body else None
|
||||
body_preview = raw_body[:1000] if raw_body else None
|
||||
# Check if issue exists
|
||||
existing = await self._find_issue(repository_id, forgejo_number)
|
||||
|
||||
# Parse dates — required fields fall back to utcnow(), optional closed_at stays None
|
||||
created_at = self._parse_iso_date(issue_payload.get("created_at")) or utcnow()
|
||||
updated_at = self._parse_iso_date(issue_payload.get("updated_at")) or utcnow()
|
||||
closed_at = self._parse_iso_date(issue_payload.get("closed_at"))
|
||||
if existing is None:
|
||||
issue = ForgejoIssue(
|
||||
organization_id=self.organization_id,
|
||||
repository_id=repository_id,
|
||||
forgejo_issue_number=forgejo_number,
|
||||
title=issue_data.get("title", ""),
|
||||
body=body_full,
|
||||
body_preview=body_preview,
|
||||
state=state,
|
||||
is_pull_request=False,
|
||||
labels=labels_data,
|
||||
assignees=assignees_data,
|
||||
milestone=milestone_data,
|
||||
forgejo_payload=issue_payload,
|
||||
forgejo_comments_payload=comments_payload,
|
||||
forgejo_timeline_payload=timeline_payload,
|
||||
forgejo_reactions_payload=reactions_payload,
|
||||
author=_author_login(issue_payload),
|
||||
html_url=_html_url(issue_payload),
|
||||
forgejo_created_at=created_at,
|
||||
forgejo_updated_at=updated_at,
|
||||
forgejo_closed_at=closed_at,
|
||||
)
|
||||
self.session.add(issue)
|
||||
await self.session.flush()
|
||||
created += 1
|
||||
else:
|
||||
existing.title = issue_data.get("title", "")
|
||||
existing.body = body_full
|
||||
existing.body_preview = body_preview
|
||||
existing.state = state
|
||||
existing.labels = labels_data
|
||||
existing.assignees = assignees_data
|
||||
existing.milestone = milestone_data
|
||||
existing.forgejo_payload = issue_payload
|
||||
existing.forgejo_comments_payload = comments_payload
|
||||
existing.forgejo_timeline_payload = timeline_payload
|
||||
existing.forgejo_reactions_payload = reactions_payload
|
||||
existing.author = _author_login(issue_payload)
|
||||
existing.html_url = _html_url(issue_payload)
|
||||
existing.forgejo_created_at = created_at
|
||||
existing.forgejo_updated_at = updated_at
|
||||
existing.forgejo_closed_at = closed_at
|
||||
existing.last_synced_at = utcnow()
|
||||
await crud.save(self.session, existing)
|
||||
updated_count += 1
|
||||
|
||||
# Check if issue exists
|
||||
existing = await self._find_issue(repository_id, forgejo_number)
|
||||
if state == "open":
|
||||
open_count += 1
|
||||
elif state == "closed":
|
||||
closed_count += 1
|
||||
|
||||
if existing is None:
|
||||
issue = ForgejoIssue(
|
||||
organization_id=self.organization_id,
|
||||
repository_id=repository_id,
|
||||
forgejo_issue_number=forgejo_number,
|
||||
title=issue_data.get("title", ""),
|
||||
body=body_full,
|
||||
body_preview=body_preview,
|
||||
state=state,
|
||||
is_pull_request=False,
|
||||
labels=labels_data,
|
||||
assignees=assignees_data,
|
||||
milestone=milestone_data,
|
||||
forgejo_payload=issue_payload,
|
||||
forgejo_comments_payload=comments_payload,
|
||||
forgejo_timeline_payload=timeline_payload,
|
||||
forgejo_reactions_payload=reactions_payload,
|
||||
author=_author_login(issue_payload),
|
||||
html_url=_html_url(issue_payload),
|
||||
forgejo_created_at=created_at,
|
||||
forgejo_updated_at=updated_at,
|
||||
forgejo_closed_at=closed_at,
|
||||
)
|
||||
self.session.add(issue)
|
||||
await self.session.flush()
|
||||
created += 1
|
||||
else:
|
||||
existing.title = issue_data.get("title", "")
|
||||
existing.body = body_full
|
||||
existing.body_preview = body_preview
|
||||
existing.state = state
|
||||
existing.labels = labels_data
|
||||
existing.assignees = assignees_data
|
||||
existing.milestone = milestone_data
|
||||
existing.forgejo_payload = issue_payload
|
||||
existing.forgejo_comments_payload = comments_payload
|
||||
existing.forgejo_timeline_payload = timeline_payload
|
||||
existing.forgejo_reactions_payload = reactions_payload
|
||||
existing.author = _author_login(issue_payload)
|
||||
existing.html_url = _html_url(issue_payload)
|
||||
existing.forgejo_created_at = created_at
|
||||
existing.forgejo_updated_at = updated_at
|
||||
existing.forgejo_closed_at = closed_at
|
||||
existing.last_synced_at = utcnow()
|
||||
await crud.save(self.session, existing)
|
||||
updated_count += 1
|
||||
|
||||
if state == "open":
|
||||
open_count += 1
|
||||
elif state == "closed":
|
||||
closed_count += 1
|
||||
|
||||
# If we got fewer than limit, we're done
|
||||
if len(issues) < limit:
|
||||
break
|
||||
# If we got fewer than limit, we're done
|
||||
if len(issues) < limit:
|
||||
break
|
||||
current_page += 1
|
||||
|
||||
# Sync repository label catalog
|
||||
|
|
|
|||
Loading…
Reference in New Issue