Skip to content

Conversation

stonebuzz
Copy link
Contributor

@stonebuzz stonebuzz commented Sep 10, 2025

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

  • It fixes !39184

Do not rely on getSingular($container['name'])

The current approach does not correctly convert names ending with double 's'.
For example:

  • 'temps passé''tempspass'<object>tempspas (in DB)

'tempspass' when using getSingular (KO)
'tempspas' when using internal process (OK)

This PR replaces getSingular with an internal process to correctly convert the container name to the PHP class name, using:

  1. Toolbox->getSystemNameFromLabel
  2. PluginFieldsContainer::getClassname

'tempspass' => 'PluginFields<object>tempspas'

Screenshots (if appropriate):

@stonebuzz stonebuzz requested a review from Lainow September 10, 2025 14:22
@stonebuzz stonebuzz self-assigned this Sep 10, 2025
@stonebuzz stonebuzz added the bug label Sep 10, 2025
@stonebuzz
Copy link
Contributor Author

Relying on the label introduces additional issues.

For example, the getSingular function already handles a specific case for plurals ending with ss, which should remain unchanged when the function is called. To address this, I added a check that detects the presence of a double s and removes the trailing s when necessary.

@trasher
Copy link
Contributor

trasher commented Sep 11, 2025

Isn't the real issue temps passé should become tempspasse instead of tempass?

@stonebuzz
Copy link
Contributor Author

Accented characters are systematically removed by the fields plugin (this has been the case for a long time).

I am somewhat hesitant to modify this behavior, especially considering the migration effort that would be required to restore everything correctly.

@stonebuzz stonebuzz force-pushed the fix_classname_resolution branch from 3834121 to 2f0e896 Compare September 11, 2025 09:21
@stonebuzz stonebuzz merged commit 1bd7cd6 into main Sep 11, 2025
3 checks passed
@stonebuzz stonebuzz deleted the fix_classname_resolution branch September 11, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants