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 getMediaModel with nested flexible Layout #267

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

trippo
Copy link

@trippo trippo commented Mar 16, 2021

Instanceof Layout without import return always false and getMediaModel must be public.

Instanceof Layout without import return always false and getMediaModel must be public.
@trippo trippo mentioned this pull request Mar 16, 2021
@toonvandenbos
Copy link
Member

Hi @trippo,

Is this PR finished or are you still tweaking it ?

Thanks !

@trippo
Copy link
Author

trippo commented May 27, 2021

The last 4 commits are for support validation on translatable fields inside flexible

@trippo
Copy link
Author

trippo commented May 27, 2021

But I don't know if there are others methods... Practically in getParsedFlexibleGroup function, convert valid JSON with locales attributes to array and this fixes the validation.
If there is a JSON field with a locale attribute like { "en": "bla bla"} this will generate a mistake but it is an unlikely condition.

@trippo
Copy link
Author

trippo commented Aug 10, 2021

Can you accept this pull request?

Copy link
Member

@toonvandenbos toonvandenbos left a comment

Choose a reason for hiding this comment

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

Hi @trippo,

Thanks for your work. I can accept this PR but would like some minor code style changes first.

In Http/ParsesFlexibleAttributes L 102, Instead of having else { if (is_string($value)) { /* ... */ } }, please prefer a more readable syntax using elseif (is_string($value)) { /* ... */ }.

Also, don't forget to add docblocks for your new methods (isTranslatableAttribute for instance).

Thanks !

@trippo trippo requested a review from toonvandenbos August 18, 2021 15:25
@trippo
Copy link
Author

trippo commented Aug 24, 2021

@Nyratas please see my changes

@pitylee
Copy link

pitylee commented Feb 23, 2022

Any update on this?

@Fabstilelook
Copy link

Nope but I'm using @trippo version long time on the production

@pitylee
Copy link

pitylee commented Feb 23, 2022

It is pretty sad that despite the collaboration from the community they have so many PR stacked.

Is his version on packagist too?

Not packagist, but in composer.json can do like so than install:

    "repositories": {
        ...
        "trippo/nova-flexible-content": {
            "type": "package",
            "package": {
                "name": "trippo/nova-flexible-content",
                "version": "1.0",
                "source": {
                    "url": "https://github.com/trippo/nova-flexible-content.git",
                    "type": "git",
                    "reference": "origin/patch-1"
                },
                "autoload": {
                    "psr-4": {
                        "Whitecube\\NovaFlexibleContent\\": "src/"
                    }
                },
                "extra": {
                    "laravel": {
                        "providers": [
                            "Whitecube\\NovaFlexibleContent\\FieldServiceProvider"
                        ]
                    }
                }
            }
        }

@voidgraphics
Copy link
Member

Please see #326

We would love to get things moving with this but need someone to help us review these PRs.

@pitylee
Copy link

pitylee commented Feb 23, 2022

I used the actual code fore translatable rule fix, and is working like a charm.
At least the validation part, but rhe error showing part on the frontend is not because translatable will have layout key only on frontend.

The media part don't know.

@pitylee
Copy link

pitylee commented Feb 24, 2022

Somehow the translatable field will not get the unique key in the payload.

This way, it is trying to find with files.0.attributes.transcript.en in the rules, but that doesn't exist, if I add the key in the scoped field rules it will not be able to check, but shows error correctly.

I am wondering how could I add the jPsvC4afdJP8SRv0 to the transcript attribute in the payload?

Payload:
files: [{"layout":"audio","key":"jPsvC4afdJP8SRv0","attributes":{"transcript":"{\"en\":\"\",\"fr\":\"fr\"}","jPsvC4afdJP8SRv0__id":"117","jPsvC4afdJP8SRv0__filetype":"audio","jPsvC4afdJP8SRv0__ComputedField":"29582"}}]

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.

5 participants