Skip to content

[Fix]: Data values not formatted when coming from fields plugin custom fields - #655

Merged
jdurand-teclib merged 8 commits into
mainfrom
fix/45898/dates-from-fields-not-formatted
Aug 26, 2026
Merged

[Fix]: Data values not formatted when coming from fields plugin custom fields#655
jdurand-teclib merged 8 commits into
mainfrom
fix/45898/dates-from-fields-not-formatted

Conversation

@jdurand-teclib

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • 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 !45898
  • Here is a brief description of what this PR does:
    • Fix an issue where data coming from fields plugin's custom fields are not passed through the formatting process before being injected in database.

No unit test were written because they would have involve references to Fields plugin, thus failing in the GitHub environment, as it is not linked to Fields plugin.

@Rom1-B Rom1-B left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!45898 also reports comma-decimal numbers from Fields custom fields failing to import, not just dates. GLPI's numeric datatype is 'decimal'/'number', never 'float', so this fallback won't trigger reformatFloat() for them. Follow-up planned?

@jdurand-teclib

Copy link
Copy Markdown
Contributor Author

!45898 also reports comma-decimal numbers from Fields custom fields failing to import, not just dates. GLPI's numeric datatype is 'decimal'/'number', never 'float', so this fallback won't trigger reformatFloat() for them. Follow-up planned?

Actually I did not completely reproduce this one. The numbers are correctly imported in my Glpi when I'm visualizing one at-a-time, so I suspect he didn't use a Decimal field when defining his field.
However, I did forget about the display in the list, I'm looking into it.

@jdurand-teclib
jdurand-teclib requested a review from Rom1-B August 25, 2026 07:28
Comment thread inc/commoninjectionlib.class.php Outdated
Comment thread inc/commoninjectionlib.class.php Outdated
@jdurand-teclib
jdurand-teclib requested a review from Rom1-B August 25, 2026 11:23
Comment thread inc/commoninjectionlib.class.php Outdated
jdurand-teclib and others added 2 commits August 25, 2026 13:44
@jdurand-teclib
jdurand-teclib requested a review from Rom1-B August 25, 2026 11:59

@Rom1-B Rom1-B left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, can you add tests cases?

@jdurand-teclib

jdurand-teclib commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

OK, can you add tests cases?

@Rom1-B I'm not sure this is relevant, the changes are covering a case that is strictly correlated with Fields plugin, as the only search options that arrive in here without checktype are the one coming from custom fields from Fields plugin.
Testing this particular aspect would have me setting up a unit test with some elements from Fields plugin, I suppose ?

@jdurand-teclib
jdurand-teclib requested a review from Rom1-B August 25, 2026 14:35
Comment thread tests/unit/CommonInjectionLibFloatDetectionTest.php
Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com>
@jdurand-teclib
jdurand-teclib requested a review from Rom1-B August 26, 2026 07:10
@jdurand-teclib
jdurand-teclib merged commit 6c9233b into main Aug 26, 2026
3 checks passed
@jdurand-teclib
jdurand-teclib deleted the fix/45898/dates-from-fields-not-formatted branch August 26, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants