Permalink
Browse files

Worked on the backend and fixed a few other issues with the admin_con…

…troller
  • Loading branch information...
1 parent 1dcf630 commit f931b13a9c0a22216368f5f34cc717b514d86bc5 @cpursley committed Nov 25, 2012
@@ -0,0 +1,3 @@
+# Place all the behaviors and hooks related to the matching controller here.
+# All this logic will automatically be available in application.js.
+# You can use CoffeeScript in this file: http://jashkenas.github.com/coffee-script/
@@ -0,0 +1,3 @@
+// Place all the styles related to the admin controller here.
+// They will automatically be included in application.css.
+// You can use Sass (SCSS) here: http://sass-lang.com/
@@ -0,0 +1,118 @@
+class AdminController < ApplicationController
+ before_filter :authenticate_user!
+ before_filter :is_admin
+
+def index
+ @num_state0 = Dream.where(:state => '0').count
+ @num_state1 = Dream.where(:state => '1').count
+ @num_state2 = Dream.where(:state => '2').count
+ @num_state3 = Dream.where(:state => '3').count
+ @num_state4 = Dream.where(:state => '4').count
+ @num_published = @num_state3 + @num_state4
+
+ @num_users = User.all.count
+ @num_users_active30days = User.where('last_sign_in_at > ?', 30.days.ago).count
+ @num_users_created30days = User.where('created_at > ?', 30.days.ago).count
@hosh
hosh Dec 17, 2012

This is where an array or a hash will work better. For example:

@stats = { state_1: Dream.where(:state => '0').count,
                  state_2: Dream.where(:state => '1').count ... }

@stats[:state_1] # get the count for that state.

Thing is, I have no idea what state '1' or '2' is. Do you? It would be more clear if it were written with named labels:

@stats = { featured: Dream.where(:state => 0).count,
                  rejected: Dream.where(:state => 0).count,
                 # ...
                }

# <%= link_to pluralize(@stats[:featured], "featured dream"), dreams_admin_index_path + '?state=4' %>,

The thing is, this would be even more clearer if you dropped it inside the Dream model instead of trying to compute it here.

class Dream
  scope :featured, where(state: '0')
  scope :rejected, where(state: '1')
  # ...
  scope :published, where("state = '3' or state = '4'")

  def self.stats
    { featured:  Dream.featured.count,
      standard: Dream.featured.count, 
      # ...
    }
end

def index
  @stats = Dream.stats
end

This principle is called "fat models, thin controllers". It's more about organizing this in a way where the code is easier to work with. If you put these kinds of computation in the model, you can use them somewhere else. They are also easier to test. If you put them in the controller, you have to pry out @num_state0 out somehow ... (by finding the instance of the controller in the request, but that means that you can't compute it unless you are getting the request; these computations don't have anything to do with the request).

There are more efficient ways to get the stats out, but that's for later. The biggest thing to improve your code is to apply the principle of "fat models, thin controllers" as broadly as you can.

@cpursley
cpursley Dec 23, 2012 owner

Hosh,

I'm a bit confused about some of this although I understand the overall "fat models, thin controllers" concept.

How do I push up my open branch (to github) without merging it with master and messing everything up?

+end
+
+def dreams
+ if params[:state]
+ @state = params[:state]
+ # invalid parameter? show submitted dreams
+ if !['0', '1', '2', '3', '4'].index(@state)
+ @state = '1'
+ end
+ else
+ # no parameter? show submitted dreams
+ @state = '1'
+ end
+
+ # different sort order for different states; verbose the state for the view
+ case @state
+ when '0' then @state_name = 'draft'; @order = 'updated_at desc'
+ when '1' then @state_name = 'submitted'; @order = 'updated_at desc'
+ when '2' then @state_name = 'rejected'; @order = 'updated_at desc'
+ when '3' then @state_name = 'accepted'; @order = 'accepted desc'
+ when '4' then @state_name = 'featured'; @order = 'accepted desc'
+ end
+
+ @dreams = Dream.where(:state => @state).order(@order)
+end
+
+# accept an dream as normal or featured dream
+def accept
+ @dream = Dream.find(params[:id])
+
+ # only submitted dreams can be accepted
+ if @dream.state == 1
@hosh
hosh Dec 17, 2012

This is where you start looking at state machine plugins.

+ @dream.state = 3
+ flash[:notice] = 'The dream has been accepted.'
+
+ if params[:value]
+ if params[:value] == '1'
+ @dream.state = 4
+ flash[:notice] = 'The dream has been accepted as a featured dream.'
+ end
+ end
@hosh
hosh Dec 17, 2012

You don't actually need the if params[:value] part, because

  if params[:value] == '1'

Will only be true if params[:value] has something in it.

