Skip to content

Conversation

@ownede
Copy link

@ownede ownede commented Jul 7, 2016

Merged my, @ubermuda, @bassrock and @DenisMedved code together.

As mentioned in #275 basic features seems to be working, no more deprecated code.

Creating a new thread, replying, views and forms (outbox/inbox/deleted/new message/reply) seems to work great. Need some more time for testing.

Currently testing this on my project.
Thanks for anyone who contributed.

DenysMedvid and others added 23 commits February 26, 2016 16:50
… into ubermuda-symfony3

Conflicts:
	Controller/MessageController.php
	FormFactory/AbstractMessageFormFactory.php
	FormHandler/AbstractMessageFormHandler.php
	FormType/NewThreadMessageFormType.php
	FormType/RecipientsType.php
	Resources/config/form.xml
	Resources/config/spam_detection.xml
	Search/QueryFactory.php
	Security/ParticipantProvider.php
… into ubermuda-symfony3

Conflicts:
	Controller/MessageController.php
	FormFactory/AbstractMessageFormFactory.php
	FormHandler/AbstractMessageFormHandler.php
	FormType/NewThreadMessageFormType.php
	FormType/RecipientsType.php
	Resources/config/form.xml
	Resources/config/spam_detection.xml
	Search/QueryFactory.php
	Security/ParticipantProvider.php
@tgalopin
Copy link
Contributor

tgalopin commented Jul 7, 2016

That's an awesome work, I'll check this tonight! Thanks!

@ownede ownede mentioned this pull request Jul 7, 2016
@ubermuda
Copy link

Amazing work! Thanks for your efforts.

@Filoz
Copy link
Contributor

Filoz commented Jul 22, 2016

Thanks for your work!

Will this be merge?

->children()
->scalarNode('factory')->defaultValue('fos_message.new_thread_form.factory.default')->cannotBeEmpty()->end()
->scalarNode('type')->defaultValue('fos_message.new_thread_form.type.default')->cannotBeEmpty()->end()
->scalarNode('class')->defaultValue('FOS\MessageBundle\FormType\NewThreadMessageFormType')->cannotBeEmpty()->end()
Copy link
Member

Choose a reason for hiding this comment

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

this is clearly a bc break, we need a bc layer to keep supporting the old option

public function __construct(RequestStack $requestStack, ComposerInterface $composer, SenderInterface $sender, ParticipantProviderInterface $participantProvider)
{
$this->request = $request;
$this->request = $requestStack->getCurrentRequest();
Copy link
Member

@GuilhemN GuilhemN Jul 22, 2016

Choose a reason for hiding this comment

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

The request must not be retrieved in the constructor.
You can do something like:

/**
 * @param Request|RequestStack $request
 */
public function __construct($request, ...) {
    if ($request instanceof Request) {
         @trigger_error(sprintf('Using an instance of "%s" as first parameter of "%s" is deprecated since version 1.3 and won\'t be supported in 2.0. Use an instance of "Symfony\Component\HttpFoundation\RequestStack" instead.', get_class($request), __METHOD), E_USER_DEPRECATED);
    }
    $this->request = $request;
    // ...
}

private function getCurrentRequest() {
   if ($this->request instanceof Request) {
      return $this->request;
   }

   return $this->request->getCurrentRequest();
}

public function process() {
    // ...
    $request = $this->getCurrentRequest();
}

@GuilhemN
Copy link
Member

BTW it looks like changes to composer.json were lost. (see https://github.com/FriendsOfSymfony/FOSMessageBundle/pull/275/files)

@GuilhemN
Copy link
Member

GuilhemN commented Jul 22, 2016

@ownede are you still interested to work on this PR ?

@ownede
Copy link
Author

ownede commented Jul 22, 2016

@Ener-Getick
Thanks for the review!

Well, this PR contains merges of various authors' forks.

I'm still interested into further development, but it may take time. If somebody is also interested, please fell free to push your changes to my fork.

btw. It seems to work fine with Symfony 3.0 on PHP 5.6, if anyone wants to use it before public release 😜

@GuilhemN
Copy link
Member

GuilhemN commented Jul 22, 2016

Take all the time you need :)
I don't use this bundle so i can't dedicate it too much time but i can review the pr of whoever wants to work on that.

Thanks for contributing to the open source community !

@tgalopin
Copy link
Contributor

tgalopin commented Sep 7, 2016

This should be fixed by #289, I close but if you have problems, don't hesitate to open an issue :) !

@tgalopin tgalopin closed this Sep 7, 2016
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.

8 participants