Forum: Rails Engines [Login Engine] Overriding controller and model

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Eduardo R. (Guest)
on 2006-04-27 15:41
(Received via mailing list)
I am struggling overriding the signup method.

I would like not to override the entire method, because I only want to
change the redirect clause. Since Rails does not allow two call to
redirect (DoubleRenderError), the only way I see is to copy n' paste
the method and change the redirect line. Is there anything else I can
do to avoid copy n' paste?

My other doubt is about the model. If I do not use email notification
and do not use a email field, the second user signup is made
impossible, since it complains that 'Email has already been taken'.
That is, the model validates the uniqueness of Email, even if email
notification is disabled. Is that a bug, right?

Even being a bug, it would be nice to be able to override the model, I
mean, there could be a way of not using some of the default
validators.

[]'s
Eduardo
Paul R. (Guest)
on 2006-04-27 17:49
(Received via mailing list)
On 27 Apr 2006, at 12:41, Eduardo R. wrote:

> redirect (DoubleRenderError), the only way I see is to copy n' paste
> the method and change the redirect line. Is there anything else I can
> do to avoid copy n' paste?

Copy and paste is the way you should do it. If you're changing the
behaviour of what happens at the end of the method, don't you want to
make sure that unexpected things don't get checked into /vendor/
plugins and start messing your code up?

Copying and pasting is not only the only way to do it to my
knowledge, but it's the most sensible way to do it, IMHO. You either
want the default engine behaviour, or you want your behaviour, but
you don't want an unpredictable and volatile mix of the two that
could change in nature every time somebody checks a patch in to the
engines' repository.

> My other doubt is about the model. If I do not use email notification
> and do not use a email field, the second user signup is made
> impossible, since it complains that 'Email has already been taken'.
> That is, the model validates the uniqueness of Email, even if email
> notification is disabled. Is that a bug, right?

Hmmm, not really. The model says that emails need to be present and
unique. If you are changing the nature of the model so that they
don't need to be present and unique (by not capturing one at all in
your view), you need to change the model.

Are you saying that you've removed the need for the presence of the
email but the uniqueness check is in there tripping up over multiple
null values? I can see how that might be considered a bug, but if you
think about it, it's behaving exactly how it is documented as an
ActiveRecord method.

> Even being a bug, it would be nice to be able to override the model, I
> mean, there could be a way of not using some of the default
> validators.

You can over-ride the model, it's just not as easy as over-riding a
method. It is documented though. However, it gets a bit more
complicated with user_engine because there is a shedload of code in
lib that needs copying over and then tweaking.

--
Paul R.
Eduardo R. (Guest)
on 2006-04-28 03:01
(Received via mailing list)
2006/4/27, Paul R. <removed_email_address@domain.invalid>:
>
> Copying and pasting is not only the only way to do it to my
> knowledge, but it's the most sensible way to do it, IMHO. You either
> want the default engine behaviour, or you want your behaviour, but
> you don't want an unpredictable and volatile mix of the two that
> could change in nature every time somebody checks a patch in to the
> engines' repository.

I think the best protection is a new test, exercising the change I
made. Doing that, I could update the login_engine and if new code
breaks what I've written, my test would tell me. At the same way, if
login_engine improves some functionality, I would not get it
'automagically'. My main aim was to avoid code duplication, code that
maybe was not necessary. By the way, do you know a trick to just
override the redirect_to?

> your view), you need to change the model.
>
> Are you saying that you've removed the need for the presence of the
> email but the uniqueness check is in there tripping up over multiple
> null values? I can see how that might be considered a bug, but if you
> think about it, it's behaving exactly how it is documented as an
> ActiveRecord method.
>

It seems to be an inconsistent feature of login_engine, ActiveRecord
just plays right: I can disable mail verification (config
:use_email_notification, false) so that's no need for email at all,
but the email still needs to me unique. Here a simple ":if" clause in
validates_uniqueness would do the trick and if you agree I can open a
ticket.

