Prevent Jira auto-linking in NEWA IDs with backwards compatibility#333
Merged
Conversation
Reviewer's GuideThis PR changes how NEWA IDs and Jira field values are handled to prevent Jira Cloud auto-linking and Jinja2 template conflicts, while maintaining backwards compatibility with existing issues by dual-format searching and robust NEWA ID matching. Sequence diagram for dual-format NEWA ID search in get_related_issuessequenceDiagram
participant Caller
participant JiraService
participant JiraConnection as JiraConnection_or_ServerConnection
Caller->>JiraService: get_related_issues(action, closed, all_respins)
activate JiraService
JiraService->>JiraService: newa_id(action, all_respins)
JiraService-->>JiraService: base_newa_id (regular hyphens)
JiraService->>JiraService: _format_for_jira(base_newa_id)
JiraService-->>JiraService: formatted_newa_id (nonbreaking hyphens)
JiraService->>JiraService: build JQL query 1 using formatted_newa_id
JiraService->>JiraService: build JQL query 2 using base_newa_id
loop for each query in [query1, query2]
alt jira_connection.is_cloud
JiraService->>JiraConnection: search_issues_v3(jql, fields, max_results)
JiraConnection-->>JiraService: search_result (dict)
else Jira Server
JiraService->>JiraConnection: search_issues(jql, fields, json_result=True)
JiraConnection-->>JiraService: search_result (dict)
end
JiraService->>JiraService: validate type(search_result) is dict
JiraService->>JiraService: merge issues into all_issues by key
JiraService->>JiraService: short_sleep()
end
JiraService->>JiraService: result = {}
loop for each jira_issue in all_issues.values
JiraService->>JiraService: description = jira_issue.fields.description or ""
JiraService->>JiraService: _newa_id_in_description(base_newa_id, description)
alt match found
JiraService-->>JiraService: matched NEWA ID string
JiraService->>JiraService: add entry to result[jira_issue.key]
else no match
JiraService-->>JiraService: None
end
end
JiraService-->>Caller: result
deactivate JiraService
Class diagram for updated JiraService NEWA ID handlingclassDiagram
class JiraService {
- jira_connection
- connection
- project
- newa_label
- transitions
- logger
+ newa_id(action: IssueAction, partial: bool) str
+ get_related_issues(action: IssueAction, closed: bool, all_respins: bool) dict
+ refresh_issue(action: IssueAction, issue: Issue) bool
+ update_issue(action: IssueAction, issue: Issue) bool
- _format_for_jira(text: str) str
- _newa_id_in_description(newa_id: str, description: str) str
}
class JiraConnection {
+ is_cloud bool
+ search_issues_v3(jql: str, fields: list, max_results: int) dict
+ search_issues(jql: str, fields: list, json_result: bool) dict
}
class IssueAction
class Issue
JiraService --> JiraConnection : uses
JiraService --> IssueAction : uses
JiraService --> Issue : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- Consider avoiding direct use of the private helper
jira_handler._newa_id_in_descriptionfromjira_helpers.pyand either make this a public API on the handler or move the logic into a shared utility to reduce tight coupling on an internal method. - The
_sanitize_jira_field_valuefunction currently replaces{{/}}with double quotes, which can unintentionally introduce or unbalance quotes in user-visible text; it might be safer to replace them with a more neutral escape sequence (e.g. HTML entities or a non-conflicting token) instead. - In
get_related_issues, raising a bareExceptionfor an unexpected search result type makes error handling less precise; using a more specific exception type or defining a custom one would make failures easier to diagnose and handle.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider avoiding direct use of the private helper `jira_handler._newa_id_in_description` from `jira_helpers.py` and either make this a public API on the handler or move the logic into a shared utility to reduce tight coupling on an internal method.
- The `_sanitize_jira_field_value` function currently replaces `{{`/`}}` with double quotes, which can unintentionally introduce or unbalance quotes in user-visible text; it might be safer to replace them with a more neutral escape sequence (e.g. HTML entities or a non-conflicting token) instead.
- In `get_related_issues`, raising a bare `Exception` for an unexpected search result type makes error handling less precise; using a more specific exception type or defining a custom one would make failures easier to diagnose and handle.
## Individual Comments
### Comment 1
<location path="newa/services/jira_service.py" line_range="213-214" />
<code_context>
+ # Build queries for both formats
+ queries = []
+ base_filter = f"project = '{self.project}' AND labels in ({self.newa_label})"
+ status_filter = "" if closed else f" AND status not in ({
+ ','.join(
+ self.transitions.closed)})"
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The multiline f-string used for `status_filter` will inject newlines/indentation into the JQL.
This will likely produce invalid JQL and can cause search failures, depending on how Jira handles the embedded whitespace. Please build the filter on a single line or avoid embedded newlines, for example:
```python
status_filter = "" if closed else f" AND status not in ({','.join(self.transitions.closed)})"
```
</issue_to_address>
### Comment 2
<location path="newa/services/jira_service.py" line_range="204-208" />
<code_context>
+ # Get base NEWA ID (with regular hyphens)
+ base_newa_id = self.newa_id(action, True) if all_respins else self.newa_id(action)
+
+ # Search with both formats for backwards compatibility
+ # New format: non-breaking hyphens
+ formatted_newa_id = self._format_for_jira(base_newa_id)
+ # Old format: regular hyphens
+ original_newa_id = base_newa_id
+
+ # Build queries for both formats
</code_context>
<issue_to_address>
**suggestion (performance):** Unconditionally building two queries can be avoided when the formatted ID is identical to the original.
When `base_newa_id` has no hyphens, `formatted_newa_id` and `original_newa_id` are equal but you still issue two identical queries. Consider only appending the second query when the two IDs differ, e.g.:
```python
formatted_newa_id = self._format_for_jira(base_newa_id)
original_newa_id = base_newa_id
queries = []
if formatted_newa_id == original_newa_id:
queries.append(f"{base_filter} AND description ~ '{original_newa_id}'{status_filter}")
else:
queries.append(f"{base_filter} AND description ~ '{formatted_newa_id}'{status_filter}")
queries.append(f"{base_filter} AND description ~ '{original_newa_id}'{status_filter}")
```
</issue_to_address>
### Comment 3
<location path="newa/cli/jira_helpers.py" line_range="352-356" />
<code_context>
is_new = False
- if jira_handler.newa_id(action) in jira_issue["description"] \
+ # Use helper function for backwards-compatible comparison
+ if (jira_handler._newa_id_in_description(
+ jira_handler.newa_id(action), jira_issue["description"]) is not None
and (not action.parent_id
- or action.parent_id not in created_action_ids):
+ or action.parent_id not in created_action_ids)):
is_new = True
</code_context>
<issue_to_address>
**suggestion:** Accessing the handler’s private method `_newa_id_in_description` from CLI code couples modules to internals.
Calling `_newa_id_in_description(...)` from the CLI introduces a dependency on a private implementation detail of the Jira handler, which hurts modularity and future refactoring. Either promote this to a public API method or move the shared logic into a separate utility that both the service and CLI can use.
Suggested implementation:
```python
ctx.logger.debug(f"Checking {jira_issue_key}")
is_new = False
# Use Jira handler public helper for backwards-compatible comparison
if (jira_handler.newa_id_in_description(
jira_handler.newa_id(action), jira_issue["description"]) is not None
and (not action.parent_id
or action.parent_id not in created_action_ids)):
is_new = True
```
To fully implement the suggestion and keep the CLI decoupled from handler internals, you should also:
1. In the Jira handler module (where `_newa_id_in_description` is defined), either:
- Rename `_newa_id_in_description` to `newa_id_in_description` (removing the leading underscore), or
- Add a public wrapper method `def newa_id_in_description(self, newa_id: str, description: str): return self._newa_id_in_description(newa_id, description)`.
2. Ensure any other internal callers in the handler are updated consistently (either call the private or the new public method according to your conventions).
3. If you prefer a shared utility instead of a handler method, extract the logic from `_newa_id_in_description` into a utility function (e.g. in a `utils` or `jira_utils` module) and have both the handler and CLI call that utility; in that case, update this CLI call accordingly.
</issue_to_address>
### Comment 4
<location path="newa/cli/jira_helpers.py" line_range="64-73" />
<code_context>
n += 1
+def _sanitize_jira_field_value(value: Any) -> Any:
+ """
+ Sanitize Jira field values to prevent Jinja2 template conflicts.
+
+ Jira uses {{ }} for code formatting, which conflicts with Jinja2 syntax.
+ This function replaces the braces with double quotes.
+ """
+ if isinstance(value, str):
+ # Replace {{ and }} with double quotes
+ return value.replace('{{', '"').replace('}}', '"')
+ if isinstance(value, list):
+ return [_sanitize_jira_field_value(item) for item in value]
+ if isinstance(value, dict):
+ return {k: _sanitize_jira_field_value(v) for k, v in value.items()}
+ # Return other types unchanged (int, bool, None, objects, etc.)
+ return value
+
+
</code_context>
<issue_to_address>
**question (bug_risk):** Replacing `{{`/`}}` with double quotes can lead to odd or ambiguous content in templates.
Given the current behavior, a value like `"{{foo}}"` becomes `""foo""`, which can distort the rendered content and introduce confusing quoting. If the goal is only to prevent Jinja from treating `{{`/`}}` as delimiters, consider escaping the braces instead (e.g. wrapping in `{% raw %}` or replacing `{`/`}` with `\u007b`/`\u007d` or `\{`/`\}`) so the visual content is preserved without altering quotes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Use non-breaking hyphens (U+2011) in NEWA IDs to prevent Jira Cloud
from auto-linking issue keys like "RHELMISC-29371", which was corrupting
NEWA IDs and preventing issue reuse. Non-breaking hyphens look identical
to regular hyphens but don't trigger Jira's auto-linking.
Key changes:
- Add _format_for_jira() to replace hyphens with non-breaking hyphens
- Add _newa_id_in_description() helper for backwards-compatible matching
- New issues created with non-breaking hyphens in NEWA IDs
- Dual JQL search in get_related_issues() to find both old and new format
- Sanitize Jira field values to prevent Jinja2 template conflicts ({{ }})
Backwards compatibility:
- Old issues with regular hyphens are still found and reused
- Search performs two queries (one per format) and merges results
- Helper function matches NEWA IDs regardless of hyphen type
- Old issues are migrated to new format when refreshed/updated
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Previously, the _newa_id_in_description method was marked as private but was being accessed from CLI code in newa/cli/jira_helpers.py. This created inappropriate coupling between modules and violated encapsulation. The method provides backwards-compatible NEWA ID matching that checks for both regular hyphens (old format) and non-breaking hyphens (new format). This functionality is a legitimate part of the public API since it's needed by callers who need to check if a NEWA ID exists in a description. Changes: - Renamed _newa_id_in_description to newa_id_in_description (removed underscore) - Updated all references in both jira_service.py and jira_helpers.py - No functional changes, only API visibility improvement This addresses code review feedback about module coupling and follows Python conventions where underscore-prefixed methods should only be accessed within the class. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
kkaarreell
force-pushed
the
ks_nonbreakable_hyphens
branch
from
ca3814f to
edb7ff8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use non-breaking hyphens (U+2011) in NEWA IDs to prevent Jira Cloud from auto-linking issue keys like "RHELMISC-29371", which was corrupting NEWA IDs and preventing issue reuse. Non-breaking hyphens look identical to regular hyphens but don't trigger Jira's auto-linking.
Key changes:
Backwards compatibility:
🤖 Generated with Claude Code
Summary by Sourcery
Ensure NEWA IDs remain stable and reusable in Jira by preventing auto-linking and handling both legacy and new formats.
New Features:
Enhancements: