Add support for YouTrack in TodoByTicketRule

[DannyvdSluijs]

This adds support for YouTrack in the the TodoByTicketRule.

There is still some unresolved issue at this point in time for which some input could help regarding the approach.
With this PR there are now two implementations of the staabm\PHPStanTodoBy\utils\TicketStatusFetcher causing the Nette autowiring to throw an exception:

Multiple services of type staabm\PHPStanTodoBy\utils\TicketStatusFetcher found: 0397, 0398

@staabm I guess I could add an enabled flag on each of the ticket fetchers config and add entries to the conditionalTags section of the extension.neon file. But this could lead two multiple issue trackers being enabled, which isn't supported by the rule.
Also the conditionalTags seems undocumented which results in me maybe not seeing all the options here. Maybe this touches #48 as well.

I'm willing to continue the PR in order to get it to the finish line but could use some of you input in order to go in the right direction.

[staabm]

thanks for the PR.

regarding youtrack in general: does youtrack use the same issue-key-format as in jira?
meaning a project identifier followed by a number (will the jira regex work for youtrack as is?)

I guess I could add an enabled flag on each of the ticket fetchers config and add entries to the conditionalTags section of the extension.neon file.

I think we should instead utilize the enabled flag and turn it into a string parameter.

• enabled=false means the rule is disabled
• enabled=true means the rule uses the jira fetcher (for BC, but deprecated)
• enabled='jira' means the rule uses the jira fetcher
• enabled='youtrack' means the rule uses the youtrack fetcher

[staabm]

There is still some unresolved issue at this point in time for which some input could help regarding the approach.
With this PR there are now two implementations of the staabm\PHPStanTodoBy\utils\TicketStatusFetcher causing the Nette autowiring to throw an exception:

I think we could typehint the concrete implementations to make the dependencies unambigous

[EmilMassey]

Hi, nice to see upcoming support for another issue tracker.

Recently I started working on introducing GitHub support (which has completely different ticket keys: #123) and this requires some refactoring to be able to support multiple issue trackers. See the draft: #62. To be able to choose a fetcher dynamically, we can use factory which has DI container injected. This way we can read parameters to decide which service to use.

Adding your fetcher would be as easy as adding those lines to the factory:

if ('youtrack' === $tracker) {
    $fetcher = $this->container->getByType(YouTrackTicketStatusFetcher::class);

    return new TicketRuleConfiguration(
        $fetcher::getKeyPattern(),
        $resolvedStatuses,
        $keyPrefixes,
        $fetcher,
    );
}

What do you think?

[staabm]

I like the direction of #62 - we should wait with this PR here until #62 is merged

[DannyvdSluijs]

I see now #62 is merged so I can rebase/redo this work with minimal effort. Great work by @EmilMassey this was the part I was missing.

I’ll try and see if I can update this PR in the upcoming week.

[staabm]

could we have a similar e2e test for youtrack, as we have added recently for jira?

#71

[staabm]

@EmilMassey any input from your side?

[staabm]

for discussion: in jira we use issue-states to tell when/whether a ticket is resolved. can/should we do the same for youtrack? does youtrack also have tickets flow thru different states?

[DannyvdSluijs]

Youtrack uses columns to track the state.

From the API documentation the resolved field of the Issue [1] Entity is explained as

The timestamp of the moment when the issue was assigned a state that is considered to be resolved. null if the issue is still in an unresolved state. Read-only. Can be null.

There is something called IssueCustomField [2] which allows you to track custom fields such as state of a ticket. But this is specific to each setup and isn't being used in our setup. So adding support for this is challenging. I would rather stick to the definition of YouTrack itself and it people really need to use custom field support creating a PR to extend the youtrack integration can be done.

[staabm]

one nit, lgtm otherwise.

thank you.

I let sit it for a few days so @EmilMassey has another chance to look into it

[EmilMassey]

Apart from these two small things I mentioned, it looks really good to me

[staabm]

thank you guys