kbsali / php-redmine-api

A simple PHP Redmine API client, Object Oriented
MIT License
420 stars 183 forks source link

XML/JSON parameter mismatch in TimeEntry.php #401

Closed LPRKR closed 4 months ago

LPRKR commented 4 months ago

Hello! Using the 2.6.0 API on PHP 8.2. Creating a time entry works, but results in error messages like this:

Warning: SimpleXMLElement::__construct(): Entity: line 1: parser error : Start tag expected, '<' not found in vendor\kbsali\redmine-api\src\Redmine\Api\TimeEntry.php on line 146

Warning: SimpleXMLElement::__construct(): {"log_type":"T","id":430626,"hours":3.0,"comments":"automated entry","spent_on in vendor\kbsali\redmine-api\src\Redmine\Api\TimeEntry.php on line 146

Warning: SimpleXMLElement::__construct(): ^ in vendor\kbsali\redmine-api\src\Redmine\Api\TimeEntry.php on line 146

Fatal error: Uncaught Exception: String could not be parsed as XML in vendor\kbsali\redmine-api\src\Redmine\Api\TimeEntry.php:146 Stack trace: #0 vendor\kbsali\redmine-api\src\Redmine\Api\TimeEntry.php(146): SimpleXMLElement->__construct('{"log_type":"T"...') #1 get_users.php(52): Redmine\Api\TimeEntry->create(Array) #2 {main} thrown in vendor\kbsali\redmine-api\src\Redmine\Api\TimeEntry.php on line 146

After a quick source code lookup, it tries to parse this exact string as XML in ->create() method by "return new SimpleXMLElement($body);", which is a JSON, not XML.

{"log_type":"T","id":430626,"hours":3.0,"comments":"automated entry","spent_on":"2024-06-14","project":{"id":4506},"issue":{"id":55757},"activity":{"id":53},"user":{"id":3094},"spentFor":{"id":150199,"start_on":"2024-06-14T00:00:00.000Z","end_on":null,"clock_action":null}} 

I am using this code to create a time entry:

$create_time_entry_result = $client->getApi('time_entry')->create([
    'project_id' => 4506,
    'issue_id' => 55757,
    'spent_on' => '2024-06-14',
    'hours' => 3,
    'activity_id' => 53,
    'comments' => 'automated entry'
]);

Seems like an API error, could I ask for a fix? :)

Art4 commented 4 months ago

Hey, thank you very much for reporting this error.

TimeEntry::create() internally uses the XML endpoint /time_entries.xml, see https://github.com/kbsali/php-redmine-api/blob/v2.6.0/src/Redmine/Api/TimeEntry.php#L137-L147

The Redmine server should response with XML, too. We even have tests for this case: https://github.com/kbsali/php-redmine-api/blob/v2.6.0/tests/Behat/features/time_entry.feature#L15-L17

I can only imagine that your Redmine server contains a bug, which is why it responds with JSON instead of XML.

Can you tell me which version of Redmine you are using? Then I can investigate the problem in more detail.

LPRKR commented 4 months ago

Thanks! Redmine reports itself as 5.1.1 stable. There is also ERPmine (redmine_wktime) plugin installed - dunno if it affects anything.

Art4 commented 4 months ago

Thank you, I will investigate this issue.

Art4 commented 4 months ago

I have investigated the problem. I could not reproduce the problem with Redmine 5.1.1, and the tests were all successful.

I strongly suspect that the problem is with the redmine_wktime plugin. My Ruby knowledge is not good enough, but it looks like responses are only sent as JSON (XML is not supported). See for example: https://github.com/dhanasingh/redmine_wktime/blob/v4.7.2/app/controllers/wklogmaterial_controller.rb#L136

Please contact the maintainer of the plugin for further support.

As a workaround: There is still no easy way to select to send the requests as JSON instead of XML (see #146). However, you can use the low-level API to call the time_entries.json endpoint directly.

This could look like this (untested):

$data = [
    'project_id' => 4506,
    'issue_id' => 55757,
    'spent_on' => '2024-06-14',
    'hours' => 3,
    'activity_id' => 53,
    'comments' => 'automated entry'
];

$client->requestPost(
    '/time_entries.json', 
    JsonSerializer::createFromArray(['time_entry' => $data])->getEncoded(),
);

Please keep in mind that this way you are on your own with validating the input parameters and react to the server response.

LPRKR commented 4 months ago

Thank you for the explanation! Yes, it seems JSON is the only format supported.

The workaround is okay, but the $client->requestPost() returns TRUE instead of some structure with a newly assigned ID. Is there something I'm missing?

Anyway, no big deal if it can't be solved on library side, in the worst case I may stick with doing a raw API call for this endpoint. I need to make the script work between few Redmine instances (not sure if all of them use the redmine_wktime plugin), so it needs to handle both cases somehow :)

Art4 commented 4 months ago

We are working on improving the handling of the responses (see #367). To have a server independent solution today you have multiple options:

Option 1 (future-proof):

You could work with the new Redmine\Http\HttpClient interface, that is implemented by the Redmine client.

First you need a Request implementation

final class JsonRequest implements \Redmine\Http\Request
{
    public function __construct(
        private string $method,
        private  string $path,
        private  string $content,
    ) {}

    public function getMethod(): string
    {
        return $this->method;
    }

    public function getPath(): string
    {
        return $this->path;
    }

    public function getContentType(): string
    {
        return 'application/json';
    }

    public function getContent(): string
    {
        return $this->content;
    }
}

Now you could use the HttpClient like this (untested):

$data = [
    'project_id' => 4506,
    'issue_id' => 55757,
    'spent_on' => '2024-06-14',
    'hours' => 3,
    'activity_id' => 53,
    'comments' => 'automated entry'
];

$response = $client->request(
    new JsonRequest(
        'POST',
        '/time_entries.json',
        \Redmine\Serializer\JsonSerializer::createFromArray(['time_entry' => $data])->getEncoded(),
    ),
);

The Response object contains everything you need to get the new ID.

$timeEntryDetails = json_decode($response->getContent());

Option 2 (deprecated):

Use the Client::getLastResponseBody() that will be deprecated in future (see #391):

$data = [
    'project_id' => 4506,
    'issue_id' => 55757,
    'spent_on' => '2024-06-14',
    'hours' => 3,
    'activity_id' => 53,
    'comments' => 'automated entry'
];

$client->requestPost(
    '/time_entries.json', 
    JsonSerializer::createFromArray(['time_entry' => $data])->getEncoded(),
);

$timeEntryDetails = json_decode($client->getLastResponseBody());

Keep in mind that Client::requestPost() and Client::getLastResponseBody() will become deprecated in future releases. This will be done in an BC way.

LPRKR commented 4 months ago

The first option code works perfectly, so I will use it in the script. Thank you for an amazing support! 💯

Art4 commented 4 months ago

You're welcome. I'm glad I could help you.

kbsali commented 4 months ago

@LPRKR I agree with you, @Art4 is amazing at supporting AND maintaining this project!!! :tada: :muscle: :tada: