handling PR updates

[Bala-Sakabattula]

Handling the PR comment updates coming through github.issue_comments topic. Initially it checked in upstream_issue handler that if the its PR and if its not closed than it would ignore the PR events now removed the check so it will sync the comments from PR coming through issue handler and also in checking the github passes filter passing correct item type..

[qodo-for-releng]

PR Summary by Qodo

Sync PR issue_comment events via upstream_issue handler and correct filter gating

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

AI Description

• Process PR comment update events through the issue handler instead of ignoring open PRs.
• Apply GitHub sync/filters using the correct item type (PR vs issue) to gate processing.
• Update tests to cover PR events rejected when pullrequest syncing is disabled.

Diagram

graph TD
  A["GitHub issue_comment event"] --> B["sync2jira/main.py handle_msg"] --> C["upstream_issue.handle_github_message(is_pr)"] --> D{{"passes_github_filters"}}
  D -->|"pass"| E["Issue.from_github"] --> F["sync_with_jira"]
  D -->|"skip"| G["Ignore event"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Make filter API boolean-based (is_pr) instead of string item_type

• ➕ Eliminates fragile string coupling (e.g., "PR" vs "pr")
• ➕ Makes call sites harder to misuse and easier to type-check
• ➖ Requires updating all callers and any downstream logging/formatting
• ➖ Slightly less flexible if more item types are added later

2. Split PR issue_comment handling into a dedicated PR comment handler

• ➕ Clearer separation between issue and PR semantics
• ➕ Allows PR-specific behaviors (e.g., deletions) without branching
• ➖ More code paths to maintain; risk of duplicated logic
• ➖ Current approach already reuses existing issue pipeline effectively

Recommendation: The current approach (route PR issue_comment events through the existing issue handler with an explicit is_pr flag) is a good tradeoff: it reuses the existing transformation/sync pipeline while correctly gating behavior via PR-specific sync configuration. If this area grows further, consider switching passes_github_filters to accept is_pr as a boolean to prevent future item_type mismatches.

Files changed (3) +18 / -14

Bug fix (1) +6 / -4

upstream_issue.pyTreat PR events as first-class items in filter gating and handling +6/-4

Treat PR events as first-class items in filter gating and handling

• Removes the prior logic that ignored PR events while the PR was still open. Ensures filtering/sync gating uses the correct item type (PR vs issue), improving correctness of which repos are allowed to sync PR-derived events.

sync2jira/upstream_issue.py

Tests (1) +9 / -7

test_upstream_issue.pyAdjust PR handling test to validate PR-specific filter rejection +9/-7

Adjust PR handling test to validate PR-specific filter rejection

• Renames and rewrites the PR-path test to verify that PR events are rejected when the repo sync list lacks 'pullrequest'. Calls handle_github_message with is_pr=True and asserts early rejection before any GitHub API usage.

tests/test_upstream_issue.py

Documentation (1) +3 / -3

main.pyClarify PR comment deletion behavior in issue_comment routing +3/-3

Clarify PR comment deletion behavior in issue_comment routing

• Updates the inline rationale for the PR comment deletion action: PR comments are synced, but deletions are intentionally not propagated to Jira. No functional behavior changes beyond documentation/clarity.

sync2jira/main.py

[qodo-for-releng]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Misleading filter-fail log ✓ Resolved 🐞 Bug ◔ Observability

Description

In upstream_issue.handle_github_message, the debug message formats the identifier as "owner/repo#…"
but passes html_url instead of the issue/PR number, making filter failures hard to correlate to a
specific item.

Code

sync2jira/upstream_issue.py[R208-215]

+    item_type = "PR" if is_pr else "issue"
+    if not passes_github_filters(issue, config, upstream, item_type=item_type):
        log.debug(
-            "%r is a pull request.  Ignoring.", issue.get("html_url", "<missing URL>")
+            "%s %s#%s does not pass filters",
+            item_type,
+            upstream,
+            issue.get("html_url", "<missing URL>"),
        )

Relevance

⭐⭐⭐ High

Team has accepted fixes improving log identifiers/formatting for better debugging correlation.

PR-#452
PR-#441

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new log line uses a repo#... format but passes html_url; elsewhere in the same file, #%s
is paired with the numeric issue["number"], showing the intended convention.

sync2jira/upstream_issue.py[203-216]
sync2jira/upstream_issue.py[235-244]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`handle_github_message` logs `"%s %s#%s does not pass filters"` but supplies `issue.get("html_url")` for the `#%s` placeholder, producing logs like `PR org/repo#https://...` instead of `PR org/repo#123`.

### Issue Context
Other logs in the same module use the conventional `owner/repo#<number>` format, so this mismatch is misleading during debugging.

### Fix Focus Areas
- sync2jira/upstream_issue.py[203-216]

Suggested change:
- Replace `issue.get("html_url", "<missing URL>")` with `issue.get("number", "<missing number>")`, or adjust the format string to log the URL without the `#` number convention (optionally log both number and URL).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[webbnh]

The changes look good, but they seem a little light on testing. (And, I flagged one other nit.)

[Bala-Sakabattula]

@webbnh I added test cases for passes_github_filters function and also instead of html url logging issue number

[webbnh]

Looks good. (Although, I did find another nit, but we can ignore it....)

they seem a little light on testing.

That comment was inaccurate: the PR did include reasonable testing for the lines that you actually changed.

But, I think you agree that passes_github_filters() wasn't adequately tested (i.e., because we're now sending PRs through it, when we weren't before), so thank you for the additions!