Add support for linking issues from an erratum#313
Merged
kkaarreell merged 2 commits intomainfrom Feb 7, 2026
Merged
Conversation
Reviewer's GuideAdds support for fetching Jira issues from ErrataTool into the Erratum model, extends Jira link configuration/rendering to handle dynamic list-based templates, refactors Jira link creation into a dedicated method with logging and validation, wires logging into the Jira handler, and updates tests and README to cover the new behavior. Sequence diagram for Jira issue creation with validated dynamic linkssequenceDiagram
actor User
participant JiraCLI as JiraCLICommand
participant Helpers as JiraHelpers
participant Handler as IssueHandler
participant Jira as JiraAPI
User->>JiraCLI: Run cmd_jira with config
JiraCLI->>Handler: Initialize IssueHandler(group, board, logger)
JiraCLI->>Helpers: _render_action_fields(action, artifact_job, jira_handler, jira_event_fields)
Helpers->>Helpers: Render fields and links (including templates)
Helpers-->>JiraCLI: rendered_fields, rendered_links
JiraCLI->>Handler: create_issue(summary, description, rendered_fields, rendered_links, transition_name)
Handler->>Jira: create_issue
Jira-->>Handler: jira_issue
Handler->>Jira: update_issue_fields
Jira-->>Handler: ok
Handler->>Handler: Build Issue instance new_issue
alt links_present
Handler->>Handler: add_issue_links(new_issue, rendered_links)
Handler->>Handler: get_details(new_issue)
Handler-->>Handler: existing_links
loop each relation and linked_key
Handler->>Jira: issue(linked_key) to verify existence
alt issue_exists_and_not_linked
Handler->>Jira: create_issue_link(type, new_issue.id, linked_key)
Jira-->>Handler: ok
Handler->>Handler: log info via logger
else already_linked_or_missing
Handler->>Handler: log debug via logger
end
end
end
Handler-->>JiraCLI: new_issue
JiraCLI-->>User: Report created or reused issue with links
Class diagram for updated Jira and Errata handlingclassDiagram
class IssueHandler {
Optional~str~ group
Optional~str_int~ board
Optional~logging_Logger~ logger
connection() jira_JIRA
create_issue(summary, description, fields, links, transition_name) Issue
add_issue_links(issue, links) void
get_details(issue) jira_Issue
drop_obsoleted_issue(issue, obsoleted_by) void
}
class ErrataService {
url str
fetch_info(erratum_id) JSON
fetch_releases(erratum_id) JSON
fetch_blocking_errata(erratum_id) JSON
fetch_jira_issues(erratum_id) JSON
get_errata(event, process_blocking_errata) list~Erratum~
}
class Erratum {
id str
product Optional~str~
release Optional~str~
status Optional~str~
people_assigned_to Optional~str~
people_package_owner Optional~str~
people_qe_group Optional~str~
people_devel_group Optional~str~
revision Optional~int~
jira_issues list~str~
}
class JiraHelpers {
_render_action_value(value, artifact_job, action, jira_event_fields) str
_render_action_fields(action, artifact_job, jira_handler, jira_event_fields) tuple
}
class JiraCLICommand {
cmd_jira(ctx, config) void
}
IssueHandler "1" --> "*" Erratum : uses_in_templates
ErrataService "1" --> "*" Erratum : creates
JiraHelpers --> Erratum : reads_jira_issues
JiraHelpers --> IssueHandler : passes_rendered_links
JiraCLICommand --> IssueHandler : constructs_with_logger
ErrataService --> ErrataToolAPI : calls
class ErrataToolAPI {
fetch_jira_issues(erratum_id) JSON
}
Flow diagram for fetching erratum Jira issues and using them in dynamic linksflowchart LR
subgraph ErrataTool
ET_API["ErrataTool API"]
end
subgraph NEWA
ES["ErrataService.get_errata"]
ERR["Erratum model (jira_issues)"]
JH["JiraHelpers._render_action_fields"]
IH["IssueHandler.add_issue_links"]
end
subgraph Jira
JIRA_API["Jira REST API"]
end
ET_API -->|"fetch_jira_issues(erratum_id)"| ES
ES -->|jira_issues list| ERR
ERR -->|ERRATUM.jira_issues in templates| JH
JH -->|"rendered_links (lists of issue keys)"| IH
IH -->|verify target issue| JIRA_API
IH -->|create_issue_link| JIRA_API
File-Level Changes
Possibly linked issues
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 5 issues, and left some high level feedback:
- The
return_nativebranch in_render_action_valuemanually strips{{ }}and walks attributes viagetattr, which won’t handle common Jinja2 use cases (filters, indexing, dict access, method calls, etc.); consider instead using aNativeEnvironmentor otherwise evaluating the template directly via Jinja to preserve full expression support and avoid duplicating its semantics. - In
add_issue_links,issue_details.fields.issuelinksis accessed unconditionally; ifissuelinksis missing on the Jira issue, this will raise anAttributeError, so it would be safer to usegetattr(issue_details.fields, 'issuelinks', [])before iterating.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `return_native` branch in `_render_action_value` manually strips `{{ }}` and walks attributes via `getattr`, which won’t handle common Jinja2 use cases (filters, indexing, dict access, method calls, etc.); consider instead using a `NativeEnvironment` or otherwise evaluating the template directly via Jinja to preserve full expression support and avoid duplicating its semantics.
- In `add_issue_links`, `issue_details.fields.issuelinks` is accessed unconditionally; if `issuelinks` is missing on the Jira issue, this will raise an `AttributeError`, so it would be safer to use `getattr(issue_details.fields, 'issuelinks', [])` before iterating.
## Individual Comments
### Comment 1
<location> `newa/services/jira_service.py:44` </location>
<code_context>
group: Optional[str] = field(default=None)
board: Optional[Union[str, int]] = field(default=None)
+ logger: Optional[Any] = field(default=None)
@property
</code_context>
<issue_to_address>
**suggestion:** Use a more specific logger type instead of `Any` for better type safety.
Since this field is always a logger, annotate it as something like `Optional[logging.Logger]` or `Optional[logging.LoggerAdapter]` so type checkers and IDEs can validate logger usage instead of accepting arbitrary objects.
Suggested implementation:
```python
group: Optional[str] = field(default=None)
board: Optional[Union[str, int]] = field(default=None)
logger: Optional["logging.Logger"] = field(default=None)
```
` section.
Here are the edits:
<file_operations>
<file_operation operation="edit" file_path="newa/services/jira_service.py">
<<<<<<< SEARCH
group: Optional[str] = field(default=None)
board: Optional[Union[str, int]] = field(default=None)
logger: Optional[Any] = field(default=None)
=======
group: Optional[str] = field(default=None)
board: Optional[Union[str, int]] = field(default=None)
logger: Optional["logging.Logger"] = field(default=None)
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
1. Ensure `logging` is imported at the top of `newa/services/jira_service.py`, for example:
- `import logging`
2. If you prefer supporting both plain loggers and logger adapters, you can instead use:
- `logger: Optional[Union["logging.Logger", "logging.LoggerAdapter"]] = field(default=None)`
and import `Union` if not already imported.
3. If `Any` is not used elsewhere in this file, you can remove it from any `from typing import ...` import list.
</issue_to_address>
### Comment 2
<location> `newa/services/jira_service.py:453-459` </location>
<code_context>
+ return
+
+ # Fetch existing issue links once before the loop
+ issue_details = self.get_details(issue)
+ existing_links = issue_details.fields.issuelinks
+ short_sleep()
+
+ # Build set of already linked issue keys for fast lookup
+ existing_linked_keys = set()
+ for link in existing_links:
+ if hasattr(link, 'inwardIssue'):
+ existing_linked_keys.add(link.inwardIssue.key)
</code_context>
<issue_to_address>
**issue:** Guard against missing or `None` `issuelinks` to avoid runtime errors.
`existing_links` is assumed iterable, but in some Jira setups `fields.issuelinks` may be missing or `None`, causing `TypeError` in the `for` loop. Consider normalizing, e.g. `existing_links = getattr(issue_details.fields, "issuelinks", []) or []`, so the loop safely handles cases with no links.
</issue_to_address>
### Comment 3
<location> `newa/cli/jira_helpers.py:89` </location>
<code_context>
+ return_native: bool = False) -> Any:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The manual expression resolution in `return_native` mode is limited and may produce surprising results for complex templates.
In the `return_native` path, the expression is resolved by splitting on `'.'` and walking attributes, so it only handles simple chains like `ERRATUM.jira_issues`. More complex Jinja expressions (indexes, filters, method calls, etc.) fall back to `template.render`, which returns a string and can silently change the expected type (e.g., list vs string). Please either document that `return_native` only supports simple attribute access, or switch to a more robust native-evaluation approach (e.g., Jinja’s `NativeEnvironment` or `compile_expression`) so results are consistently native-typed.
```suggestion
return_native: If True, return native type (e.g., list) instead of string representation.
Only simple dotted attribute paths (e.g., ``ERRATUM.jira_issues``) are resolved
natively by walking attributes. More complex Jinja expressions (indexes, filters,
method calls, etc.) are rendered via ``template.render`` and will return a string.
```
</issue_to_address>
### Comment 4
<location> `tests/unit/test_cli.py:66-68` </location>
<code_context>
""" Return empty json for blocking errata """
return {}
+ def mock_et_fetch_jira_issues(self, id: str):
+ """ Return empty list for jira issues """
+ return []
+
def mock_et_fetch_system_info(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add assertions that `Erratum.jira_issues` is correctly populated and passed through to the CLI logic
`mock_et_fetch_jira_issues` always returns an empty list, and there are no assertions around `Erratum.jira_issues` or its use in the CLI (e.g. `_render_action_fields` with `{{ ERRATUM.jira_issues }}` in `action.links`). Please add or extend a test that:
- sets `mock_et_fetch_jira_issues` to return a non-empty payload (e.g. `[{"key": "JIRA-1"}, {"key": "JIRA-2"}]`),
- exercises the path that builds `Erratum` objects via `get_errata` and processes an action whose links reference `{{ ERRATUM.jira_issues }}`, and
- asserts the rendered links include the expected Jira keys.
This will verify the new endpoint is correctly wired into `Erratum` and the JIRA CLI flow.
Suggested implementation:
```python
def mock_et_fetch_jira_issues(self, id: str):
""" Return list for jira issues used to populate Erratum.jira_issues in CLI tests """
return [{"key": "JIRA-1"}, {"key": "JIRA-2"}]
```
To fully implement your review comment, you will also need to:
1. Add or extend a CLI test that:
- Uses the existing fixture/setup that wires `fetch_jira_issues` to `mock_et_fetch_jira_issues`.
- Calls the CLI command (or helper) that internally invokes `get_errata` and then processes an action whose `links` include a template reference to `{{ ERRATUM.jira_issues }}` (for example, via `_render_action_fields`).
- Asserts the rendered CLI output (e.g., printed links or returned structure) contains both `"JIRA-1"` and `"JIRA-2"` in the expected place.
2. Place this new test at module scope in `tests/unit/test_cli.py`, alongside the other CLI tests, ensuring it reuses existing fixtures rather than recreating them.
Because only a small fragment of `tests/unit/test_cli.py` is visible here, you will need to:
- Find the existing test that exercises actions/links rendering (it likely already asserts on other `ERRATUM` fields),
- Extend its assertions to check that rendered links include `JIRA-1` and `JIRA-2`, or
- Create a new test (e.g. `test_cli_renders_jira_issues_in_action_links`) that calls the same code path and adds the appropriate assertions.
</issue_to_address>
### Comment 5
<location> `tests/unit/test_cli.py:83` </location>
<code_context>
monkeypatch.setattr(newa.ErrataTool, 'fetch_info', mock_et_fetch_info)
monkeypatch.setattr(newa.ErrataTool, 'fetch_releases', mock_et_fetch_releases)
monkeypatch.setattr(newa.ErrataTool, 'fetch_blocking_errata', mock_et_fetch_blocking_errata)
+ monkeypatch.setattr(newa.ErrataTool, 'fetch_jira_issues', mock_et_fetch_jira_issues)
monkeypatch.setattr(newa.ErrataTool, 'fetch_system_info', mock_et_fetch_system_info)
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for link rendering when `action.links` is a template string resolving to a list and for invalid link configurations
Given the new `_render_action_value(..., return_native=True)` behavior and updated `_render_action_fields`, please add tests that cover:
1. `action.links["rel"] = "{{ ERRATUM.jira_issues }}"` where `ERRATUM.jira_issues` is a list, asserting `_render_action_fields` returns that list unchanged.
2. `action.links["rel"]` as a list of template strings, asserting each element is rendered and collected correctly.
3. Invalid types for `action.links["rel"]` (e.g., `42`, `{}`), asserting an exception is raised with the expected message.
This will lock in the new link templating semantics and prevent regressions.
Suggested implementation:
```python
monkeypatch.setattr(newa.ErrataTool, 'fetch_blocking_errata', mock_et_fetch_blocking_errata)
# New tests for link rendering and validation
def test_render_action_links_rel_template_resolves_to_list():
"""
When action.links["rel"] is a single template string whose rendered value
is a list, _render_action_fields should return that list unchanged.
"""
from elliottlib.cli import _render_action_fields # adjust import if different
# Context with ERRATUM.jira_issues as a list
context = {
"ERRATUM": {
"jira_issues": ["JIRA-1", "JIRA-2"],
}
}
# action.links["rel"] is a template that resolves to the jira_issues list
action = {
"links": {
"rel": "{{ ERRATUM.jira_issues }}",
}
}
rendered = _render_action_fields(action, context)
assert "links" in rendered
assert "rel" in rendered["links"]
# The rendered rel should be exactly the list from context
assert rendered["links"]["rel"] == ["JIRA-1", "JIRA-2"]
def test_render_action_links_rel_list_of_templates():
"""
When action.links["rel"] is a list of template strings, each element
should be rendered and collected into the resulting list.
"""
from elliottlib.cli import _render_action_fields # adjust import if different
context = {
"ERRATUM": {
"jira_issues": ["JIRA-1", "JIRA-2"],
}
}
# Each element is a template pointing at an individual jira issue
action = {
"links": {
"rel": [
"{{ ERRATUM.jira_issues[0] }}",
"{{ ERRATUM.jira_issues[1] }}",
],
}
}
rendered = _render_action_fields(action, context)
assert "links" in rendered
assert "rel" in rendered["links"]
assert rendered["links"]["rel"] == ["JIRA-1", "JIRA-2"]
def test_render_action_links_rel_invalid_types_raise():
"""
Invalid types for action.links["rel"] should raise with an informative message.
"""
import pytest
from elliottlib.cli import _render_action_fields # adjust import if different
context = {
"ERRATUM": {
"jira_issues": ["JIRA-1"],
}
}
# rel is an int -> invalid
action_int = {
"links": {
"rel": 42,
}
}
# rel is a dict -> invalid
action_dict = {
"links": {
"rel": {"foo": "bar"},
}
}
with pytest.raises((TypeError, ValueError), match=r"links\['rel'\]"):
_render_action_fields(action_int, context)
with pytest.raises((TypeError, ValueError), match=r"links\['rel'\]"):
_render_action_fields(action_dict, context)
```
1. Adjust the import path for `_render_action_fields` at the top of the new tests if it lives in a different module than `elliottlib.cli`.
2. If the real error type or message for invalid `links["rel"]` is different, update the `pytest.raises` tuple and `match` argument to match the actual behavior (e.g. a specific custom exception class or a more precise error message).
3. Ensure that any required fixtures or helpers used by other tests (e.g. when they construct `action` objects) are reused here if your test suite prefers a specific `action` structure instead of plain dicts; in that case, replace the literal `action = {...}` with calls to those helpers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kkaarreell
force-pushed
the
ks_link_erratum_issues
branch
from
bd8ebff to
be78b33
Compare
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
_render_action_valuebranch that usesNativeEnvironmentcreates a fresh Jinja environment without any of the filters/extensions used byrender_template; consider wiring it through the same environment (or copying its configuration/filters) so templates behave consistently regardless ofreturn_native. - In
_render_action_fields, when a string-valuedlinks[relation]renders to a list viareturn_native=True, the resulting list is passed through unchanged; you may want to validate/normalize each element to a string to avoid accidentally propagating non-string values as issue keys. test_erratum_jira_issues_populatednever actually asserts thatjira_issuesis present or has the expected values (the assertion is commented as “might not be in the YAML”); it would be clearer either to assert the intended behavior explicitly or to narrow the test’s scope to what is actually guaranteed today.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `_render_action_value` branch that uses `NativeEnvironment` creates a fresh Jinja environment without any of the filters/extensions used by `render_template`; consider wiring it through the same environment (or copying its configuration/filters) so templates behave consistently regardless of `return_native`.
- In `_render_action_fields`, when a string-valued `links[relation]` renders to a list via `return_native=True`, the resulting list is passed through unchanged; you may want to validate/normalize each element to a string to avoid accidentally propagating non-string values as issue keys.
- `test_erratum_jira_issues_populated` never actually asserts that `jira_issues` is present or has the expected values (the assertion is commented as “might not be in the YAML”); it would be clearer either to assert the intended behavior explicitly or to narrow the test’s scope to what is actually guaranteed today.
## Individual Comments
### Comment 1
<location> `tests/unit/test_cli.py:786-787` </location>
<code_context>
+ yaml_data = yaml_parser().load(event_file.read_text())
+ erratum = yaml_data.get('erratum')
+ assert erratum is not None
+ # The jira_issues should be populated by the ErrataTool fetch
+ # Note: Currently this might not be in the YAML, as it depends on
+ # whether ErrataTool.fetch_jira_issues is being called during event processing
+
</code_context>
<issue_to_address>
**issue (testing):** This test does not currently assert that `jira_issues` are present/populated, so it doesn't verify the behavior described in its name and docstring.
As written, this will pass even if `jira_issues` is never written to the YAML or is empty. Please either:
- add assertions on `erratum['jira_issues']` (that it exists and matches the values from `_mock_errata_tool`, e.g. `['JIRA-1', 'JIRA-2']`), or
- if `jira_issues` is not expected in the YAML yet, adjust the test name/docstring or mark it `xfail` with a clear reason to avoid a false sense of coverage.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kkaarreell
force-pushed
the
ks_link_erratum_issues
branch
from
be78b33 to
b3ae582
Compare
kkaarreell
force-pushed
the
ks_link_erratum_issues
branch
2 times, most recently
from
71fa8a7 to
e8a2b45
Compare
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
IssueHandler.add_issue_links, existing links are de-duplicated solely by issue key, so you will skip creating a link with a different relation type to an already-linked issue; if multiple relations to the same key should be allowed, consider tracking existing links by both relation and key. - The per-key existence check in
add_issue_links(self.connection.issue(linked_key)) can become expensive for configurations with many links; you may want to make this validation optional or narrow the exception handling to only skip on 404-like errors while surfacing other failures. - In
ErrataTool.get_errata,fetch_jira_issuesis assumed to return a list of dicts with akeyfield; consider adding defensive handling (e.g., try/except or type/shape checks) to fall back to an empty list rather than raising if the API response format changes or is unexpected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `IssueHandler.add_issue_links`, existing links are de-duplicated solely by issue key, so you will skip creating a link with a different relation type to an already-linked issue; if multiple relations to the same key should be allowed, consider tracking existing links by both relation and key.
- The per-key existence check in `add_issue_links` (`self.connection.issue(linked_key)`) can become expensive for configurations with many links; you may want to make this validation optional or narrow the exception handling to only skip on 404-like errors while surfacing other failures.
- In `ErrataTool.get_errata`, `fetch_jira_issues` is assumed to return a list of dicts with a `key` field; consider adding defensive handling (e.g., try/except or type/shape checks) to fall back to an empty list rather than raising if the API response format changes or is unexpected.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kkaarreell
force-pushed
the
ks_link_erratum_issues
branch
from
e8a2b45 to
16089e1
Compare
Fetch and store Jira issue keys associated with errata to enable better
tracking and linking between errata and their related Jira issues.
Changes:
- Add jira_issues attribute to Erratum model to store list of Jira keys
- Add fetch_jira_issues() method to ErrataTool to retrieve Jira issues
from /advisory/{id}/jira_issues.json endpoint
- Update get_errata() to fetch and populate Jira issue keys for each erratum
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit implements comprehensive support for creating Jira issue links
defined in the issue-config `links` attribute, ensuring they work for both
newly created issues and re-used existing issues.
Key improvements:
1. Extract link creation into reusable method:
- Created `add_issue_links()` method in IssueHandler to handle link
creation with existence checking and validation
- Refactored `create_issue()` to use the new method instead of inline code
- Follows same optimization pattern as `link_erratum_issues()` (fetch all
existing links once, use set for fast lookup)
2. Enable links for re-used issues:
- Call `add_issue_links()` when existing issues are re-used (not just
when created)
- Ensures links defined in issue-config are always applied regardless of
whether issue is new or re-used
3. Support Jinja2 template references to list variables:
- Enhanced link rendering to handle both list of strings and template
references to list variables using YAML parsing approach
- Unified with recipes.py pattern: render template as string, then parse
as YAML to get native types
- Enables use cases like `"is related to": "{{ ERRATUM.jira_issues }}"` to
dynamically link to all erratum Jira issues
- Removed NativeEnvironment dependency in favor of consistent YAML parsing
4. Documentation:
- Added comprehensive section on using links in issue configuration
- Documented three supported formats: static lists, template strings, and
template references to list variables
- Updated example configuration to show dynamic linking
The implementation ensures link creation is idempotent (checks for existing
links and target issue existence), provides appropriate debug/info logging,
and works consistently across all issue lifecycle states.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
kkaarreell
force-pushed
the
ks_link_erratum_issues
branch
from
16089e1 to
cb14366
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.
Summary by Sourcery
Integrate erratum-associated Jira issues into Jira linking workflows and enhance link configuration flexibility and safety.
New Features:
Enhancements:
Tests: