storyblok / field-plugin

Create and deploy Storyblok Field Plugin
https://www.storyblok.com/docs/plugins/field-plugins/introduction
25 stars 3 forks source link

fix(lib): add translatable as part of the plugin payload #360

Closed BibiSebi closed 4 months ago

BibiSebi commented 4 months ago

What?

Library

Added new property to the plugin.data object called translatable. This is a field setting that can be adjusted by the content editor and came in as a feature request from a customer.

Sandbox

In order to make the local development easier, sandbox was extended with a checkbox 'Translatable' where the user can manipulate the new property accordingly.

Documentation

You can review the Documentation updated here.

Open Tasks

Why?

JIRA: EXT-2225

vercel[bot] commented 4 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plugin-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 22, 2024 9:40am
eunjae-lee commented 4 months ago

@BibiSebi looks good but I see some tests are failing here, and also in #361 (probably rooted from here)

BibiSebi commented 4 months ago

I need to check it, however I haven't changed anything that would affect the templates, maybe it has something to do with the origin feature where messaging only works for two specific domains?

eunjae-lee commented 4 months ago

I need to check it, however I haven't changed anything that would affect the templates, maybe it has something to do with the origin feature where messaging only works for two specific domains?

Our test helper is using the sandbox origin, though 🤔

https://github.com/storyblok/field-plugin/blob/d576fe405b854c0f0401d1531cbd4bd13cd514e4/packages/field-plugin/helpers/test/src/index.ts#L12

Let me know if you need help!

BibiSebi commented 4 months ago

You are absolutely right @eunjae-lee. I think I have found the cause of this :) looking at it now

BibiSebi commented 4 months ago

@eunjae-lee I made an addition to @storyblok/field-plugin/test.

Fixing this issue, made me think that maybe some validations are missing in regards to releases, currentReleaseId and interfaceLang because inside PR #345 the tests passed without any changes to @storyblok/field-plugin/test. What do you think?

eunjae-lee commented 4 months ago

@eunjae-lee I made an addition to @storyblok/field-plugin/test.

Fixing this issue, made me think that maybe some validations are missing in regards to releases, currentReleaseId and interfaceLang because inside PR #345 the tests passed without any changes to @storyblok/field-plugin/test. What do you think?

Ahhh... okay, so Storyfront already has been sending translatable in the payload. We made the change on the SDK side, but actually it was not feature-parity with the container implementation of the test helper.

Oh then, this leads to the fact that we still have two different implementation of container (the test helper and the sandbox), and when we extract the implementation and reuse it, we could write proper tests for better validation.

In the meantime, do you have any idea how to improve this at the current stage? (but feel free to skip if not)

BibiSebi commented 4 months ago

I agree with you @eunjae-lee it would be much clearer if the container and tests shared this functionality, I will create a ticket for it (if we do not already have one). I think there is not much more we can do at this point, just to keep it in the back of our minds when reviewing such changes.