+
+ # freeeeezing!
+ @dream.freezebody = @dream.title + "\n\n" + @dream.teaser + "\n\n" + @dream.body + "\n\n" + @dream.version + "\n\n" + @dream.changelog
+ @dream.accepted = Time.now
+
+ # save dream
+ if !@dream.save
+ flash[:notice] = 'There was an error while accepting the dream.'
+ end
+ else
+ flash[:notice] = 'Only submitted dreams can be published.'
+ end
+
+ redirect_to :action => 'dreams', :state => 1
+end
+
+# display form to enter reject message
+def editreject
+ @dream = Dream.find(params[:id])
+ # only submitted dreams can be rejected
+ if @dream.state != 1
+ flash[:notice] = 'Only submitted dreams can be rejected.'
+ redirect_to :action => 'dreams', :state => 1
+ end
+end
+
+# reject the dream (updates the dream)
+def reject
+ @dream = Dream.find(params[:id])
+
+ if @dream.state == 1
+ if params[:dream][:message]
+ @dream.state = 2
+ @dream.message = params[:dream][:message]
+ @dream.freezebody = @dream.title + "\n\n" + @dream.teaser + "\n\n" + @dream.body + "\n\n" + @dream.version + "\n\n" + @dream.changelog
+
+ if @dream.save
+ flash[:notice] = "The artile was rejected."
+ redirect_to :action => 'dreams', :state => 1
+ else
+ render :action => "editreject"
+ end
+ else
+ flash[:notice] = "No reject without reject message."
+ redirect_to :action => 'dreams', :state => 1
+ end
+ else
+ flash[:notice] = "Only submitted dreams can be rejected."
+ redirect_to :action => 'dreams', :state => 1
+ end
+end
+
+ protected
+ def is_admin
+ if current_user
+ if current_user.id == 1
+ return 1
+ end
+ end
+ redirect_to root_url
@hosh
hosh Dec 17, 2012

You can simplify this to

def redirect_unless_admin
  redirect_to root_url unless current_user.try(:id) == 1
end

If current_user is nil, then it will return a nil, which is not a 1, so that will redirect to root_url
If current_user is not nil, then it will call id on it, and check to see if it is a 1.

Now, using the 'fat models, thin controllers' we can change this code by adding:

class User
  def admin?
    self.id == 1
  end
end

def redirect_unless_admin
  redirect_to root_url unless current_user.try(:admin?)
end

Does that look more readable to you?

