From 94f1aab569a1d5a7d196864efa82ae76a01e6ab0 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Thu, 15 Jun 2017 23:33:26 +0200 Subject: [PATCH 01/11] Remove current_organization magic in order to use standard REST urls --- app/controllers/application_controller.rb | 25 +---------- app/controllers/members_controller.rb | 16 +++++++- app/controllers/organizations_controller.rb | 36 ++++++++-------- app/controllers/posts_controller.rb | 41 +++++++++++++------ app/controllers/users_controller.rb | 32 ++++++++++----- app/helpers/posts_helper.rb | 7 +++- app/views/application/_navbar.html.erb | 10 ++--- .../_organization_listings_menu.html.erb | 4 +- .../menus/_organization_switcher.html.erb | 8 +--- app/views/inquiries/index.html.erb | 2 +- app/views/layouts/application.html.erb | 4 +- app/views/offers/index.html.erb | 2 +- app/views/shared/_post.html.erb | 2 +- app/views/shared/_post_form.html.erb | 4 +- app/views/users/_user_rows.html.erb | 2 +- app/views/users/give_time.html.erb | 6 +-- app/views/users/index.html.erb | 6 +-- 17 files changed, 110 insertions(+), 97 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2b934011e..ede758798 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,7 +9,6 @@ class ApplicationController < ActionController::Base append_before_filter :check_for_terms_acceptance!, unless: :devise_controller? before_filter :configure_permitted_parameters, if: :devise_controller? before_filter :set_locale - before_filter :set_current_organization after_filter :store_location rescue_from MissingTOSAcceptance, OutadedTOSAcceptance do @@ -18,7 +17,7 @@ class ApplicationController < ActionController::Base rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized - helper_method :current_organization, :admin?, :superadmin? + helper_method :admin?, :superadmin? protected @@ -26,14 +25,6 @@ def configure_permitted_parameters devise_parameter_sanitizer.for(:sign_up) << :username end - def set_current_organization - if org_id = session[:current_organization_id] - @current_organization = Organization.find(org_id) - elsif current_user - @current_organization = current_user.organizations.first - end - end - def store_location # store last url - this is needed for post-login redirect to whatever the # user last visited. @@ -66,20 +57,8 @@ def check_for_terms_acceptance! end end - def current_organization - @current_organization ||= current_user.try(:organizations).try(:first) - end - - def current_member - @current_member ||= current_user.as_member_of(current_organization) if current_user - end - - def pundit_user - current_member - end - def admin? - current_user.try :manages?, current_organization + current_user.try :manages?, @organization end def superadmin? diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index c88ea52df..1b78d0b3d 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -1,5 +1,17 @@ class MembersController < ApplicationController before_filter :authenticate_user! + before_filter :load_organization + + # TODO: move to abstract controller for all nested resources + # TODO: check authorization + # + def load_organization + @organization = Organization.find_by_id(params[:id]) + + raise not_found unless @organization + + @organization + end def destroy find_member @@ -34,11 +46,11 @@ def toggle_active private def find_member - @member ||= current_organization.members.find(params[:id]) + @member ||= @organization.members.find(params[:id]) end def toggle_active_posts - current_organization.posts.where(user_id: @member.user_id). + @organization.posts.where(user_id: @member.user_id). each { |post| post.update_attributes(active: false) } end end diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 0e3ab87a7..8278e1d9b 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -1,20 +1,17 @@ class OrganizationsController < ApplicationController - before_filter :load_resource - - def load_resource - if params[:id] - @organization = Organization.find(params[:id]) - else - @organizations = Organization.all - end - end + before_filter :load_resource, only: [:show, :update, :destroy, :give_time] def new @organization = Organization.new end + # TODO: define which organizations we should display + # def index - @organizations = @organizations.matching(params[:q]) if params[:q].present? + context = Organization.all + context = context.matching(params[:q]) if params[:q].present? + + @organizations = context end def show @@ -56,13 +53,6 @@ def give_time @sources = find_transfer_sources_for_admin end - def set_current - if current_user - session[:current_organization_id] = @organization.id - end - redirect_to root_path - end - private def organization_params @@ -71,8 +61,18 @@ def organization_params neighborhood city domain]) end + def load_resource + @organization = Organization.find_by_id(params[:id]) + + raise unless @organization + + # TODO: authorize + + @organization + end + def find_transfer_offer - current_organization.offers. + @organization.offers. find(params[:offer]) if params[:offer].present? end diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 08c47942c..4bd9caaca 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -1,4 +1,6 @@ class PostsController < ApplicationController + before_filter :load_organization + has_scope :by_category, as: :cat has_scope :tagged_with, as: :tag has_scope :by_organization, as: :org @@ -11,9 +13,9 @@ def index type: "phrase_prefix", fields: ["title^2", "description", "tags^2"] } } ] - if current_organization.present? + if @organization.present? # filter by organization - must << { term: { organization_id: { value: current_organization.id } } } + must << { term: { organization_id: { value: @organization.id } } } end posts = model.__elasticsearch__.search( query: { @@ -24,8 +26,8 @@ def index ).page(params[:page]).per(25).records else posts = model.active.of_active_members - if current_organization.present? - posts = posts.merge(current_organization.posts) + if @organization.present? + posts = posts.merge(@organization.posts) end posts = apply_scopes(posts).page(params[:page]).per(25) end @@ -40,9 +42,9 @@ def new def create post = model.new(post_params) - post.organization = current_organization + post.organization = @organization if post.save - redirect_to send("#{resource}_path", post) + redirect_to polymorphic_url([@organization, post]) else instance_variable_set("@#{resource}", post) render action: :new @@ -50,13 +52,13 @@ def create end def edit - post = current_organization.posts.find params[:id] + post = @organization.posts.find params[:id] instance_variable_set("@#{resource}", post) end def show scope = if current_user.present? - current_organization.posts.active.of_active_members + @organization.posts.active.of_active_members else model.all.active.of_active_members end @@ -65,34 +67,36 @@ def show end def update - post = current_organization.posts.find params[:id] + post = @organization.posts.find params[:id] authorize post instance_variable_set("@#{resource}", post) if post.update_attributes(post_params) - redirect_to post + redirect_to polymorphic_url([@organization, post]) else render action: :edit, status: :unprocessable_entity end end def destroy - post = current_organization.posts.find params[:id] + post = @organization.posts.find params[:id] authorize post - redirect_to send("#{resources}_path") if post.update!(active: false) + redirect_to polymorphic_url([@organization, resources]) if post.update!(active: false) end private + # TODO: Investigate why we need this def resource controller_name.singularize end + # TODO: Investigate why we need this def resources controller_name end def set_user_id(p) - if current_user.manages?(current_organization) + if current_user.manages?(@organization) p.update publisher_id: current_user.id p.reverse_merge! user_id: current_user.id else @@ -109,4 +113,15 @@ def post_params set_user_id(p) end end + + # TODO: move to abstract controller for all nested resources + # TODO: check authorization + # + def load_organization + @organization = Organization.find_by_id(params[:organization_id]) + + raise not_found unless @organization + + @organization + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 68f6fb828..2eca903fa 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,13 +1,23 @@ class UsersController < ApplicationController before_filter :authenticate_user! + before_filter :load_organization, only: [:index] + + # TODO: move to abstract controller for all nested resources + def load_organization + @organization = Organization.find_by_id(params[:organization_id]) + + raise not_found unless @organization + + @organization + end def scoped_users - current_organization.users + @organization.users end def index @users = scoped_users - @memberships = current_organization.members. + @memberships = @organization.members. where(user_id: @users.map(&:id)). includes(:account).each_with_object({}) do |mem, ob| ob[mem.user_id] = mem @@ -16,7 +26,9 @@ def index def show @user = find_user - @member = @user.as_member_of(current_organization) + authorize @user + + @member = @user.as_member_of(@organization) @movements = @member.movements.order("created_at DESC").page(params[:page]). per(10) end @@ -42,7 +54,7 @@ def create @user.setup_and_save_user if @user.persisted? - @user.tune_after_persisted(current_organization) + @user.tune_after_persisted(@organization) redirect_to_after_create else @user.email = "" if empty_email @@ -64,7 +76,7 @@ def update def give_time @user = scoped_users.find(params[:id]) @destination = @user.members. - find_by(organization: current_organization).account.id + find_by(organization: @organization).account.id @source = find_transfer_source @offer = find_transfer_offer @transfer = Transfer.new(source: @source, @@ -86,19 +98,19 @@ def user_params end def find_transfer_offer - current_organization.offers. + @organization.offers. find(params[:offer]) if params[:offer].present? end def find_transfer_source current_user.members. - find_by(organization: current_organization).account.id + find_by(organization: @organization).account.id end def find_transfer_sources_for_admin return unless admin? - [current_organization.account] + - current_organization.member_accounts.where("members.active is true") + [@organization.account] + + @organization.member_accounts.where("members.active is true") end def find_user @@ -110,7 +122,7 @@ def find_user end def redirect_to_after_create - id = @user.member(current_organization).member_uid + id = @user.member(@organization).member_uid if params[:more] redirect_to new_user_path, notice: I18n.t("users.new.user_created_add", diff --git a/app/helpers/posts_helper.rb b/app/helpers/posts_helper.rb index 1737bf674..ea4721ed8 100644 --- a/app/helpers/posts_helper.rb +++ b/app/helpers/posts_helper.rb @@ -1,13 +1,16 @@ module PostsHelper # Returns the right path to index list depending on type of post + # + # TODO: doesn't Rails URL helpers already provide this? def get_index_path(post, hparams) klass = post.class + hparams[:organization_id] = @organization.id case when klass == String - post.eql?("offers") ? offers_path(hparams) : inquiries_path(hparams) + post.eql?("offers") ? organization_offers_path(hparams) : organization_inquiries_path(hparams) else - post.type.eql?("Offer") ? offers_path(hparams) : inquiries_path(hparams) + post.type.eql?("Offer") ? organization_offers_path(hparams) : organization_inquiries_path(hparams) end end end diff --git a/app/views/application/_navbar.html.erb b/app/views/application/_navbar.html.erb index f6520881d..ada5324b4 100644 --- a/app/views/application/_navbar.html.erb +++ b/app/views/application/_navbar.html.erb @@ -10,11 +10,7 @@ - <% if current_user && current_organization %> - TimeOverflow - <% else %> TimeOverflow - <% end %>
@@ -41,7 +37,7 @@
-<% if current_user && current_organization %> +<% if current_user && @organization %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/application/menus/_organization_listings_menu.html.erb b/app/views/application/menus/_organization_listings_menu.html.erb index 6ea4ba67e..92fd16e0b 100644 --- a/app/views/application/menus/_organization_listings_menu.html.erb +++ b/app/views/application/menus/_organization_listings_menu.html.erb @@ -12,14 +12,14 @@ <% end %>
  • - <%= link_to offers_path(org: current_organization) do %> + <%= link_to organization_offers_path(organization_id: @organization.id) do %> <%= glyph :link %> <%= glyph :eye_open %> <%= t "application.navbar.offer_public_link" %> <% end %>
  • - <%= link_to inquiries_path(org: current_organization) do %> + <%= link_to organization_inquiries_path(organization_id: @organization.id) do %> <%= glyph :link %> <%= glyph :eye_open %> <%= t "application.navbar.inquiry_public_link" %> diff --git a/app/views/application/menus/_organization_switcher.html.erb b/app/views/application/menus/_organization_switcher.html.erb index 10d3ca099..2fd3b383f 100644 --- a/app/views/application/menus/_organization_switcher.html.erb +++ b/app/views/application/menus/_organization_switcher.html.erb @@ -1,10 +1,6 @@ -
  • - <%= current_organization.name %> -
    • -<% (current_user.organizations - [current_organization]).each do |org| %> +<% current_user.organizations.each do |org| %>
  • - <%= link_to set_current_organization_path(org), method: :post do %> - <%= glyph(:retweet) %> + <%= link_to organization_path(org) do %> <%= org.name %> <% end %>
    • diff --git a/app/views/inquiries/index.html.erb b/app/views/inquiries/index.html.erb index 4475ff71b..720773506 100644 --- a/app/views/inquiries/index.html.erb +++ b/app/views/inquiries/index.html.erb @@ -39,7 +39,7 @@ - <% if current_user && current_organization && !params[:org] %> + <% if current_user && @organization %> - <% if current_user && current_organization && !params[:org] %> + <% if current_user && @organization %>