> > Even being a bug, it would be nice to be able to override the model, I
> > mean, there could be a way of not using some of the default
> > validators.
>
> You can over-ride the model, it's just not as easy as over-riding a
> method. It is documented though. However, it gets a bit more
> complicated with user_engine because there is a shedload of code in
> lib that needs copying over and then tweaking.
>

Exactly. Since including AuthenticatedUser gives me all that
validations right away, there is no easy way of override them. Engines
is all about code reuse with easy configuration/customization, and
what I am claiming here is an easier way of doing that in
login_engine.

> --
> Paul R.

Paul, thanks for your fast reply and sorry for me being late :(
Paul R. (Guest)
on 2006-04-28 03:43
(Received via mailing list)
On 27 Apr 2006, at 23:57, Eduardo R. wrote:

> I think the best protection is a new test

But imagine this: you write your code, use svn:externals to grab the
engines, you distribute it, somebody does a svn update, doesn't
bother running the tests but something has changed and....

Yeah, trust me, you don't want to do that. If you need to change the
behaviour of a method just copy the whole method and adapt it.

What would be interesting is if there was a way of creating a new
class that inherits from, say, User, then doing #super without it
throwing out the redirect or any renders, before running your own
code. That could get awkward though. I'm not even sure how you would
go about doing it.

> 'automagically'. My main aim was to avoid code duplication, code that
> maybe was not necessary. By the way, do you know a trick to just
> override the redirect_to?

'Avoid Code Duplication' is a guide to writing efficient code, not a
fascist mantra that should be observed when there is a good reason to
duplicate. :-)

Remember, you're not duplicating yourself - you're replacing somebody
else's code with a more suitable version, and you're only copying the
bits you absolutely have to in order to remain syntactically correct
within the language you're working with.

No, I know of no way to over-ride just the redirect_to.

I promise you, that whilst I truly admire your technique and
resistance to doing things the wrong way, in my opinion you are doing
NOTHING wrong by copying just the methods you want to change and
adapting them, and you are doing it the CORRECT and AGILE and RAILS
way. You are not duplicating yourself. You are changing somebody
else's code. You are doing the right thing. Stop feeling guilty! :-)

If you want to change the way this works in the future, think about
how you could adapt the engines so that it would be easier to do what
it is you want. There are various ways I can think of, but whilst
you're over-riding existing code, this is The Correct Way. :-)

> It seems to be an inconsistent feature of login_engine, ActiveRecord
> just plays right: I can disable mail verification (config
> :use_email_notification, false) so that's no need for email at all,
> but the email still needs to me unique. Here a simple ":if" clause in
> validates_uniqueness would do the trick and if you agree I can open a
> ticket.

Well, you don't need my agreement - I'm just this guy on a mailing
list who uses user_engine and login_engine a lot. :-)

I think you're right though. You know what would be better than just
opening a ticket? Try coding it yourself - it's a pretty easy fix -
and then submit a patch. You'll feel all important if you do, I
promise. ;-)

> Exactly. Since including AuthenticatedUser gives me all that
> validations right away, there is no easy way of override them. Engines
> is all about code reuse with easy configuration/customization, and
> what I am claiming here is an easier way of doing that in
> login_engine.

