DRY and engines


#1

Hello,

I’m using the UserEngine in my Rails application, in casu Riki. In the
page to create a new book, the administrator can select the book
administrator for that book, or create a new one if it does not exists.
In that case I link_to /user/new, but in the normal case I get
redirected to /user/list, whereas I would like to be redirected back to
the /book/create page. I solved this in the following way:

  1. I added at the top of my create.rhtml (or in the create action)
    session[‘return-to’] = ‘/books/create’

  2. And I changed the new action in the user controller to

           if session['return-to']
             redirect_to session['return-to']
           else
             redirect_to :action => 'list'
           end
    

In that case I get redirected back to the create book page. This is one
way, but I have the feeling that it is not the most elegant. I could of
course copy the complete new action, but this goes against the DRY
principle, and against the reusability of Engines. In the end, I might
have copied all code inside my rails application. This follows a
discussion I had with Jay:

Another thing to think about: You want the engine to be easily
extensible without folks having to modify the actual engine files, or
cut-and-paste >huge swaths of code. So keeping subroutines short (and
thus keeping overrides granular) is even more important than it is in
“normal” code. >UserEngine does a good job of keeping the “core” code
under lib, but I still found I had to copy an entire action from
user_controller just to change >one string. I’m not sure I can think of
specific patterns to encourage for extensibility, but it’s something
that should be kept in mind.

Or there other ways to solve this, I mean, reusing engine code
efficiently?

By the way, I still have then another problem in this way. When the
administrator has already filled in some fields of the form, they get
lost when he gets back to the create page. I know this is probably more
a Rails question in general, but does anybody has some ideas on how to
keep the field data? My gutfeeling is that it is lost, unless I
integrate it all together, by integrating the new user partial into the
create form, and duplicating the user/new code.

How could this be rewritten, so the new action of the user controller
could be reused? Or is this not the purpose of Engines, and maybe more
of plug-ins, where Engines are meant to be more complete applications?

enjoy!

Bart


#2

Both the User and Login engines could do with serious refactoring with
a view to making them much cleaner in terms of overriding behaviour. A
big part of this is probably going to be establishing a clear API for
both. Let the discussion commence!

  • james

On 2/28/06, Bart M. removed_email_address@domain.invalid wrote:

session['return-to'] = '/books/create'

course copy the complete new action, but this goes against the DRY
user_controller just to change >one string. I’m not sure I can think of
integrate it all together, by integrating the new user partial into the
Bart


engine-users mailing list
removed_email_address@domain.invalid
http://lists.rails-engines.org/listinfo.cgi/engine-users-rails-engines.org

  • J *
    ~

#3

Hi,

Ok, I will try a bit more how far I can go with this
session[‘return-to’] idea, which is already used nicely in the
LoginEngine, and see how it can be usefull for this process.

And I had to reread Chapter 8 of the pragmatic programmer book to
realize that sessions will also solve my other, non-engine related
problem…

Bart


#4

Hello,

I found a solution for my problem, and even better then I thought. It
should be general enough for other uses. It works in two directions:

  1. you can say where the user/new has to return to, after completion (in
    my case, back to the book creation)

  2. In my book/create, I get immediately the id of the newly created
    user, so I can use it in my book/create form. The administrator does not
    have to select it again.

For doing this I changed the following lines in the user_controller of
the UserEngine:

redirect_to :action => ‘list’

changed to

          if session['return-to']
            session['return-to'][:new_user_id] = @user.id
            redirect_to( session['return-to'] )
          else
            redirect_to :action => 'list'
          end

My book/create action, looks like this:

def create
case request.method
when :get
@book = partially_created_book
@all_users_mapped = User.find_all.map { |c| ["#{c.fullname}",
c.id] }
if !params[:new_user_id].nil? # retrieve the newly created
user.
@admin = User.find(params[:new_user_id])
end
when :post
if params[:commit]==“Create new user”
session[:partially_created_book] = Book.new(params[:book])
session[‘return-to’] = {:controller => ‘books’, :action =>
‘create’} # to set the action where to return to
redirect_to :controller => ‘user’, :action => ‘new’
elsif params[:commit]==“Create”
Book.create(params[:book])
redirect_to ‘/’ + params[:book][:url_name]
end
end
end

It works, and seems general enough. For the full code, you’ll have to
wait for Riki 0.3.1, coming soon…

Any ideas to improve this?

Bart


#5

Hi!

In attendance of the new post, I can already say that indeed that was
the first way I did it, but it did not give me back the newly created
user. And that was something which I found quite user-friendly, putting
this newly created information directly in the form of creating the
book.

Just extending the general usage a bit…

bart


#6

Any ideas to improve this?

Bart
Hi Bart!

I was writing an answer while you have added yours.
I can still publish it as a general suggestion. Here it is:

"…
I don’t have a final solution there, but rather a small suggestion:

avoid the if/else construct and just set the
session[‘return-to’] ||= ‘/your_controller/list’
as default.

In this way you just
redirect_to session[‘return-to’]
whenever you need it. It scales better, doesn’t
matter how many new spacial cases you want to introduce.
…"

About the DRYness I create a new POST so that we don’t confuse the Riki
specific issue to the genear problem.

See you in the new post :wink:

Mauro