Add Jira issue selector to collect issues since last successful build (#453)

[ahmedanwar1]

Related issue

Resolves #453

Changes

• Added SinceLastSuccessfulBuildIssueSelector to collect JIRA issues from all builds since the last successful build, excluding the last successful build itself.
• Refactored AbstractIssueSelector and DefaultIssueSelector for reusability and consistent issue extraction logic across selectors.
• Added comprehensive unit tests for the new selector to ensure correct issue collection.

Tests

• Verified that issues are correctly collected from multiple builds after the last successful build.
• Confirmed that issues from the last successful build are excluded.
• Checked that log messages reflect the correct number of builds and issues processed.

• I have updated/added relevant documentation in the docs/ directory
• I have verified that the Code Coverage is not lower than before / that all the changes are covered as needed
• I have tested my changes with a Jira Cloud / Jira Server

[Copilot]

Pull Request Overview

This PR adds a new SinceLastSuccessfulBuildIssueSelector that collects JIRA issues from all builds since the last successful build, excluding the last successful build itself. This addresses issue #453 for collecting issues across multiple failed/unstable builds.

Key changes:

• New issue selector implementation that traverses builds backwards until the last successful build
• Refactored shared code from DefaultIssueSelector into AbstractIssueSelector base class
• Comprehensive unit tests for the new selector functionality

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
SinceLastSuccessfulBuildIssueSelector.java	New selector that collects issues from builds since last successful build
AbstractIssueSelector.java	Refactored to include shared issue extraction methods moved from DefaultIssueSelector
DefaultIssueSelector.java	Removed methods that were moved to AbstractIssueSelector base class
Messages.properties	Added display name for the new selector
SinceLastSuccessfulBuildIssueSelectorTest.java	Comprehensive unit tests for the new selector

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[rantoniuk]

@ahmedanwar1 thanks for contributing!

Before going into details:

• can you remove the AbstractIssueSelector changes? This class is Abstract on purpose and should not contain any logic and I don't why would you need those changes for the new selector you're providing

• please extend the documentation, i.e. features.md and preferably as well in the use-cases.md, since this selector is a more complex one. If you wish, you can extract all selectors documetantion into a separate Markdown file.

[ahmedanwar1]

Thanks for the feedback @rantoniuk

I modified AbstractIssueSelector to avoid code duplication, both DefaultIssueSelector and the new selector need the same logic for extracting issues from changelogs and parameters. Having SinceLastSuccessfulBuildIssueSelector depend on DefaultIssueSelector methods seemed architecturally wrong since they're independent strategies

Would you prefer a separate utility class like IssueSelectorUtils for shared logic or is some duplication acceptable to keep selectors self contained or you suggest another approach?

and should this selector also collect issues from the build parameters like DefaultIssueSelector does or is changelog only sufficient?

[rantoniuk]

Would you prefer a separate utility class like IssueSelectorUtils for shared logic or is some duplication acceptable to keep selectors self contained or you suggest another approach?

I'm fine with both solutions, i.e. extend DefaultIssueSelector or extract to IssueSelectorUtils.

and should this selector also collect issues from the build parameters like DefaultIssueSelector does or is changelog only sufficient?

The reason DefaultIssueSelector does this is described in JENKINS-12312, so I think we need the same behavior here.

[ahmedanwar1]

I may tend to prefer extracting the shared logic into an IssueSelectorUtils class. i think extending DefaultIssueSelector feels semantically off and would introduce tighter coupling between the selectors

[ahmedanwar1]

Actually after looking deeper through the codebase, I realized IssueSelectorUtils won't work well. I found that JobIssueSelector extends DefaultIssueSelector and overrides addIssuesFromChangeLog()

so the current design relies on inheritance and method overriding and the utils class would break that flow

I think adding an abstract class BaseIssueSelector in between AbstractIssueSelector and the concrete selectors to hold the shared logic could be a cleaner solution

this approach keeps AbstractIssueSelector as-is, JobIssueSelector can still override methods from the base class and avoids code duplication and the semantic issue of having SinceLastSuccessfulBuildIssueSelector extend DefaultIssueSelector

The hierarchy would be:

AbstractIssueSelector --> BaseIssueSelector --> DefaultIssueSelector
AbstractIssueSelector --> BaseIssueSelector --> SinceLastSuccessfulBuildIssueSelector

What do you think?

[rantoniuk]

Let's leave it then as it is now (in AbstractIssueSelector), just make sure to cover the changes with tests accordingly (see some missing branches).

Edit: I actually need to see if this is valid at all - see my last comment. Allow me some time to test this on a live instance.

[rantoniuk]

@ahmedanwar1 I looked at the implementation and I'm wondering what do you think about a different approach.

How about having a sinceLastSuccessfulBuild flag in the DefaultIssueSelector, with a default setting of false (for backwards compatibility)?
I feel that this would simplify the change, maintenance and also changes to the actual pipeline DSLs. Would that also work for you?

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rantoniuk]

@ahmedanwar1 did you have a chance to look at my previous comment? Any thoughts?

[ahmedanwar1]

Hi @rantoniuk
Sorry for the late response, I’ve been quite busy over the past few months

From a design perspective i leaned toward keeping DefaultIssueSelector and SinceLastSuccessfulBuildIssueSelector as independent implementations since they represent different selection strategies

Introducing a flag in DefaultIssueSelector would couple multiple behaviors into a single class which could make it harder to maintain or extend if additional selector variations are added in the future

Keeping them separate helps preserve a clearer separation of concerns and avoids accumulating conditional logic in one place over time

What do you think is the best fit?