Skip to content

Merge the SqlRequestSettings endpoints into one domain resource - #415

Open
PrestaEdit wants to merge 4 commits into
PrestaShop:devfrom
PrestaEdit:domain/sql-request-settings
Open

PrestaEdit wants to merge 4 commits into
PrestaShop:devfrom
PrestaEdit:domain/sql-request-settings

Conversation

@PrestaEdit

Copy link
Copy Markdown
Contributor
Questions Answers
Branch? dev
Description? Consolidates the two pending SqlRequestSettings Admin API PRs into a single domain resource, with a read/write symmetric endpoint and API-only integration tests
Type? new feature
BC breaks? no
Deprecations? no
Fixed ticket? Related to PrestaShop/PrestaShop#39630
How to test? See below
Sponsor company PrestaEdit

What this PR does

Merges #280 (PUT /sql-request-settings) and #335 (GET /sql-request-settings) into one PR, then reworks the resource and the tests as a whole, following the mutualisation done on the Product domain in #410.

Endpoints

Method URI CQRS Scope
GET /sql-request-settings GetSqlRequestSettings sql_management_read
PUT /sql-request-settings SaveSqlRequestSettingsCommand + GetSqlRequestSettings sql_management_write

Design decisions

  • One resource class: SqlRequestSettings replaces the SqlRequestSettings / SqlRequestSettingsRead pair from the two source PRs. Both declared the exact same two properties.
  • Read/write symmetry: the PUT no longer answers an empty 204. It declares CQRSQuery: GetSqlRequestSettings::class, so the command result is replayed through the GET query and the endpoint returns the updated settings with a 200.
  • Singleton resource: no identifier, read: false on the update since there is nothing to provide before the command runs.
  • The SqlRequestSettingsConstraintException → 422 mapping is kept: unlike some other settings commands, SaveSqlRequestSettingsCommand really does validate its input, and the test covers it.

Tests

SqlRequestSettingsEndpointTest replaces SqlRequestSettingsEndpointTest + SqlRequestSettingsReadEndpointTest. It covers GET, PUT, the GET of the updated settings and the rejection of an unsupported encoding, chained with @depends:

  • every fixture comes from the API itself — the previous write test asserted through \Configuration::get(), which is gone;
  • assertions compare the complete structure, so a missing or extra field fails the test;
  • the encoding is stored as an int in the configuration (utf-8 → 1) and mapped back to its string value on read, so asserting the PUT response against the GET is what actually pins that round-trip;
  • the rejected update is followed by a GET asserting nothing changed.

How to test

GET  /sql-request-settings  (scope sql_management_read)  -> 200 {fileEncoding, fileSeparator}
PUT  /sql-request-settings  (scope sql_management_write) -> 200, the same payload, updated
GET  /sql-request-settings                               -> 200, identical to the PUT response
PUT  /sql-request-settings  with fileEncoding=utf-32     -> 422

Covered by SqlRequestSettingsEndpointTest.

Supersedes

Both will be closed once the CI is green here.

PrestaEdit and others added 4 commits August 20, 2026 10:26
PUT /sql-request-settings (SaveSqlRequestSettingsCommand) sets the SQL manager export file encoding
(utf-8 / iso-8859-1) and separator. A singleton resource. The integration test updates the settings
and verifies them via Configuration.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The handler maps the charset to an int (utf-8 => 1, iso-8859-1 => 2), so assert the integer value.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Expose GetSqlRequestSettings as GET /sql-request-settings (scope sql_management_read):
return the file encoding and separator used to export SQL request results.

Related to PrestaShop/PrestaShop#39630

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidates PrestaShop#280 (PUT /sql-request-settings) and PrestaShop#335 (GET /sql-request-settings)
into a single SqlRequestSettings domain resource:

- one resource class instead of SqlRequestSettings + SqlRequestSettingsRead, since
  both declared the same two properties
- the PUT now replays GetSqlRequestSettings and returns the updated settings (200)
  instead of an empty 204, so read and write share the same URI and mapping
- a single integration test covering GET, PUT, the GET of the updated settings and
  the rejection of an unsupported encoding, asserting the full JSON structure and
  building every fixture through the API alone (no Configuration:: reads, no SQL)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

2 participants