Yeah, you're probably right, that needs another think about it. I've
recently had to take on a project where the role a user is actually
assigned depending on both the user and the project - i.e. a user has
one role for one project, another role for another project. I did
find a work-around eventually, and I'll try and create a new engine
based on it, but what was annoying was that I basically had to copy
the entirety of vendor/plugins/user_engine/lib/* to my own /lib and
tweak it just to add half a dozen well-chosen lines.

That said, at least the code was pre-written for me and I didn't have
to start from scratch. :-)

--
Paul R.
Eduardo R. (Guest)
on 2006-04-28 04:41
(Received via mailing list)
2006/4/27, Paul R. <removed_email_address@domain.invalid>:
>
> 'Avoid Code Duplication' is a guide to writing efficient code, not a
> I promise you, that whilst I truly admire your technique and
>
I bet you once felt as guilty as me and just learned let it go! :)
Thanks for conving me on this one, I really needed some support here :)

> I think you're right though. You know what would be better than just
> opening a ticket? Try coding it yourself - it's a pretty easy fix -
> and then submit a patch. You'll feel all important if you do, I
> promise. ;-)
>

I will try.

> find a work-around eventually, and I'll try and create a new engine
> based on it, but what was annoying was that I basically had to copy
> the entirety of vendor/plugins/user_engine/lib/* to my own /lib and
> tweak it just to add half a dozen well-chosen lines.
>

Nice to know. I was kind of struggling with DRY DRY DRY all over my
head, and honestly I am looking forward being more pragmatic and know
better how to balance these concerns. I feel Rails has this kind of
culture, DHH once wrote a controversial post about 'reuse is
overrated' and I am still getting to grips with thoughts like that.
Surely this email will help me, thanks a lot!
Paul R. (Guest)
on 2006-04-28 12:42
(Received via mailing list)
On 28 Apr 2006, at 01:39, Eduardo R. wrote:

> I bet you once felt as guilty as me and just learned let it go! :)
> Thanks for conving me on this one, I really needed some support
> here :)

Yup, of course.

You know what though, this could be seen as a bug. The problem is,
how do you fix it. If you think there is a better way to do it, go
for it.

> Nice to know. I was kind of struggling with DRY DRY DRY all over my
> head, and honestly I am looking forward being more pragmatic and know
> better how to balance these concerns. I feel Rails has this kind of
> culture, DHH once wrote a controversial post about 'reuse is
> overrated' and I am still getting to grips with thoughts like that.
> Surely this email will help me, thanks a lot!

There are no absolutes in this game, just pretty good guesses. DRY is
a really excellent way of producing good code and in the long run
more adaptable and flexible code.

The one thing I hate about some of the way things have developed in
the last year is how what DHH says is seen as 100% correct for all
situations, and some of the things he says just aren't that concrete.
In fact if you read the article you're referring to he says "usually
that means it'll take more time to reuse something than it'll take to
create it from scratch", which is correct. People miss the "usually"
at the start though and read it as "it'll take more time..." and get
in to a state of coding paralysis.

In another part he says: "When you try to [sic] hard to reuse, you'll
often end up with a frankenstein of slightly different approaches and
philosophies that creates rough, unpolished surfaces that simply
can't make you happy because it can't be beautiful."

OK, but reuse in this particular context does not create a rough and
unpolished surface that isn't beautiful. It's beautiful because:

a) It's a well-documented case
b) It's an elegant way of doing it
c) It's best of both worlds - you get all that functionality that
you're not copying and pasting and still getting a really customised
version of the engines you need
d) You're not going to distribute this code to anybody other than as
a finished product, or maybe as an engine itself

Look at how user_engine is built on top of login_engine, and you will
see why reuse, over-riding this way is jaw-droppingly beautiful.

And if you really aren't into re-use, technically you shouldn't be
using scaffolds. Or generators of any sort. Or Rails. Here's a good
counter-argument to the article you read: http://www.nshb.net/reuse-
is-definately-not-overrated

So, think about what you're doing: you're not repeating yourself in
the obtuse copy'n'paste because you're too lazy to work out how to
inherit, kind of way, you're doing something which is a perfectly
valid way of re-defining a method to do something different. This is
the Ruby way. Doing it this way is the Ruby way, not the C way where
you'd end up creating a copy of the whole controller just to change
one line - you're copying just one method and moulding it around what
you need, letting the other methods live back in /vendor/plugins

If you can work out a 100% non-repeating way of getting the same
effect, cool, let's see it! I bet it'd be damn ugly though...

And stop drinking the DHH Kool Aid. :-)

Good luck with your project!

--
Paul R.
This topic is locked and can not be replied to.