-
Notifications
You must be signed in to change notification settings - Fork 4
chore: add exception message for invalid path
value
#68
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
base: master
Are you sure you want to change the base?
chore: add exception message for invalid path
value
#68
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #68 +/- ##
=========================================
Coverage 93.88% 93.88%
- Complexity 461 462 +1
=========================================
Files 72 72
Lines 1620 1620
=========================================
Hits 1521 1521
Misses 99 99 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me this looks legit. I would suggest to change the commit prefix from chore to fix because this will likely be released as a patch.
Yes and please add proper exception messages to all the other Asserts too. |
if (null !== $values['path']) { | ||
$path = TrimmedNonEmptyString::fromString($values['path'])->toString(); | ||
if (null !== $values['path'] && '' !== $values['path']) { | ||
$path = TrimmedNonEmptyString::fromString($values['path'], 'The value of key "path" is invalid.')->toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KristianH i am not so happy with this. Its too generic. Maybe something like The value of key "path" must be an trimmed non empty string
TL;DR
Note
I encountered a strange bug related to a Story in Storyblok. This PR might help identify the root cause.
Long Version
In the Storyblok API responses (both Management API and Content Delivery API), the
path
value was returned as an empty string (""
) instead ofnull
.Running the following command resulted in an error:Which produced:
As a result, many links were missing in the
LinkRegistry
, which is part of thestoryblok/symfony-bundle
.How to Fix This in Storyblok
Review the Content Delivery API response from Retrieve Multiple Links, for example:
Look for entries where
"path": ""
(the expected default is"path": null
).Delete and recreate the affected stories in the Storyblok editor.Final Note
I'm not sure whether this bug is easily reproducible in Storyblok — and I didn’t want to risk reproducing it in a production environment.
For context: the affected story had its
first_published_at
value changed, and multi-language support was enabled.