Asset versioning feature disabled results in a 403 error

Created on 18 July 2024, 4 months ago

Problem/Motivation

There is a 403 error when the Asset versioning feature is disabled. It works properly when the Asset versioning feature is enabled at WIDEN DAM

Steps to reproduce

  1. Disable Asset versioning feature
  2. Try browse media asset at content type

Expectation

The expected result is that user would be able to access the asset even the the Acquia DAM Asset Versioning feature disabled.

🐛 Bug report
Status

Active

Version

1.0

Component

Code

Created by

🇮🇳India vipin.mittal18 Greater Noida

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @vipin.mittal18
  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 1035s
    #227731
  • Pipeline finished with Failed
    4 months ago
    Total: 1073s
    #227736
  • Pipeline finished with Failed
    4 months ago
    Total: 1120s
    #227793
  • Pipeline finished with Failed
    4 months ago
    Total: 1457s
    #228646
  • Pipeline finished with Failed
    4 months ago
    Total: 1077s
    #230785
  • Pipeline finished with Failed
    4 months ago
    Total: 1094s
    #230788
  • Pipeline finished with Failed
    4 months ago
    Total: 1036s
    #230792
  • Pipeline finished with Failed
    4 months ago
    Total: 1013s
    #230797
  • Pipeline finished with Failed
    4 months ago
    Total: 996s
    #230798
  • Status changed to Needs work 4 months ago
  • Pipeline finished with Failed
    4 months ago
    Total: 1204s
    #231336
  • Pipeline finished with Failed
    4 months ago
    Total: 1070s
    #231362
  • Pipeline finished with Failed
    4 months ago
    Total: 1134s
    #231365
  • Status changed to Needs review 4 months ago
  • Pipeline finished with Failed
    4 months ago
    Total: 1252s
    #232012
  • Pipeline finished with Failed
    4 months ago
    Total: 1199s
    #232022
  • Status changed to Postponed 4 months ago
  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺

    By briefly running through the suggested changes, I foresee the potential of a code conflict. The self-explanatorily incorrect assumption of the inline comment

    “// We can assume, that the latest version is the last item of the array.”

    has been overridden once already in 🐛 Look for the "finalized" asset version not the latest modified Needs review .

    Not counting this one, there are already 5–6 other open tickets already in Needs Review status. Most of them means a significant amount of code change. To avoid double-work I would suggest let's focus on getting them merged first, then get return here with a fresher 1.1.x branch.

    Therefore I dare to mark this as Postponed for now but feel free to switch back if you feel it's urgent to work on.

  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺

    Now MR tested, I confirm that it solves the 403 error.

    I also agree with renaming our DAM client's getAssetVersionList() method to a more generic getAssetDetails() which will better describe its functioning.

    Still I suggest to let's wait until 🐛 Look for the "finalized" asset version not the latest modified Needs review lands, then it will be easier to integrate these changes.

  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺




    Some thoughts on the Widen API v1/v2 usage

    The Client\AcquiaDamClient::getAssetVersionList() in the current 1.0.x dev HEAD calls Widen API like this:

    $response = $this->get("https://$domain/api/rest/asset/uuid/$id/assetversions"); (see API docs)

    Which is provided by the v1 API. This MR suggests replacing this method of our client with a different one, which would call API v2:

    $response = $this->get("/v2/assets/$id"); (see API docs)

    Generally, I agree with that as it leans towards modernisation. However, now I checked our source code and at some point, we already rely on v2 API endpoints, for example AcquiadamAuthService::sendAuthRequest() (source).

    This mixed usage of Widen API causes some heavy feelings for me. I think we could – after some discussion – put some work into searching through the entire codebase and checking all external HTTP calls (heads up, not only the Client class!) to ensure that the next stable release of this module will rely on exclusively on Widen API v2.

  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺

    Tests are failing on D9, setting back to NW.

  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺
    • MR updated to fresh 1.0.x target branch
    • The Client class reworked to rely on API v2 for fetching asset versions (points of comment #8 🐛 Asset versioning feature disabled results in a 403 error Postponed are still valid)
    • API response caching improved
    • Test adapted to the code changes

    Ready for review.

  • Pipeline finished with Skipped
    about 2 months ago
    #297488
  • First commit to issue fork.
  • 🇺🇸United States japerry KVUO
  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺
  • 🇭🇺Hungary Balu Ertl Budapest 🇪🇺
  • Status changed to Fixed about 2 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024