Skip to content
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

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

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

BibiSebi
Copy link
Contributor

@BibiSebi BibiSebi commented Feb 20, 2024

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

Copy link

vercel bot commented Feb 20, 2024

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
Copy link
Contributor

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

@BibiSebi
Copy link
Contributor Author

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
Copy link
Contributor

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 🤔

const sandboxOrigin: string = 'https://plugin-sandbox.storyblok.com'

Let me know if you need help!

@BibiSebi
Copy link
Contributor Author

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

@BibiSebi
Copy link
Contributor Author

BibiSebi commented Feb 22, 2024

@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
Copy link
Contributor

@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
Copy link
Contributor Author

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.

@BibiSebi BibiSebi merged commit a706b89 into main Feb 22, 2024
12 checks passed
@BibiSebi BibiSebi deleted the EXT-2238-update-field-plugin branch February 22, 2024 10:45
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