Merge pull request #384 from coopdevs/feature/set-current-org-on-post-show

Automatically switch to post's organization on post show

Author: enricostano

app/controllers/application_controller.rb

@@ -17,6 +17,7 @@ class ApplicationController < ActionController::Base
   end
 
   rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized
+  rescue_from ActiveRecord::RecordNotFound, with: :resource_not_found
 
   helper_method :current_organization, :admin?, :superadmin?
 
@@ -118,4 +119,8 @@ def user_not_authorized
     flash[:error] = "You are not authorized to perform this action."
     redirect_to(request.referrer || root_path)
   end
+
+  def resource_not_found
+    render 'errors/not_found', status: 404
+  end
 end

app/controllers/posts_controller.rb

@@ -57,13 +57,13 @@ def edit
     instance_variable_set("@#{resource}", post)
   end
 
+  # GET /offers/:id
+  # GET /inquiries/:id
+  #
   def show
-    scope = if current_user.present?
-              current_organization.posts.active.of_active_members
-            else
-              model.all.active.of_active_members
-            end
-    post = scope.find params[:id]
+    post = Post.active.of_active_members.find(params[:id])
+    update_current_organization!(post.organization)
+
     instance_variable_set("@#{resource}", post)
   end
 
@@ -115,4 +115,20 @@ def post_params
       set_user_id(p)
     end
   end
+
+  # TODO: remove this horrible hack ASAP
+  #
+  # This hack set the current organization to the post's
+  # organization, both in session and controller instance variable.
+  #
+  # Before changing the current organization it's important to check that
+  # the current_user is an active member of the organization.
+  #
+  # @param organization [Organization]
+  def update_current_organization!(organization)
+    return unless current_user && current_user.active?(organization)
+
+    session[:current_organization_id] = organization.id
+    @current_organization = organization
+  end
 end

app/policies/application_policy.rb

@@ -1,6 +1,10 @@
 class ApplicationPolicy
   attr_reader :member, :user, :organization, :record
 
+  # TODO: Investigate how to just pass current_user here.
+  #       Probably this will be solved by scoping the resources
+  #       under `/organization`.
+  #
   def initialize(member, record)
     @member = member
     @user = member.user if member

app/views/inquiries/show.html.erb

@@ -1,15 +1,8 @@
-<p class="actions text-right">
-  <% if admin? || @inquiry.user == current_user %>
-    <%= link_to edit_inquiry_path(@inquiry), class: "btn btn-warning" do %>
-      <%= glyph :pencil %>
-      <%= t "global.edit" %>
+<% if @inquiry.organization == current_organization %>
+  <p class="actions text-right">
+    <% if admin? or @inquiry.user == current_user %>
+      <%= render 'shared/post_actions', post: @inquiry %>
     <% end %>
-    <%= link_to @inquiry,
-                data: { method: :delete, confirm: "sure?" },
-                class: "btn btn-danger" do %>
-      <%= glyph :trash %>
-      <%= t "global.delete" %>
-    <% end %>
-  <% end %>
-</p>
+  </p>
+<% end %>
 <%= render "shared/post", post: @inquiry %>

app/views/offers/show.html.erb

@@ -1,22 +1,15 @@
-<p class="actions text-right">
-  <% if admin? or @offer.user == current_user %>
-    <%= link_to edit_offer_path(@offer), class: "btn btn-warning" do %>
-      <%= glyph :pencil %>
-      <%= t "global.edit" %>
+<% if @offer.organization == current_organization %>
+  <p class="actions text-right">
+    <% if admin? or @offer.user == current_user %>
+      <%= render 'shared/post_actions', post: @offer %>
     <% end %>
-    <%= link_to @offer,
-                data: { method: :DELETE, confirm: "sure?" },
-                class: "btn btn-danger" do %>
-      <%= glyph :trash %>
-      <%= t "global.delete" %>
+    <% if current_user and @offer.user != current_user %>
+      <%= link_to new_transfer_path(id: @offer.user.id, offer: @offer.id, destination_account_id: @destination_account.id),
+                  class: "btn btn-success" do %>
+        <%= glyph :time %>
+        <%= t ".give_time_for" %>
+      <% end %>
     <% end %>
-  <% end %>
-  <% if current_user and @offer.user != current_user %>
-    <%= link_to new_transfer_path(id: @offer.user.id, offer: @offer.id, destination_account_id: @destination_account.id),
-                class: "btn btn-success" do %>
-      <%= glyph :time %>
-      <%= t ".give_time_for" %>
-    <% end %>
-  <% end %>
-</p>
+  </p>
+<% end %>
 <%= render "shared/post", post: @offer %>

app/views/shared/_post.html.erb

@@ -27,7 +27,7 @@
         <% end %>
       </div>
     </div>
-    <% if current_user && current_organization %>
+    <% if current_user && current_organization == post.organization %>
       <div class="panel panel-info">
         <div class="panel-heading">
           <h3 class="panel-title">
@@ -60,13 +60,17 @@
     </div>
   </div>
 </div>
+<% if !current_user || post.organization != current_organization %>
+  <div class="alert alert-info">
+    <%= t 'posts.show.info',
+      type: post.class.model_name.human,
+      organization: post.organization.name %>
+  </div>
+<% end %>
 <% unless current_user %>
   <div class="alert alert-info">
