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

Unable to modify or remove field in group #82

Closed
voyou-sbeaulieu opened this issue May 23, 2019 · 17 comments
Closed

Unable to modify or remove field in group #82

voyou-sbeaulieu opened this issue May 23, 2019 · 17 comments

Comments

@voyou-sbeaulieu
Copy link

We are unable to Remove a field create inside a group. We always get an error :
Field 'logo' not found.

Here the current used code :

<?php

namespace App;

use StoutLogic\AcfBuilder\FieldsBuilder;

$theme_params_footer = new FieldsBuilder('theme_params_footer');

$config = (object) [
	'ui' => 1,
	'wrapper' => ['width' => 50],
];

$group_footer = $theme_params_footer->addGroup('footer',['label' => 'Pied de page']);
$group_footer_sections = $theme_params_footer->addGroup('footer_sections',['label' => 'Sections pied de page']);

$group_footer
			->addImage('logo',['label' => 'Logo','preview_size' => 'full'])
			->addFields(get_field_partial('components.bg_image'))
;

$group_footer_sections			
			->addFields(get_field_partial('partials.builder_footer'))
;

$group_footer->removeField('logo');
	
return $theme_params_footer;

?>

Even when we do

$group_footer
			->addImage('logo',['label' => 'Logo','preview_size' => 'full'])
			->addFields(get_field_partial('components.bg_image'))
->removeField('logo')
;

We get the same error. Is this impossible to modify or remove a field from a group?

Thank for your time!

@stevep
Copy link
Member

stevep commented May 23, 2019

@voyou-sbeaulieu It does indeed look like the GroupBuilder class is currently missing that function, good catch.

It currently delegates to the parent scope because the function doesn't exist, and there is no logo field in that scope. It would be wrong the wrong field anyways even if there was.

    /**
     * Remove a field by name
     * @param  string $name Field to remove
     * @return $this
     */
    public function removeField($name)
    {
        $this->fieldsBuilder->removeField($name);

        return $this;
    }

Should be added to the GroupBuilder class

@voyou-sbeaulieu
Copy link
Author

Sweet, thx so if we add this function it's should work as expected right?
Do you think this would be added in a patch soon so the new project gets this directly from composer?

Thank for the fix and your time!

@voyou-sbeaulieu
Copy link
Author

Hmm I just added the function to /vendor/stoutlogic/acf-builder/src/GroupBuilder.php
But I have the same error... Even if I use it directly in the group like so :

$theme_params_footer = new FieldsBuilder('theme_params_footer');

$group_footer = $theme_params_footer->addGroup('footer',['label' => 'Pied de page']);
$group_footer
			->addImage('logo',['label' => 'Logo','preview_size' => 'full'])
			->addFields(get_field_partial('components.bg_image'))
->removeField('logo')
;

is there something I misunderstood?

@stevep
Copy link
Member

stevep commented May 23, 2019

Sorry I've edited the code above,

$this->getFieldManager()->removeField($name);

should be

$this->fieldsBuilder->removeField($name);

@voyou-sbeaulieu
Copy link
Author

Working as expected!

Thank you!

@voyou-sbeaulieu
Copy link
Author

Do you think this fix would be added soon in the stoutlogic/acf-builder composer version?

@stevep
Copy link
Member

stevep commented May 23, 2019

Maybe within a week or so. I need to add to the unit tests and make sure nothing unexpected happens.

@voyou-sbeaulieu
Copy link
Author

Is modifyfield the same issues tho?

removeField is working now but not the modifyfield.

@stevep
Copy link
Member

stevep commented May 23, 2019

Yes most likely

@voyou-sbeaulieu
Copy link
Author

We have added the function from FieldsBuilder.php and made some change getFieldManager() to fieldsBuilder but we still get a error No such function: replaceField

Here the full function we added to GroupBuilder.php

