lesstif / php-jira-rest-client

PHP classes interact Jira with the REST API.
Other
510 stars 263 forks source link

Method addIssueLink() does not return a result (only NULL) #403

Closed igittigitt closed 2 years ago

igittigitt commented 2 years ago

I wondered why i always get NULL in return of calling addIssueLink($link) in my code. I've expected something JSON in return, as from the Jira API-Docs about issueLinks.

A quick look into the code (src/IssueLink/IssueLinkService.php) shows why. It's because the method addIssueLink just ends an do not return anything:

    public function addIssueLink($issueLink)
    {
        $this->log->info("addIssueLink=\n");

        $data = json_encode($issueLink);

        $this->log->debug("Create IssueLink=\n".$data);

        $url = $this->uri.'/issueLink';
        $type = 'POST';

        $this->exec($url, $data, $type);
    }

I've simply added a "return" in front of the last line, which gives me a TRUE on success and an json-string on error. I'm shure this can be done better but it should be done :-)

lesstif commented 2 years ago

hi @igittigitt .

in accordance to the jira api document mentioned by you, JIRA just to return HTTP 201 status code when issue link create successfully.

so I think it's the correctly action to return NULL in this state.

thanks!

igittigitt commented 2 years ago

Hi @lesstif , thanks for answering. As you say, on success the REST API give back an HTTP 201, and if fail there will be a 40x or 50x. So it gives something back, what your code don't respect because it just always return NULL. So the caller never knows if the action took place or not. I would therefore strongly suggest to implement one of those options: 1.) throw an exception on an result other than 20x 2.) return TRUE of HTTP 20x, otherwise FALSE 3.) return what comes from the $this->exec an so give the caller the correct HTTP response

lesstif commented 2 years ago

Hi @igittigitt, thanks for your detailed reply.

I'm on the same page, but sorry I can't have time for this modification.

could you make this PR, please?

thanks.

igittigitt commented 2 years ago

Thanks mate, i added a PR for it