Skip to content

[fix] name mapping in request data classes #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 28, 2025
Merged

[fix] name mapping in request data classes #226

merged 2 commits into from
Jan 28, 2025

Conversation

joelbutcher
Copy link
Collaborator

@joelbutcher joelbutcher commented Jan 28, 2025

This PR adds a snake case name mapper PHP attribute to BaseData.

closes #225

Problem Context

As discovered in #225, the API tests are returning a false positive, because of the Laravel Data configuration used for testbench:

    'name_mapping_strategy' => [
        'input' => \Spatie\LaravelData\Mappers\SnakeCaseMapper::class,
        'output' => \Spatie\LaravelData\Mappers\SnakeCaseMapper::class,
    ],

Setting both of these values to NULL and running the test suite highlights the error. Of course, this is fine in our tests as we are specifying the Laravel app to uses these mappers. However applications consuming this package might not have Laravel Data configured this way.

The fix

Since we have architectural tests in place to ensure that all XyzRequestData classes must be declared final and must extend BaseData, we are able to fix the problem by simply adding the following attribute above the class declaration for BaseData:

#[MapName(SnakeCaseMapper::class)]
abstract class BaseData extends Data
{
   ...
}

@joelbutcher joelbutcher requested a review from jbrooksuk January 28, 2025 09:18
@joelbutcher joelbutcher added this to the v3.0 milestone Jan 28, 2025
@joelbutcher joelbutcher added the bug Something isn't working label Jan 28, 2025
@jbrooksuk jbrooksuk merged commit 10e8622 into cachethq:main Jan 28, 2025
25 checks passed
@sconnole
Copy link
Contributor

thanks for the fix @joelbutcher 👍

jbrooksuk added a commit that referenced this pull request Feb 10, 2025
* main: (103 commits)
  Update PHPStan parameters
  Change schedule running
  Fix dutch translations (#231)
  fix: remove additional prompt for name (#227)
  [fix] name mapping in request data classes (#226)
  fix: define getTitle method on Settings pages to use localised name (#224)
  Set OG images
  Fix code styling
  Demo Mode (#223)
  Fix code styling
  fix: make user command does not accept a name argument (#216)
  Apply ordering of component groups to status page
  Use root language for local dialects
  Compile Assets
  Updated PH localization (#213)
  Bump vite from 5.4.8 to 5.4.14 (#214)
  Fix code styling
  Support the `HTTP_REMOTE_USER` header (#208)
  New adjustments added for German translation and added Dutch (#209)
  Fix code styling
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

when creating a component through the API component_group_id does not get populated in the database table
3 participants