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

CED-1124 NGSTACK-831 edit infocollection #89

Open
wants to merge 87 commits into
base: 3.x
Choose a base branch
from

Conversation

Gintron
Copy link

@Gintron Gintron commented Jan 12, 2024

Implemented editing the info collection.

Copy link
Member

@ludwig031 ludwig031 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Tested on project and works as expected.
@pspanja please confirm so we can tag it

@ludwig031
Copy link
Member

Rebased branch to 3.x to get new changes

use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Validator\Constraints;

final class UserUpdateType extends UserType
Copy link
Member

Choose a reason for hiding this comment

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

We won't need this for info collection, please remove it.

Copy link
Author

Choose a reason for hiding this comment

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

solved in 7822313

use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Validator\Constraints;

final class UserCreateType extends UserType
Copy link
Member

Choose a reason for hiding this comment

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

We won't need this for info collection, please remove it.

Copy link
Author

Choose a reason for hiding this comment

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

removed in bfce0ac

*
* @var mixed
*/
public $payload;
Copy link
Member

Choose a reason for hiding this comment

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

Missing type hint (below as well).

Copy link
Author

Choose a reason for hiding this comment

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

solved in 6266154

public $definition;

/**
* The target struct that applies to. E.g. Content, User, Section object and so on.
Copy link
Member

Choose a reason for hiding this comment

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

Adapt descriptions for info collection context.

Copy link
Author

Choose a reason for hiding this comment

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

solved in 4ccfd43


declare(strict_types=1);

namespace Netgen\Bundle\InformationCollectionBundle\Form\Type;
Copy link
Member

Choose a reason for hiding this comment

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

Since we have this namespace how, move other form types here.

Copy link
Author

Choose a reason for hiding this comment

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

solved in 49dc73c

netgen_information_collection.form.fieldtype_handler.ezimage:
class: Netgen\Bundle\InformationCollectionBundle\Form\FieldTypeHandler\Image
tags:
- { name: netgen.ibexa_forms.form.fieldtype_handler, alias: ezimage }
Copy link
Member

Choose a reason for hiding this comment

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

Adapt tag name for this bundle.

Copy link
Author

Choose a reason for hiding this comment

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

solved in f52577d

* @throws \OutOfBoundsException
* @throws \RuntimeException When type is not a FieldTypeHandlerInterface instance nor a callable factory
*/
public function get(string $identifier): FieldTypeHandlerInterface|\Netgen\Bundle\IbexaFormsBundle\Form\FieldTypeHandlerInterface
Copy link
Member

Choose a reason for hiding this comment

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

Second return type hint was for Birthday field type? We should copy it's handler here once the tag names are updated, so the handlers are not in conflict.

Copy link
Author

Choose a reason for hiding this comment

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

solved in e46037b

/**
* Register a $service for FieldType $identifier.
*
* @param \Netgen\Bundle\InformationCollectionBundle\Form\FieldTypeHandlerInterface|callable $handler
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove callable, this was initially implemented for quite old SF version.

Copy link
Author

Choose a reason for hiding this comment

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

solved in 706fed5

*
* @param \Netgen\Bundle\InformationCollectionBundle\Form\FieldTypeHandlerInterface|callable $handler
*/
public function register(string $identifier, $handler): void
Copy link
Member

Choose a reason for hiding this comment

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

Typehit FieldTypeHandlerInterface.

Copy link
Author

Choose a reason for hiding this comment

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

solved in d2abeb6

}
if (!$this->map[$identifier] instanceof FieldTypeHandlerInterface && !$this->map[$identifier] instanceof \Netgen\Bundle\IbexaFormsBundle\Form\FieldTypeHandlerInterface) {

if (!is_callable($this->map[$identifier])) {
Copy link
Member

Choose a reason for hiding this comment

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

Remove handling callable.

Copy link
Author

Choose a reason for hiding this comment

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

sovled in 38530c5

@iherak iherak changed the title Ced 1124 edit infocollection CED-1124 NGSTACK-831 edit infocollection Jul 30, 2024
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.

4 participants