kbsali / php-redmine-api

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

Add support for updating attachments #395

Closed Art4 closed 7 months ago

Art4 commented 7 months ago

Closes #320.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.42%. Comparing base (2b4c451) to head (8d265a4).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## v2.x #395 +/- ## ========================================= Coverage 98.41% 98.42% - Complexity 604 606 +2 ========================================= Files 29 29 Lines 1772 1781 +9 ========================================= + Hits 1744 1753 +9 Misses 28 28 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Art4 commented 7 months ago

The endpoint PATCH /attachments/x.json is undocumented so far, but one can see in the source code and tests how the endpoint should work.

https://www.redmine.org/projects/redmine/repository/svn/entry/tags/5.1.2/test/integration/api_test/api_routing_test.rb

  def test_attachments
    should_route 'GET /attachments/1' => 'attachments#show', :id => '1'
    should_route 'PATCH /attachments/1' => 'attachments#update', :id => '1'
    should_route 'DELETE /attachments/1' => 'attachments#destroy', :id => '1'
    should_route 'POST /uploads' => 'attachments#upload'
  end

https://www.redmine.org/projects/redmine/repository/svn/entry/tags/5.1.2/test/integration/api_test/attachments_test.rb

  test "PATCH /attachments/:id.json should update the attachment" do
    patch(
      '/attachments/7.json',
      :params => {:attachment => {:filename => 'renamed.zip', :description => 'updated'}},
      :headers => credentials('jsmith')
    )
    assert_response :no_content
    assert_nil response.media_type
    attachment = Attachment.find(7)
    assert_equal 'renamed.zip', attachment.filename
    assert_equal 'updated', attachment.description
  end

But I have trouble testing this behavior. If I call the endpoint the Redmine server responses with a 200 OK status code and the unchanged body.

Behat test:

    @attachment
    Scenario: Updating the details of an attachment
        Given I have a "NativeCurlClient" client
        And I create a project with name "Test Project" and identifier "test-project"
        And I upload the content of the file "%tests_dir%/Fixtures/testfile_01.txt" with the following data
            | property          | value                |
            | filename          | testfile.txt         |
        When I update the attachment with the id "1" with the following data
            | property          | value                |
            | filename          | testfile2.txt        |
        Then the response has the status code "204"
        And the response has an empty content type
        And the response has the content ""
        And the returned data is true

Test result:

$ sudo docker compose exec php composer behat -- --tags=attachment
> behat --config tests/Behat/behat.yml --format progress '--tags=attachment'
............F---...................................................... 70
.......................F---........................................... 140
......................

--- Failed steps:

001 Scenario: Updating the details of an attachment # features/attachments.feature:30
      Then the response has the status code "204"   # features/attachments.feature:39
        Raw response content: {"attachment":{"id":1,"filename":"testfile.txt","filesize":65,"content_type":"text/plain","description":null,"content_url":"http://redmine-50102:3000/attachments/download/1/testfile.txt","author":{"id":1,"name":"Redmine Admin"},"created_on":"2024-03-18T13:38:43Z"}}
        Failed asserting that 200 is identical to 204.

002 Scenario: Updating the details of an attachment # features/attachments.feature:30
      Then the response has the status code "204"   # features/attachments.feature:39
        Raw response content: {"attachment":{"id":1,"filename":"testfile.txt","filesize":65,"content_type":"text/plain","description":null,"content_url":"http://redmine-50008:3000/attachments/download/1/testfile.txt","author":{"id":1,"name":"Redmine Admin"},"created_on":"2024-03-18T13:38:48Z"}}
        Failed asserting that 200 is identical to 204.

16 scenarios (14 passed, 2 failed)
162 steps (154 passed, 2 failed, 6 skipped)
0m13.65s (13.80Mb)
Script behat --config tests/Behat/behat.yml --format progress handling the behat event returned with error code 1

Update: If I'm using PUT instead of PATCH I can update the attachment successfully. I'm not sure why this works. 🤷

I have working tests for this behavior, so I think we should go with PUT instead of PATCH. I will document this in the code too.