+ end
+end
@@ -8,6 +8,15 @@ class ApplicationController < ActionController::Base
def about
end
+def all
+ @dreams = Dream.where(:state => ['3', '4'])
+
+ respond_to do |format|
+ format.html { render 'index' }
+ format.json { render :json => @dreams }
+ end
+end
+
protected
def count_dreams
@num_private = Dream.where(:state => '0').count.to_s
@@ -5,16 +5,25 @@ class DreamsController < ApplicationController
# GET /dreams
# GET /dreams.json
def index
- @dreams = Dream.where(:state => '1').search(params[:search]).order('accepted desc').paginate(:page => params[:page], :per_page => 10) # published view
+ @dreams = Dream.where(:state => '1').search(params[:search]).order('accepted desc').paginate(:page => params[:page], :per_page => 10)
respond_to do |format|
format.html # index.html.erb
format.json { render json: @dreams }
end
end
+ def all
+ @dreams = Dream.where(:state => ['3', '4']).paginate(:page => params[:page], :per_page => 10)
+
+ respond_to do |format|
+ format.html { render 'index' }
+ format.json { render :json => @dreams }
+ end
+ end
+
def mydreams
- @mydreams = Dream.where(:state => ['0', '1']) # published view
+ @mydreams = current_user.dreams.all
respond_to do |format|
format.html # index.html.erb
@@ -76,28 +85,63 @@ def create
# PUT /dreams/1
# PUT /dreams/1.json
def update
- @dream = current_user.dreams.find(params[:id])
+ @dream = current_user.dreams.find(params[:id])
- respond_to do |format|
- if @dream.update_attributes(params[:dream])
- format.html { redirect_to @dream, notice: 'Dream was successfully updated.' }
- format.json { head :no_content }
- else
- format.html { render action: "edit" }
- format.json { render json: @dream.errors, status: :unprocessable_entity }
- end
+ # if an dream has already been accepted, the user is not allowed to change title and teaser
+ if @dream.state > 2
+ params[:dream].delete(:title)
+ params[:dream].delete(:teaser)
+ end
+
+ respond_to do |format|
+ if @dream.update_attributes(params[:dream])
+ format.html { redirect_to(@dream, :notice => 'Dream was successfully updated.') }
+ format.json { head :ok }
+ else
+ format.html { render :action => "edit" }
+ format.json { render :json => @dream.errors, :status => :unprocessable_entity }
end
end
+end
+
+def submit
+ @dream = current_user.dreams.find(params[:id])
+
+ # submit only, if dream is currently in draft or rejected-state
+ if (@dream.state == 0) or (@dream.state == 2)
+ @dream.state = 1
+ @dream.submitted = Time.now
+
+ if @dream.save
+ flash[:notice] = 'Your dream was successfully submitted for approval.'
+ else
+ flash[:error] = 'There was an error while submitting your dream.'
+ end
+ else
+ flash[:error] = 'This dream can not be submitted.'
+ end
+
+ respond_to do |format|
+ format.html { redirect_to(:action => 'mydreams') }
+ format.json { head :ok }
+ end
+end
# DELETE /dreams/1
# DELETE /dreams/1.json
def destroy
- @dream = current_user.dreams.find(params[:id])
- @dream.destroy
+ @dream = current_user.dreams.find(params[:id])
+
+ # only draft, submitted or rejected dreams can be deleted by the user
+ if (@dream.state < 3)
+ @dream.destroy
+ else
+ flash[:error] = 'The dream could not be deleted.'
+ end
respond_to do |format|
format.html { redirect_to dreams_url }
- format.json { head :no_content }
+ format.json { head :ok }
end
end
@@ -0,0 +1,2 @@
+module AdminHelper
+end
View
@@ -12,7 +12,7 @@ class Dream < ActiveRecord::Base
validates :version, :length => { :maximum => 120 }
validates :changelog, :length => { :maximum => 2000 }
validates :message, :length => { :maximum => 5000 }
- validates :state, :presence => true, :numericality => true, :inclusion => { :in => 0..1 }
+ validates :state, :presence => true, :numericality => true, :inclusion => { :in => 0..4 }
def count_ratings
self.ratings.all.count
@@ -0,0 +1,17 @@
+<section id = "admin">
+<h2>Admin: <%= @state_name.capitalize %> Dreams</h2>
+
+ <% @dreams.each do |dream| %>
+ <p>
+ <h4><%= link_to dream.title, dream %></h4>
+ Author: <%= dream.user.fullname %>
+ <%= link_to 'Delete', dream, :confirm => 'This is admin delete despite of the dreams state. Are you sure?', :method => :delete %>
+
+ <% if dream.state == 1 %>
+ <%= link_to 'Reject', editreject_admin_path(dream) %>
+ <%= link_to 'Publish standard', accept_admin_path(dream, :value => '0'), :method => :put %>
+ <%= link_to 'Publish featured', accept_admin_path(dream, :value => '1'), :method => :put %>
+ <% end %>
+ </p>
+ <% end %>
+</section>
@@ -0,0 +1,28 @@
+<section id = "admin">
+ <h4><%= link_to @dream.title, @dream %></h4>
+ Author: <%= @dream.user.fullname %><br />
+ <%= @dream.teaser %>
+ <br /><br /><br />
+
+ <%= form_for(@dream, :url => reject_admin_path(@dream)) do |f| %>
+ <% if @dream.errors.any? %>
+ <div id="error_explanation">
+ <h2><%= pluralize(@dream.errors.count, "error") %> prohibited this dream from being saved:</h2>
+ <ul>
+ <% @dream.errors.full_messages.each do |msg| %>
+ <li><%= msg %></li>
+ <% end %>
+ </ul>
+ </div>
+ <% end %>
+
+ <div class="field">
+ <label for="dream_message">Reject message</label><br />
+ <%= f.text_area :message, :size => "100%x10", :class => "fullwidth" %>
+ </div>
+
+ <div class="actions">
+ <%= f.submit "Reject Dream" %>
+ </div>
+ <% end %>
+</section>
@@ -0,0 +1,24 @@
+<section id = "admin">
+ <h2>Admin: Dashboard</h2>
+
+ <h3>Dreams</h3>
+ <p>
+ Statistics:
+ <%=pluralize(@num_published, "total published dreams")%>,
+ <%= link_to pluralize(@num_state4, "featured dream"), dreams_admin_index_path + '?state=4' %>,
+ <%= link_to pluralize(@num_state3, "standard dream"), dreams_admin_index_path + '?state=3' %>,
+ <%= link_to pluralize(@num_state2, "rejected dream"), dreams_admin_index_path + '?state=2' %>,
+ <%= link_to pluralize(@num_state1, "submitted dream"), dreams_admin_index_path + '?state=1' %>,
+ <%= link_to pluralize(@num_state0, "private dream"), dreams_admin_index_path + '?state=0' %>
+ <br />
+ <% if @num_state1 > 0 %>
+ <b>There are <%= link_to pluralize(@num_state1, "submitted dream") , dreams_admin_index_path + '?state=1' %> waiting for approval!</b>
+ <% end %>
+ </p>
+
+ <h3>Users</h3>
+ <p>
+ Statistics: <%=pluralize(@num_users, "user") %> total, <%= pluralize(@num_users_active30days, "user") %> have been signed-in within the last 30 days.<br />
+ New users: <%= pluralize(@num_users_created30days, "user") %> have been created within the last 30 days.
+ </p>
+</section>
@@ -11,7 +11,7 @@
</div>
<% end %>
- <% if @dream.state < 1 %>
+ <% if @dream.state < 3 %>
<div class="field">
<label for="dream_title">Title</label><br />
<%= f.text_field :title, :size => "100%", :class => "fullwidth" %>
Oops, something went wrong. Retry.

0 comments on commit f931b13

Please sign in to comment.