2

In my view I have a dropdown of people within an organisation that a ticket can be assigned to.

<label for="assigned_support_id">Assigned To</label>
<%= select( "ticket", "assigned_support_id", Person.find_admin_and_support.collect {|p| ["#{p.name}", p.id]}, {:include_blank => "&lt; Unassigned &gt;"} ) %>

Because the blank inserted above has no value, if that option is selected then the ticket will remain assigned to the same person.

I've considered creating an 'Unassigned' person in the database but that will probably be quite complicated having that 'person' belong to every organisation....

I also considered using JavaScript to manipulate the DOM and give it a value but I wanted to do this the 'Rails Way' so I've come up with the following:

ticket.rb

before_save :check_unassigned

...

private

def check_unassigned
  if self.assigned_support_id.nil? or defined?(self.assigned_support_id)
    self.assigned_support_id = 0
  end
end

The nil check didn't seem to do anything so I added the defined check but the problem with that is that every time I reassign a ticket to another user it the table will be updated with 0 so something isn't quite right and I'm not sure what the bug is.

I also considered updating the update method in my controller by checking the [:params][:assigned_support_id] but thought the above was a neater solution.

martincarlin87
  • 10,848
  • 24
  • 98
  • 145
  • you can directly use `self.assigned_support_id ||= 0` (if `self.assigned_support_id` is `nil` it will set it to zero) and you don't need to test the .nil? and defined? – MrYoshiji Aug 09 '13 at 13:50
  • thanks, it almost works, behaves correctly when going from unassigned to an assigned person but when going from assigned to unassigned the db value isn't populated with 0. – martincarlin87 Aug 09 '13 at 13:55
  • Why don't you want to use `nil` in your database? – ctilley79 Aug 09 '13 at 14:00
  • it could be `nil`, just thought `0` would be 'nicer' but `nil` or `0` isn't too important, it's just trying to figure out why the logic behind it isn't working. – martincarlin87 Aug 09 '13 at 14:02
  • Sounds good. I think you're trying to solve a problem that would still exist. Avoiding nils is good if you try to access attributes for an object that is nil. It sucks getting that error. You would still run into this problem if a Person with id of 0 does not exist either. – ctilley79 Aug 09 '13 at 14:06
  • 2
    It is worst to use `0` as a foreign key than to use `nil`. Why? Because every objects with `assigned_support_id = 0` will be considered as having a AssignedSupport associated, and when you will to do `@ticket.assigned_support` for example, it will raise and Error instead of returning `nil`. Don't use `0` as default value for foreign keys – MrYoshiji Aug 09 '13 at 14:13
  • ok, thanks, so any ideas on how to update the database to null if the unassigned blank is selected? – martincarlin87 Aug 09 '13 at 14:14
  • You could do it the brute force way and update the database with a select statement. Or you could fix your code, and update every record manually using your edit form – ctilley79 Aug 09 '13 at 14:16
  • all the CRUD is already in place and working, the only 'bug' is when trying to select the appended blank. Technically not a bug since it's the correct behaviour but you know what I mean :) – martincarlin87 Aug 09 '13 at 14:17

2 Answers2

1

If salf.assigned_support_id exists then it's true under 'defined?'...

so any code where you have

if (some_irrelevant_evaluation) or defined?(self.assigned_support_id) 

is ALWAYS going to evaluate to true.

SteveTurczyn
  • 36,057
  • 6
  • 41
  • 53
0

I would do:

  if self.assigned_support_id.nil? or self.assigned_support_id.blank?

It's possible that it is passing a blank string if you select the blank option and are passing it back in a form.

Try reworking your form into something like this:

<%= form_for @organization do |f| %>
    <%= f.label :assigned_support_id %>
    <%= f.collection_select(:assigned_support_id, Person.find_admin_and_support, :id, :name, {include_blank: "&lt; Unassigned &gt;"}) %>
<% end %>
Drew
  • 2,601
  • 6
  • 42
  • 65
  • thanks, just tried it but when selecting the blank option the id in the table remains the same as before. – martincarlin87 Aug 09 '13 at 14:27
  • You would still need to handle the insertion logic for what you are trying to do. But this would pinpoint if the blank option is selected. – Drew Aug 09 '13 at 14:29
  • yeah, I have this `def check_unassigned if self.assigned_support_id.nil? or self.assigned_support_id.blank? self.assigned_support_id = 0 end end`. Using 0 just now to test if it works. – martincarlin87 Aug 09 '13 at 14:30
  • Can I ask why you are not using form_for if you are updating the record on the model? – Drew Aug 09 '13 at 14:44
  • Please see my edits, I am not sure how you are passing the object to the model to be saved, but this is how I would handle it to ensure the 'before_save' has the proper value. – Drew Aug 09 '13 at 14:54