Moreover, can't we move out explicit save in the controller to model like this:
## model.rb def mark_deleted self.update_attribute(:status, "deleted") end Thanks, Abhinav -- अभिनव http://twitter.com/abhinav On Sun, Sep 6, 2009 at 12:56 PM, Colin Law <clan...@googlemail.com> wrote: > > 2009/9/5 brianp <brian.o.pea...@gmail.com>: > > > > So after what i end up with is a: > > > > // word.rb, definition.rb > > def set_status(status) > > self.status = status > > end > > Arguably set_status is not a good name, as it requires the caller to > know about the status field in the model. Something mark_deleted > might be a better name. > > Colin > > > > > // word_controller.rb > > def destroy > > @word = Word.find(params[:id], :include => :definitions) > > > > begin > > ActiveRecord::Base.transaction do > > @word.set_status("deleted") > > @word.save > > > > @word.definitions.each do |h| > > h.set_status("deleted") > > h.save > > end > > end > > rescue ActiveRecord::RecordInvalid => invalid > > flash[:notice] = 'Word was not deleted' > > render :action => "new" > > end > > > > respond_to do |format| > > format.html { redirect_to(words_url) } > > end > > end > > > > Which seems much more appropriate. > > > > > > On Sep 5, 1:07 pm, brianp <brian.o.pea...@gmail.com> wrote: > >> Your are very right. I shouldn't actually have the controller changing > >> the status at all. I should maybe have the controller call the models > >> change status method when called but that is it. And because the > >> method will now be in the model it wont be in the controller and I > >> wont be breaking dry like I thought of before. > >> > >> I should have thought of that earlier once i realized destroy was > >> doing a little more work then actually destroying. > >> > >> Thanks for the input, I knew I was looking at it from the wrong angle > >> it just felt wrong. > >> > >> On Sep 5, 12:25 pm, pharrington <xenogene...@gmail.com> wrote: > >> > >> > On Sep 5, 3:07 pm, brianp <brian.o.pea...@gmail.com> wrote: > >> > >> > > Hey, > >> > > I was just wondering what the best practice for this situation would > >> > > be. I've got two models(word, definition) both with destroy methods > in > >> > > the controllers that just change the model.status to "deleted". In > the > >> > > words controller I'd like it to call the definition_controller > destroy > >> > > method on definition models. And I'm just blanking on how to do this > >> > > like a regular model method. Would I have to double the destory > method > >> > > in the model? Then I wouldn't be adhering to dry. > >> > >> > > // words_controller.rb > >> > > def destroy > >> > > begin > >> > > ActiveRecord::Base.transaction do > >> > > @word = Word.find(params[:id], :include => :definitions) > >> > > @word.status = 'deleted' > >> > > @word.save > >> > >> > > @word.definitions.each do |h| > >> > > h.destroy // Actually destorys the record instead of just > >> > > setting the status to "deleted" > >> > > end > >> > > end > >> > > rescue ActiveRecord::RecordInvalid => invalid > >> > > flash[:notice] = 'Word was not deleted' > >> > > render :action => "new" > >> > > end > >> > >> > > respond_to do |format| > >> > > format.html { redirect_to(words_url) } > >> > > end > >> > > end > >> > >> > > // definitions_controller.rb > >> > > def destroy > >> > > begin > >> > > ActiveRecord::Base.transaction do > >> > > @definition = Definition.find(params[:id]) > >> > > @definition.status = 'deleted' > >> > > @definition.save > >> > > end > >> > > rescue ActiveRecord::RecordInvalid => invalid > >> > > flash[:notice] = 'Definition was not deleted' > >> > > render :controller => :words, :action => "new" > >> > > end > >> > >> > > respond_to do |format| > >> > > format.html { redirect_to(words_url) } > >> > > end > >> > > end > >> > >> > > Thanks, > >> > > bp > >> > >> > I'm not sure how creating a method that sets the status attribute to > >> > "deleted" and saves the record would conflict with DRY principles? > >> > Also, it seems that the specifics of this what-would-be "mark_deleted" > >> > or maybe "set_status('deleted')" method is something the controller > >> > *probably* doesn't care about; more the controller just wants your > >> > model to perform a specific unit of logic. > >> > >> > Moreover, it doesn't make sense (as far as I can see) to call a > >> > separate controller's actions outside the realm of the HTTP redirect > >> > dance. Controllers exist to sit in between your user's request and the > >> > view/content that they want rendered back, having the appropriate > >> > models perform whatever actual logic is required to make this happen. > > > > > > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk@googlegroups.com To unsubscribe from this group, send email to rubyonrails-talk+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