/**
     * Modify an already defined field
     * @param  string $name   Name of the field
     * @param  array|\Closure  $modify Array of field configs or a closure that accepts
     * a FieldsBuilder and returns a FieldsBuilder.
     * @throws ModifyFieldReturnTypeException if $modify is a closure and doesn't
     * return a FieldsBuilder.
     * @throws FieldNotFoundException if the field name doesn't exist.
     * @return $this
     */
    public function modifyField($name, $modify)
    {
        if (is_array($modify)) {
            $this->fieldsBuilder->modifyField($name, $modify);
        } elseif ($modify instanceof \Closure) {
            $field = $this->fieldsBuilder->getField($name);

            // Initialize Modifying FieldsBuilder
            $modifyBuilder = new FieldsBuilder('');
            $modifyBuilder->addFields([$field]);

            /**
             * @var FieldsBuilder
             */
            $modifyBuilder = $modify($modifyBuilder);

            // Check if a FieldsBuilder is returned
            if (!$modifyBuilder instanceof FieldsBuilder) {
                throw new ModifyFieldReturnTypeException(gettype($modifyBuilder));
            }

            // Insert field(s)
            $this->fieldsBuilder->replaceField($name, $modifyBuilder->getFields());
        }

        return $this;
    }

If we change the $this->fieldsBuilder->replaceField back to $this->getFieldManager()->replaceField we get the same error field 'logo' not found. Hope you could sort this out.

Thank for your time

@voyou-sbeaulieu
Copy link
Author

Just to add the first part

if (is_array($modify)) {
            $this->fieldsBuilder->modifyField($name, $modify);
        }

is working as exepted and we can modify the field label using `$group_footer->modifyField('logo',['label' => 'Logo Modify']

@voyou-sbeaulieu
Copy link
Author

Okie I think I over tought about it... as the first condition was working fine with the good scope, I tried to just use this fonction directly and it's worked wonderful.. So With this code in GroupBuilder.php :

/**
     * Modify an already defined field
     * @param  string $name   Name of the field
     * @param  array|\Closure  $modify Array of field configs or a closure that accepts
     * a FieldsBuilder and returns a FieldsBuilder.
     * @throws ModifyFieldReturnTypeException if $modify is a closure and doesn't
     * return a FieldsBuilder.
     * @throws FieldNotFoundException if the field name doesn't exist.
     * @return $this
     */
    public function modifyField($name, $modify)
    {
       
            $this->fieldsBuilder->modifyField($name, $modify);

        return $this;
    }

Modify is working as expected with both conditions inside `FieldsBuilder.php

Do you think this should also be added in your next build after some tests?

@voyou-sbeaulieu
Copy link
Author

Can you confirm if this is OK as a fix to use modifyField on a group field and if this could be implemented in a new build?

We are currently updating and creating projects using this group logic we want to be sure we won't be working against your updates.

Thank for your time.

@stevep
Copy link
Member

stevep commented May 28, 2019

@voyou-sbeaulieu Yes we'll get this added at the same time. 99% sure unless something goes weird when writing the unit tests.

voyou-sbeaulieu added a commit to voyou-sbeaulieu/acf-builder that referenced this issue Jul 10, 2019
Those functions were missing from the Group scope. Liked to issue StoutLogic#82
@voyou-sbeaulieu
Copy link
Author

Any update on this ?

Thanks for your time!

@capellopablo
Copy link

Any update on this ?

stevep added a commit that referenced this issue Sep 16, 2021
stevep added a commit that referenced this issue Sep 17, 2021
…esting. #142 #91 #141 #82

If modifying a Layout (FieldsBuilder), pass updated array config to FieldsBuilder::updateGroupConfig
This change also calls removeField and modifyField recursively along the nesting path, so the correct modify / remove action can be taken if running into a Flexible Content field
@stevep
Copy link
Member

stevep commented Sep 17, 2021

This has been released in version 1.12.0

@stevep stevep closed this as completed Sep 17, 2021
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

No branches or pull requests

3 participants