Topic: Help with some nasty code

Like the title says, prepare yourself for some gross code.  What I'm trying to do is create a message that can be sent to multiple users of the system, but I am having some problems. 

How it should work:
The user clicks a link to send a message and is presented with a form that allows him to enter the title and body of the message. Along with that are three list boxes: groups, system users, and the list of users to send the message to. The idea is that the user can select multiple users and add them to the list that will receive the message and also remove them from the list.

The problem:
Everything works almost correctly except for one piece. Because I use multiple forms, the title and body of the message are only set if the 'send message' button is clicked. Therefore, if the user clicks the button to move users between lists, the title and body are lost. Other than this, I'm sure there is a more elegant way to accomplish everything and I'm just not seeing it.

I'm not sure if multiple form_for tags are appropriate, but since each contains model information, I thought this was the way to go.

The code:
The models are set up as HABTM between messages and users.


def add
  @groups = @current_user.groups
  @new_message = session[:message] ||
  @message_group = @new_message.users
  @users = User.find(:all, :order => 'last_name DESC') - @message_group
    if params["add"]
      @new_message.title = params[:new_message][:title]
      @new_message.message = params[:new_message][:message]
      @new_message.sent_by =
        redirect_to(:controller => 'login', :action => 'index')
      if params["add_users"] and not params[:user][:id].nil?
        @new_message.users << User.find(params[:user][:id])
      elsif params["remove_users"] and not params[:message_user][:id].nil?
      elsif params["add_group"]
        @group = Group.find(params[:group][:id])
        @new_message.users << unique(@new_message.users, User.find(@group.users))
      session[:message] = @new_message
      redirect_to(:controller => 'message', :action => 'add')

<%= error_messages_for 'message' %>

<h3>Create New Message</h3>

<div id="message-form">
    <% form_for :message, :url => { :action => "add" } do |f| %>   
    <label for="title">Title:</label>
        <%= f.text_field :title, :size => 40 %>
        <label for="message">Message:</label>
        <%= f.text_area :message, :rows => 10 %>
        <%= submit_tag "Send Message", :name => "add", :class => "submit" %>
    <% end %>
<div id="group-form-outline">
    <div id="message-groups" align="left">
        <% form_for :group, :url => { :action => "add" } do |g| %>
            <%= g.collection_select(:id, @groups, :id, :name, {}) %>
            &nbsp;<%= submit_tag "Add Group", :name => "add_group", :class => "submit" %>
        <% end %>
    <div id="users">       
        <% form_for :user, :url => { :action => "add" } do |u| %>
            <%= u.collection_select(:id, @users, :id, :username, {}, :class => "user-list", :size => 10, :multiple => true) %>
    <div id="list-buttons">
            <br><%= submit_tag ">>", :name => "add_users", :class => "submit" %>
        <% end %>
        <% form_for :message_user, :url => { :action => "add" } do |m| %>
            <br><br><%= submit_tag "<<", :name => "remove_users", :class => "submit" %>
    <div id="group-users">
        <h2>Send To:</h2>
            <%= m.collection_select(:id, @message_group, :id, :username, {}, :class => "user-list", :size => 10, :multiple => true) %>
        <% end %>           

Like I said above, I am sure there are many places where I could improve this code, so please critique at will.

If anything needs to be clarified, let me know.

Thanks for any help you can give.

Re: Help with some nasty code

I'd change the adding and removing of users to be done via javascript instead of updating your model and refreshing the screen.

You'd then have one form for teh whole page and Send Message would update the message and supply the send_to list.

Here's sample links and the underlying function to get you started:

<a href="javascript:moveSelectedOptions(document.form_name.all_users,

<a href="javascript:moveSelectedOptions(document.form_name.send_to,

function moveSelectedOptions(from,to){
    if (!hasOptions(from))
    for (var i=0; i<from.options.length; i++) {    
        var o = from.options[i];
        if (o.selected) {
            if (!hasOptions(to)){
                var index = 0;
            } else {
                var index=to.options.length;
            to.options[index] = new Option( o.text, o.value, false, false);            }
    // Delete them from original
    for (var i=(from.options.length-1); i>=0; i--) {
        var o = from.options[i];
        if (o.selected){
            from.options[i] = null;

    from.selectedIndex = -1;
    to.selectedIndex = -1;