Allow deleting new embedded form without save the parent form #182

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

mmonguilod commented Jan 22, 2013

When you embed a relation in a form, if you add a new object in the form, you cannot delete it until you save the form and delete the item. This is very annoying, because if you click by mistale on Add you need to create the object, save and delete afterwards.

There is a new option for embedRelation name 'add_delete_empty' to add a delete checkbox that allow to delete the newly created form. It defaults to false in order to not change the actual behavior.
There is new options also to change the behavior of the delete checkbox:

  • delete_alert_text : corresponds to alert_text
  • delete_hide_parent : corresponds to hide_parent
  • delete_parent_level : corresponds to parent_level

The new checkbox appears in the same place as the delete checkbox for regular objects:

new_checkbox

rsichny commented Jan 22, 2013

I'm sure the better way will be adding the option to to sfFormPropelCollection, not to sfFormPropel (this will allow to use Collection form with that option too):

https://github.com/warsoul/sfPropelORMPlugin/commit/0931bfb52b0ab11a5b7a1084a1beb37144debe4d
https://github.com/warsoul/sfPropelORMPlugin/commit/476055f3eb0bd3465e9736c64d39b623b5a869ea

Owner

willdurand commented Jan 24, 2013

@mmonguilod what do you think? Is @Warsoul's patch good for you?

Contributor

mmonguilod commented Jan 25, 2013

@Warsoul @willdurand I agree with warsoul. It think it will allow to use not only on embedRelation but also in mergeRelation.
One question: I'm new to github (I only use it for Propel, I'm a svn man at work ;-)), It's necessary I made an additional PR with the changes or you can merge directly the changes of warsoul and mines together?

Owner

willdurand commented Jan 25, 2013

I can merge both.

William Durand | http://www.williamdurand.fr

On Fri, Jan 25, 2013 at 9:50 AM, Manel Monguilod
notifications@github.comwrote:

@Warsoul https://github.com/warsoul @willdurandhttps://github.com/willdurandI agree with warsoul. It think it will allow to use not only on
embedRelation but also in mergeRelation.
One question: I'm new to github (I only use it for Propel, I'm a svn man
at work ;-)), It's necessary I made an additional PR with the changes or
you can merge directly the changes of warsoul and mines together?


Reply to this email directly or view it on GitHubhttps://github.com/propelorm/sfPropelORMPlugin/pull/182#issuecomment-12692559.

rsichny commented Jan 25, 2013

@willdurand Don't do that, mine is not the patch, but just another working solution (mmonguilod's PR is not needed if you will merge my code - actually only the options are named in different way, i can change this if his naming fits better) :)

Owner

willdurand commented Jan 25, 2013

Yep I know. I need you guys to work together.

William Durand | http://www.williamdurand.fr

On Fri, Jan 25, 2013 at 1:48 PM, warsoul notifications@github.com wrote:

@willdurand https://github.com/willdurand Don't do that, mine is not
the patch, but just another working solution (mmonguilod's PR is not needed
if you will merge my code - actually only the options are named in
different way, i can change this if his naming fits better) :)


Reply to this email directly or view it on GitHubhttps://github.com/propelorm/sfPropelORMPlugin/pull/182#issuecomment-12699466.

Owner

willdurand commented Apr 8, 2013

ping @rozwell

Contributor

rozwell commented May 8, 2013

I'll look into this soon.

Contributor

rozwell commented May 13, 2013

I've encountered similar issue in one project and got it fixed there and used "remove" button.
There was also some issue with embedded forms keys (when embedded objects were already added).

I'll review the code and provide unified solution.

rsichny commented May 13, 2013

There was also some issue with embedded forms keys (when embedded objects were already added).

it's fixed in my last pr

Contributor

rozwell commented May 13, 2013

@rsichny in which PR exactly?

rsichny commented May 13, 2013

@rozwell #196 , lines 155-164 in sfFormPropel.class.php (custrics/sfPropelORMPlugin@115d648#L0L155)

Contributor

rozwell commented May 13, 2013

Let's move discussion there.

Contributor

mmonguilod commented Jun 22, 2013

@rozwell @rsichny sorry for the delay in the response, but I've been really busy.

What can I do to help? The code of rsichny is merged? I can move the options to sfPropelFormCollection to have a generic sollution.

Owner

willdurand commented Sep 1, 2013

no news for 2 months, closing

@willdurand willdurand closed this Sep 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment