Bacon / BaconUser

BaconUser provides simple user management
BSD 2-Clause "Simplified" License
14 stars 9 forks source link

Remove form from services #19

Closed bakura10 closed 11 years ago

bakura10 commented 11 years ago

ping @ocramius, @mac_nibblet, @Dasprids

As told on IRC, this PR removes the form from the service, so that it is decouples and allow to use BaconUser in REST context.

Now, the user is responsible to fetch the form in the controller, validate the data, and pass the correct user to the service.

coveralls commented 11 years ago

Coverage Status

Coverage decreased (-0%) when pulling c209c8665f570d0d3674bcab003423137d6b8767 on bakura10:service-entity into e2b475aa37715ca8ed8b29bcda641d3fe9e4051a on Bacon:master.

coveralls commented 11 years ago

Coverage Status

Coverage decreased (-0%) when pulling c209c8665f570d0d3674bcab003423137d6b8767 on bakura10:service-entity into e2b475aa37715ca8ed8b29bcda641d3fe9e4051a on Bacon:master.

Ocramius commented 11 years ago

@bakura10 I like :)

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling f471cfbecb4c6911e387590b76c73bf439306d0f on bakura10:service-entity into e2b475aa37715ca8ed8b29bcda641d3fe9e4051a on Bacon:master.

coveralls commented 11 years ago

Coverage Status

Coverage decreased (-0%) when pulling ea0f264a9f2145f74a811b76d777503f2640223b on bakura10:service-entity into e2b475aa37715ca8ed8b29bcda641d3fe9e4051a on Bacon:master.

DASPRiD commented 11 years ago

I'm not going to accept this one; It makes the service layer completely obsolete. What we discussed earlier was exchanging the form against input-filter and hydrator.

bakura10 commented 11 years ago

But it replaced it !

Input filters and hydrators are bound to the form, so people using form will just need to do that in their controllers:

$form = $serviceLocator->get('FormElementManager')->getRegistrationForm();
$form->setData($_POST);

if ($form->isValid) {
   $userService->register($form->getData());
}

As it is today it's not usable with REST. The service is not obsolete at all. It needs to have an event manager too (to send mails). So I suggest reopening the branch =).

Ocramius commented 11 years ago

@bakura10 I think @DASPRiD is right here - instantiating the user object is up to the service layer here. The InputFilter fix seems ok on the other side... Can lead to double validation, but that's no problem either imo :)

DASPRiD commented 11 years ago

This one needs re-evaluation.