COmanage Developers List

[Benn Oshrin <>]

Looks good. I'd accept your pull request if we were using git ;)

On 2/17/12 6:14 PM, Scott Koranda wrote:
> > On 2/17/12 5:30 PM, Scott Koranda wrote:
> >
> > > As discussed on the call today I have added an add() method to
> > > the CoPeopleController class that overrides the add() method
> > > in the parent StandardController class.
> > >
> > > A diff is attached.
> > >
> > > I have tested this patch and it correctly updates the
> > > CoPeople and CoOrgIdentityLink tables.
> > >
> > > Benn can you please look at the diff and let me know what if
> > > anything you want changed or if I have overlooked some side
> > > effect?
> >
> > Can we simplify this to only update ['*_id']['required']?
>
> Yes.
>
> New diff attached. Please comment.
>
> > I'm
> > thinking of a scenario where we change the 'rule' or some other
> > aspect of validation in the model and don't remember to change it
> > here.
> >
> > Also, on the same note, it'd be good to add a comment near $validate
> > in the model mentioning that it may be overridden by the controller
> > in add().
>
> Done, included in diff.
>
> > Nitpicking, there's an extra newline after the call to parent::add().
>
> Fixed.
>
> Thanks,
>
> Scott