-    <%= t "posts.show.info",
-          type: post.class.model_name.human,
-          organization: post.organization.name %>
     <%= link_to t("layouts.application.login"),
-                new_user_session_path,
-                class: "btn btn-primary" %>
+      new_user_session_path,
+      class: "btn btn-primary" %>
   </div>
 <% end %>

app/views/shared/_post_actions.html.erb

@@ -0,0 +1,9 @@
+<%= link_to post, class: "btn btn-warning" do %>
+  <%= glyph :pencil %>
+  <%= t "global.edit" %>
+<% end %>
+
+<%= link_to post, data: { method: :delete, confirm: "sure?" }, class: "btn btn-danger" do %>
+  <%= glyph :trash %>
+  <%= t "global.delete" %>
+<% end %>

config/locales/en.yml

@@ -407,7 +407,7 @@ en:
       impulsem-link: "Impulsem el que fas"
   posts:
     show:
-      info: "%{type} of %{organization} to see person's details you have to"
+      info: "This %{type} belongs to %{organization}."
   reports:
     cat_with_users:
       title: Offered Services

spec/controllers/offers_controller_spec.rb

@@ -1,15 +1,15 @@
 require "spec_helper"
 
-describe OffersController, type: :controller do
-  let(:test_organization) { Fabricate(:organization) }
-  let(:member) { Fabricate(:member, organization: test_organization) }
-  let(:another_member) { Fabricate(:member, organization: test_organization) }
+RSpec.describe OffersController, type: :controller do
+  let(:organization) { Fabricate(:organization) }
+  let(:member) { Fabricate(:member, organization: organization) }
+  let(:another_member) { Fabricate(:member, organization: organization) }
   let(:yet_another_member) { Fabricate(:member) }
   let(:test_category) { Fabricate(:category) }
   let!(:offer) do
     Fabricate(:offer,
               user: member.user,
-              organization: test_organization,
+              organization: organization,
               category: test_category)
   end
 
@@ -59,29 +59,105 @@
     end
   end
 
-  describe "GET #show" do
-    context "with valid params" do
-      context "with a logged user" do
-        before { login(another_member.user) }
-
-        it "assigns the requested offer to @offer" do
-          get :show, id: offer.id
-          expect(assigns(:offer)).to eq(offer)
+  describe 'GET #show' do
+    context 'when the user is logged in' do
+      before { login(another_member.user) }
+
+      context 'when the requested offer' do
+        context 'is not active' do
+          before do
+            offer.active = false
+            offer.save!
+          end
+
+          it 'renders the 404 page' do
+            get :show, id: offer.id
+            expect(response.status).to eq(404)
+          end
         end
 
-        it 'assigns the account destination of the transfer' do
-          get :show, id: offer.id
-          expect(assigns(:destination_account)).to eq(member.account)
+        context 'is active' do
+          context 'and the user that created the offer is not active anymore' do
+            before do
+              member.active = false
+              member.save!
+            end
+
+            it 'renders the 404 page' do
+              get :show, id: offer.id
+              expect(response.status).to eq(404)
+            end
+          end
+
+          context 'and the user that created the offer is active' do
+            it 'renders a successful response' do
+              get :show, id: offer.id
+              expect(response.status).to eq(200)
+            end
+
+            it 'assigns the requested offer to @offer' do
+              get :show, id: offer.id
+              expect(assigns(:offer)).to eq(offer)
+            end
+
+            it 'assigns the account destination of the transfer' do
+              get :show, id: offer.id
+              expect(assigns(:destination_account)).to eq(member.account)
+            end
+
+            it 'displays the offer\'s user details' do
+              get :show, id: offer.id
+              expect(response.body).to include(offer.user.email)
+            end
+          end
         end
       end
 
-      context "without a logged in user" do
-        it "assigns the requested offer to @offer" do
-          get :show, id: offer.id
-          expect(assigns(:offer)).to eq(offer)
+      context 'when the user pertains to multiple organizations' do
+        context 'and user\'s current organization is different than offer\'s organization' do
+          let(:another_organization) { Fabricate(:organization) }
+
+          before do
+            Fabricate(:member, user: another_member.user, organization: another_organization)
+            allow(controller).to receive(:@current_organization).and_return(another_organization)
+          end
+
+          it 'displays the offer\'s user details' do
+            get :show, id: offer.id
+            expect(response.body).to include(offer.user.email)
+          end
+
+          it 'sets the offer\'s organization as user\'s current organization' do
+            get :show, id: offer.id
+            expect(session[:current_organization_id]).to eq(offer.organization_id)
+            expect(assigns(:current_organization)).to eq(offer.organization)
+          end
         end
       end
     end
+
+    context 'when the user is not a member of the organization where the offer is published' do
+      let(:another_user) { Fabricate(:user) }
+
+      before { login(another_user) }
+
+      it 'doesn\'t display the offer\'s user details' do
+        get :show, id: offer.id
+        expect(response.body).to_not include(offer.user.email)
+      end
+    end
+
+    context 'when the user is not logged in' do
+      it 'assigns the requested offer to @offer' do
+        get :show, id: offer.id
+        expect(assigns(:offer)).to eq(offer)
+      end
+
+      it 'doesn\'t display the offer\'s user details' do
+        get :show, id: offer.id
+        expect(response.body).to_not include(offer.user.email)
+      end
+    end
   end
 
   describe "POST #create" do