bookio / server-old

2 stars 0 forks source link

Ruby and Rails version #4

Open mounte opened 10 years ago

mounte commented 10 years ago

Just wanted to initiate a discussion regarding the choice of platform. Since all new heroku-apps created are based on Ruby 2 are there any issues or similar behind sticking to 1.9 for Bookio at the moment? Also Rails is moving on with the 4.x branches and I guess some time in the future a move to 4 is needed. Should we move already before the development picks up the pace?

joakimbengtson commented 10 years ago

I think we should move to the latest versions. Just before Magnus vacation we shifted to the latest JQM. There is no reason to stay in a certain version during development according to me. The same goes for backward compatibility with browsers, go for the latest versions! :)

javidgon commented 10 years ago

I agree with that, let's have the libraries as up-to-date as possible

magnus-engstrom commented 10 years ago

I tried to bump the Rails version when building the pull request for basic testing (https://github.com/bookio/server/pull/8), but ran into a couple of errors regarding routes.

I'm reluctant to change any routes until we have tests that can verify that they stay intact after refactoring, but someone with deeper knowledge about the API structure could have a go with doing the bump to 4.x.

mounte commented 10 years ago

I have "succesfully" moved to rails 4. I got the same problems as @mageng. What I did with the routes was first and foremost to replace all match-statements with corresponding get, put and delete statement. One problem with a named route (I think it was "get "/clients/:id" => "clients#show", :defaults => { :format => 'json' }, :as => :client") it needs to be placed before the route that has the canonical client name i.e. "get "/client" => "clients#self_get", :defaults => { :format => 'json' }".

also, I think routes should use resources whenever possible. For instance I successfully managed to change all icon routes:

  match "/icons" => "icons#index", :via => :get, :defaults => { :format => 'json' }
  match "/icons" => "icons#create", :via => :post, :defaults => { :format => 'json' }
  match "/icons/all" => "icons#all", :via => :get, :defaults => { :format => 'json' }
  match "/icons/folder/:folder" => "icons#get_by_folder", :via => :get, :defaults => { :format => 'json' }
  match "/icons/:id" => "icons#fetch", :via => :get, :defaults => { :format => 'json' }, :as => :icon
  match "/icons/:id" => "icons#destroy", :via => :delete, :defaults => { :format => 'json' }

to the more expressive:

  resources :icons, defaults: { format: :json } do
    collection do
      get 'all'
      get 'folder/:folder' => 'icons#get_by_folder'
    end
  end

The only change needed in the controller was to change the method fetch to be named show instead (that is the default name for getting a resouce by id).

Finally updated (not commited anywhere I think) routes.rb if you want to test

Booker::Application.routes.draw do
  get "/rentals" => "rentals#index", :defaults => { :format => 'json' }
  post "/rentals" => "rentals#create", :via => :post, :defaults => { :format => 'json' }
  get "/rentals/query" => "rentals#query", :defaults => { :format => 'json' }
  get "/rentals/:id" => "rentals#show", :defaults => { :format => 'json' }, :as => :rental
  delete "/rentals/:id" => "rentals#destroy", :defaults => { :format => 'json' }
  put "/rentals/:id" => "rentals#update", :defaults => { :format => 'json' }

  get "/scenes" => "scenes#index", :defaults => { :format => 'json' }
  post "/scenes" => "scenes#create", :via => :post, :defaults => { :format => 'json' }
  get "/scenes/:id" => "scenes#show", :defaults => { :format => 'json' }, :as => :scene
  delete "/scenes/:id" => "scenes#destroy", :defaults => { :format => 'json' }
  put "/scenes/:id" => "scenes#update", :defaults => { :format => 'json' }

  get "/categories" => "categories#fetch_all", :defaults => { :format => 'json' }
  post "/categories" => "categories#create", :via => :post, :defaults => { :format => 'json' }
  get "/categories/:id" => "categories#fetch", :defaults => { :format => 'json' }, :as => :category
  delete "/categories/:id" => "categories#destroy", :defaults => { :format => 'json' }
  put "/categories/:id" => "categories#update", :defaults => { :format => 'json' }

  get "/customers" => "customers#index", :defaults => { :format => 'json' }
  post "/customers" => "customers#create", :via => :post, :defaults => { :format => 'json' }
  get "/customers/search_email" => "customers#search_email", :defaults => { :format => 'json' }
  get "/customers/search/:search_text" => "customers#search", :defaults => { :format => 'json' }
  get "/customers/:id" => "customers#show", :defaults => { :format => 'json' }, :as => :customer
  delete "/customers/:id" => "customers#destroy", :defaults => { :format => 'json' }
  put "/customers/:id" => "customers#update", :defaults => { :format => 'json' }

  get "/users" => "users#index", :defaults => { :format => 'json' }
  post "/users" => "users#create", :via => :post, :defaults => { :format => 'json' }
  get "/users/guest" => "users#guest", :defaults => { :format => 'json' }
  get "/users/:id" => "users#show", :defaults => { :format => 'json' }, :as => :user
  delete "/users/:id" => "users#destroy", :defaults => { :format => 'json' }
  put "/users/:id" => "users#update", :defaults => { :format => 'json' }

  resources :icons, defaults: { format: :json } do
    collection do
      get 'all'
      get 'folder/:folder' => 'icons#get_by_folder'
    end
  end

  get "/clients/:id" => "clients#show", :defaults => { :format => 'json' }, :as => :client
  get "/client" => "clients#self_get", :defaults => { :format => 'json' }
  put "/client" => "clients#self_update", :defaults => { :format => 'json' }

  get "/clients" => "clients#index", :defaults => { :format => 'json' }
  post "/clients" => "clients#create", :defaults => { :format => 'json' }

  delete "/clients/:id" => "clients#destroy", :defaults => { :format => 'json' }
  put "/clients/:id" => "clients#update", :defaults => { :format => 'json' }

  delete "/sessions/:id" => "sessions#destroy", :defaults => { :format => 'json' }
  get "/sessions" => "sessions#index", :defaults => { :format => 'json' }
  get "/sessions/:id" => "sessions#show", :defaults => { :format => 'json' }, :as => :session

  get "/signup" => "sessions#signup", :defaults => { :format => 'json' }
  get "/login" => "sessions#login", :defaults => { :format => 'json' }
  get "/logout" => "sessions#logout", :defaults => { :format => 'json' }
  get "/verify" => "sessions#verify", :defaults => { :format => 'json' }

  get "/reservations" => "reservations#index", :defaults => { :format => 'json' }
  post "/reservations" => "reservations#create", :via => :post, :defaults => { :format => 'json' }
  get "/reservations/foo" => "reservations#foo", :defaults => { :format => 'json' }
  get "/reservations/:id" => "reservations#show", :defaults => { :format => 'json' }, :as => :reservation
  delete "/reservations/:id" => "reservations#destroy", :defaults => { :format => 'json' }
  put "/reservations/:id" => "reservations#update", :defaults => { :format => 'json' }

  get "/settings" => "settings#index", :defaults => { :format => 'json' }
  put "/settings/:section/:name" => "settings#put", :defaults => { :format => 'json' }
  get "/settings/:section/:name" => "settings#get", :defaults => { :format => 'json' }
  delete "/settings/:section/:name" => "settings#destroy", :defaults => { :format => 'json' }
  delete "/settings/:section" => "settings#destroy_section", :defaults => { :format => 'json' }

  #resources :users, :has_many => :reservations
  #resources :rentals, :has_many => :reservations
  #resources :customers, :has_many => :reservations

  root :to => "home#index"
end
javidgon commented 10 years ago

Don't be afraid of creating a different branch with these new changes :smile: This way we could keep a better track of the changes thank to the "Compare" view that Github offers. And the same time, allow others to test your changes just by "pulling" them.

mounte commented 10 years ago

Ok, commited to branch mounte_dev feel free to cherrypick :) no promises on stability etc. just work in progress but as @javidgon wrote, easier for you to pull and merge now. https://github.com/bookio/server/commit/2a3ba4296ae40a3580997591209789a505f1610e