[Bug] Include key ID and exception message in classification store field definition warning#468
Open
jcPimcore wants to merge 4 commits into
Open
[Bug] Include key ID and exception message in classification store field definition warning#468jcPimcore wants to merge 4 commits into
jcPimcore wants to merge 4 commits into
Conversation
…eld definition warning The warning logged when a classification store key's field definition could not be resolved omitted the key ID and swallowed the original exception message, making it impossible to identify which key was affected or why.
Contributor
There was a problem hiding this comment.
Pull request overview
Improves classification-store diagnostics when field definitions cannot be resolved.
Changes:
- Adds the affected key ID to warnings.
- Preserves the caught exception message.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…warning Covers the diagnostic behavior introduced in 8d8c257: when getFieldDefinitionFromKeyConfig() throws, the logged warning must include the key type, key ID, group ID, and the original exception message.
Substring assertions were weak: 'input' also appears inside the exception text, and bare numeric checks for 42/9 didn't verify they were logged as the key ID and group ID specifically. Asserting the full expected message ensures a regression that drops or mislabels any field fails the test. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Matches the style already used by the other warning in this class (getNormalizedValue) and avoids spacing/concatenation mistakes if the message is changed again. Co-Authored-By: Claude Sonnet 5 <[email protected]>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: pimcore/platform-version#190
Summary
ClassificationStoreAdapter::getFieldDefinitionForKey) omitted the key ID and swallowed the original exception message, making the warning impossible to act on.generic-data-index:update:index -rproduced many warnings likeCould not get field definition for type input in group 9with no way to identify which key was affected or why.Test plan
tests/Unit/SearchIndexAdapter/DataObject/FieldDefinitionAdapter/ClassificationStoreAdapterTest.phppass