Make contributions restful and authorized

app/controllers/announcements_controller.rb

@@ -51,7 +51,7 @@ def redirect_after_create
       if context.can_admin?
         redirect_to announcements_path, notice: "#{notice} We'll review."
       else
-        redirect_to contribution_thank_you_path, notice: notice
+        redirect_to thank_you_path, notice: notice
       end
     end
 

app/controllers/asks_controller.rb

@@ -15,7 +15,7 @@ def create
     submission = SubmissionForm.build submission_params
     if submission.save
       EmailNewSubmission.run! submission: submission, user: current_user
-      redirect_to contribution_thank_you_path, notice: 'Ask was successfully created.'
+      redirect_to thank_you_path, notice: 'Ask was successfully created.'
     else
       render_form(submission)
     end

app/controllers/community_resources_controller.rb

@@ -58,7 +58,7 @@ def redirect_after_create
       if context.can_admin?
         redirect_to community_resources_path, notice: notice
       else
-        redirect_to contribution_thank_you_path, notice: "#{notice} We'll review."
+        redirect_to thank_you_path, notice: "#{notice} We'll review."
       end
     end
 

app/controllers/contributions_controller.rb

@@ -1,13 +1,10 @@
 # frozen_string_literal: true
 
 class ContributionsController < ApplicationController
-  include NotUsingPunditYet
-
-  before_action :authenticate_user!, except: %i[thank_you], unless: :peer_to_peer_mode?
-  before_action :set_contribution, only: %i[show triage]
-
-  layout 'without_navbar', only: [:thank_you]
+  before_action :authenticate_user!, unless: :peer_to_peer_mode?
+  skip_after_action :verify_policy_scoped
 
+  # FIXME: this should probably be wrapped by a policy scope?
   def index
     @filter_types = FilterTypeBlueprint.render([ContributionType, Category, ServiceArea, UrgencyLevel, ContactMethod])
     filter = BrowseFilter.new(filter_params)
@@ -19,39 +16,26 @@ def index
   end
 
   def show
-    contribution = Listing.find(params[:id])
-    @communication_logs = CommunicationLog.where(person: @contribution.person).order(sent_at: :desc)
-
-    render(
-      :show,
-      locals: {
-        contribution: contribution,
-      }
-    )
+    @communication_logs = CommunicationLog.where(person: contribution.person).order(sent_at: :desc)
   end
 
-  def combined_form; end
-
-  def thank_you; end
-
-  def triage; end
+  def edit; end
 
-  def triage_update
-    @contribution = Listing.find(params[:id])
-    contribution_params = params[@contribution.type.downcase.to_sym]
+  def update
+    contribution_params = params[contribution.type.downcase.to_sym]
     title = contribution_params[:title]
     description = contribution_params[:description]
     inexhaustible = contribution_params[:inexhaustible]
 
-    if @contribution.update(title: title, description: description, inexhaustible: inexhaustible)
-      # CommunicationLog.create!(person: @contribution.person,
+    if contribution.update(title: title, description: description, inexhaustible: inexhaustible)
+      # CommunicationLog.create!(person: contribution.person,
       #                          sent_at: Time.current,
       #                          subject: "triaged by #{current_user.name}",
       #                          delivery_status: "connected",
-      #                          delivery_method: @contribution.person.preferred_contact_method)
-      redirect_to contribution_path(@contribution), notice: 'Contribution was successfully updated.'
+      #                          delivery_method: contribution.person.preferred_contact_method)
+      redirect_to contribution_path(contribution), notice: 'Contribution was successfully updated.'
     else
-      render triage_contribution_path(@contribution)
+      render :edit
     end
   end
 
@@ -79,7 +63,8 @@ def allowed_params
     @allowed_params ||= params.permit(:format, **BrowseFilter::ALLOWED_PARAMS)
   end
 
-  def set_contribution
-    @contribution = Listing.find(params[:id])
+  def contribution
+    @contribution ||= authorize Listing.find(params[:id]), policy_class: ContributionPolicy
   end
+  helper_method :contribution
 end

app/controllers/donations_controller.rb

@@ -24,7 +24,7 @@ def create
     @donation = Donation.new(donation_params)
 
     if @donation.save
-      redirect_to @admin_status ? donations_path : contribution_thank_you_path, notice: 'Donation was successfully created.'
+      redirect_to @admin_status ? donations_path : thank_you_path, notice: 'Donation was successfully created.'
     else
       set_form_dropdowns
       render :new

app/controllers/listings_controller.rb

@@ -69,7 +69,7 @@ def edit
   def create
     @listing = Listing.new(listing_params)
     if @listing.save
-      redirect_to contribution_thank_you_path, notice: 'Listing was successfully created.'
+      redirect_to thank_you_path, notice: 'Listing was successfully created.'
     else
       set_form_dropdowns
       render :new

app/controllers/offers_controller.rb

@@ -15,7 +15,7 @@ def create
     submission = SubmissionForm.build submission_params
     if submission.save
       EmailNewSubmission.run! submission: submission, user: current_user
-      redirect_to contribution_thank_you_path, notice: 'Offer was successfully created.'
+      redirect_to thank_you_path, notice: 'Offer was successfully created.'
     else
       render_form(submission)
     end

app/controllers/thank_you_controller.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+class ThankYouController < PublicController
+  layout 'without_navbar'
+
+  def show
+    @current_organization = Organization.current_organization
+    @system_setting = context.system_settings
+  end
+end

app/helpers/application_helper.rb

@@ -42,7 +42,7 @@ def triage_button(resource, button_color_class=nil)
     if resource_class != Person && (resource_class.superclass != ApplicationRecord)
       resource = resource.becomes(resource.class.superclass)
     end
-    link_to(triage_contribution_path(resource),
+    link_to(edit_contribution_path(resource),
             title: 'Triage',
             class: "button triage-button #{button_color_class || 'is-primary'}") do
       "<span class='fa fa-edit'></span><span style='padding-left: 0.25em'> Triage</span>".html_safe

app/policies/contribution_policy.rb

@@ -1,13 +1,6 @@
 class ContributionPolicy < ApplicationPolicy
-  def read?
-    acting_user && (acting_user.admin_role? || acting_user.sys_admin_role? )
-  end
-
-  def change?
-    acting_user && (acting_user.admin_role? || acting_user.sys_admin_role? )
-  end
-
-  def read_details?
-    change?
-  end
+  def read?;         true       end
+  def change?;       can_admin? end
+  def delete?;       can_admin? end
+  def read_details?; change?    end
 end

app/views/contributions/_tentative_match.html.erb

@@ -1,22 +1,22 @@
-<% match_assn_name = @contribution.ask? ? "matches_as_receiver" : "matches_as_provider" %>
-<% matches_for_this_contribution = @contribution.public_send(match_assn_name) %>
+<% match_assn_name = contribution.ask? ? "matches_as_receiver" : "matches_as_provider" %>
+<% matches_for_this_contribution = contribution.public_send(match_assn_name) %>
 
 <% if matches_for_this_contribution.any? %>
-  <strong>This <%= @contribution.type %> has been Matched:</strong>
+  <strong>This <%= contribution.type %> has been Matched:</strong>
   <br>
   <% matches_for_this_contribution.order(created_at: :desc).each do |match| %>
     <%= edit_button(match, "#{shorthand_display(match.created_at)} #{match.category.upcase}: #{match.short_name}", nil, nil, nil, {}, "View Match") %>
     <br>
   <% end %>
   <br><br>
-<% elsif @contribution.ask? %>
-  <%= render "layouts/view_add_new_button", button_text: "Add tentative Match", table_name: "matches", new_params: "receiver_id=#{@contribution.id}&receiver_type=Ask" %>
+<% elsif contribution.ask? %>
+  <%= render "layouts/view_add_new_button", button_text: "Add tentative Match", table_name: "matches", new_params: "receiver_id=#{contribution.id}&receiver_type=Ask" %>
   <%= link_to("Find Match [TBD]",
-              match_listing_path(@contribution, receiver_id: @contribution.id, receiver_type: "Ask"),
+              match_listing_path(contribution, receiver_id: contribution.id, receiver_type: "Ask"),
               class: "button") if params[:admin] %>
-<% elsif @contribution.offer? %>
-  <%= render "layouts/view_add_new_button", button_text: "Add tentative Match", table_name: "matches", new_params: "provider_id=#{@contribution.id}&provider_type=Offer" %>
+<% elsif contribution.offer? %>
+  <%= render "layouts/view_add_new_button", button_text: "Add tentative Match", table_name: "matches", new_params: "provider_id=#{contribution.id}&provider_type=Offer" %>
   <%= link_to("Find Match [TBD]",
-              match_listing_path(@contribution, provider_id: @contribution.id, provider_type: "Offer"),
+              match_listing_path(contribution, provider_id: contribution.id, provider_type: "Offer"),
               class: "button") if params[:admin] %>
-<% end %>
+<% end %>

app/views/contributions/_top_level_card.html.erb

@@ -3,27 +3,27 @@
     <strong>Contribution:</strong>
     <br>
     <span class="title is-1">
-      <span class="<%= @contribution.icon_class %>"></span> <%= @contribution&.type %>: <%= @contribution&.all_tags_to_s %>
+      <span class="<%= contribution.icon_class %>"></span> <%= contribution&.type %>: <%= contribution&.all_tags_to_s %>
     </span>
   </div>
   <div class="columns">
     <div class="contribution column">
-      <%#= edit(@contribution) %><!-- # TODO - add update action back in once we have moved controllers-->
-      <div class="created_at">Created on <%= shorthand_display(@contribution.created_at) %></div>
-      <div class="block"><p class="subtitle"><%= @contribution.description %></p></div>
+      <%#= edit(contribution) %><!-- # TODO - add update action back in once we have moved controllers-->
+      <div class="created_at">Created on <%= shorthand_display(contribution.created_at) %></div>
+      <div class="block"><p class="subtitle"><%= contribution.description %></p></div>
       <p class="control">
-        <% if @contribution.matched? %>
+        <% if contribution.matched? %>
           <i class="far fa-check-circle has-text-primary is-size-5 has-text-weight-semibold"><span class="ml-1">Claimed</span></i>
         <% else %>
-          <%= button_to "Claim This #{@contribution.type}", new_contribution_claim_path(@contribution), class: "button is-primary", method: :get %>
+          <%= button_to "Claim This #{contribution.type}", new_contribution_claim_path(contribution), class: "button is-primary", method: :get %>
         <% end %>
       </p>
       <br>
       <% if policy(:contribution).read_details? %>
         <div class="field is-grouped">
-          <p class="control"> <%= triage_button(@contribution) %> </p>
+          <p class="control"> <%= triage_button(contribution) %> </p>
           <br>
-          <%= render "tentative_match", contribution: @contribution %>
+          <%= render "tentative_match", contribution: contribution %>
         </div>
         <span class="subtitle is-7 is-italic"><%= link_to "More info about Triage", dispatch_steps_path %></span>
       <% end %>
@@ -33,7 +33,7 @@
         <div class="contact-info block">
           <strong>Created by:</strong>
           <br>
-          <%= show_button(person, person&.name, "fa fa-user-circle", "title is-5", nil, contribution_id: @contribution.id) %>
+          <%= show_button(person, person&.name, "fa fa-user-circle", "title is-5", nil, contribution_id: contribution.id) %>
           <br>
           <strong>Preferred contact info:</strong>
           <span class="<%= person.preferred_contact_method.icon_class %>"></span>
@@ -48,7 +48,7 @@
         <div class="block">
           <strong>Created by:</strong>
           <br>
-          <strong><%= @contribution.person.anonymized_name_and_email %></strong>
+          <strong><%= contribution.person.anonymized_name_and_email %></strong>
         </div>
       <% end %>
     </div>

app/views/contributions/edit.html.erb

@@ -0,0 +1,18 @@
+<h1 class="title is-3">
+  <%= "Update #{contribution.name}" %>
+</h1>
+
+<%= simple_form_for contribution, url: contribution_path(contribution) do |f| %>
+  <%= f.error_notification %>
+
+  <div class="form-inputs">
+    <%= f.input :title, label: "Summary/title (45 chars)", input_html: { maxlength: 45 } %>
+    <%= f.input :description,
+                label: "Notes (for THIS CATEGORY only! create a new <a href='/asks/new?person_id=#{contribution.person&.id}'> Ask
+                  </a> or <a href='/offers/new?person_id=#{contribution.person&.id}'> Offer
+                  </a>, as needed, but save this form first!)".html_safe %>
+    <%= f.input :inexhaustible, as: :radio_buttons %>
+  </div>
+
+  <%= f.button :submit, "Update #{contribution.type}", class: "button mt-1 is-primary" %>
+<% end %>

app/views/contributions/show.html.erb

@@ -1,8 +1,8 @@
-<% person = @contribution.person %>
+<% person = contribution.person %>
 <div class="block">
   <%= link_to "<i class=\"fas fa-arrow-left\"></i> View Contributions".html_safe, contributions_path %>
 </div>
-<%= render "top_level_card", person: person, contribution: @contribution %>
+<%= render "top_level_card", person: person, contribution: contribution %>
 
 <% if policy(:contribution).read_details? %>
   <div class="columns">
@@ -23,10 +23,10 @@
         <% contribution_assn = type.to_s.pluralize.downcase %>
         <strong><%= "#{person.name}'s " %> other matchable <%= contribution_assn.titleize %>:</strong>
         <br>
-        <% if @contribution.person.public_send(contribution_assn).matchable.
-            where.not("listings.id = ?", @contribution.id).any? %>
-          <% @contribution.person.public_send(contribution_assn).matchable.
-              where.not("listings.id = ?", @contribution.id).
+        <% if contribution.person.public_send(contribution_assn).matchable.
+            where.not("listings.id = ?", contribution.id).any? %>
+          <% contribution.person.public_send(contribution_assn).matchable.
+              where.not("listings.id = ?", contribution.id).
               order(created_at: :desc).each do |contribution| %>
             <%= link_to("<span class='#{contribution.icon_class}'></span>  #{
                         shorthand_display(contribution.created_at)} #{contribution.all_tags_to_s.upcase}".html_safe,
@@ -44,7 +44,7 @@
       <strong><%= "#{person.name}'s " %> Match history:</strong>
       <br>
       <% no_matches_comments = [] %>
-      <% @contribution.person.listings.where.not(id: @contribution).order(created_at: :desc).each do |listing| %>
+      <% contribution.person.listings.where.not(id: contribution).order(created_at: :desc).each do |listing| %>
         <% match_assn_name = listing.ask? ? "matches_as_receiver" : "matches_as_provider" %>
         <% other_matches = listing.public_send(match_assn_name) %>
         <% if other_matches.any? %>
@@ -60,4 +60,4 @@
       <%= no_matches_comments.uniq.join if no_matches_comments.uniq.length == 1 %>
     </div>
   </div>
-<% end %>
+<% end %>

config/routes.rb

@@ -27,14 +27,8 @@
   end
   resources :community_resources
   resources :contact_methods
-  get '/combined_form', to: 'contributions#combined_form', as: 'combined_form'
-  get '/thank_you', to: 'contributions#thank_you', as: 'contribution_thank_you'
-  resources :contributions, only: %i[index show] do
-    member do
-      get '/triage', to: 'contributions#triage', as: 'triage'
-      patch '/triage', to: 'contributions#triage_update'
-      post '/triage', to: 'contributions#triage_update'
-    end
+  resource :thank_you, only: %i[show], controller: :thank_you
+  resources :contributions, except: %i[destroy] do
     resources :claims, only: %i[new create]
   end
   resources :custom_form_questions

spec/policies/contribution_policy_spec.rb

@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+# FIXME: remove or consolidate this block once #834 is resolved
+RSpec.configure do
+  Pundit::Matchers.configure do |config|
+    config.user_alias = :acting_user
+  end
+end
+
+RSpec.describe ContributionPolicy do
+  let(:context) { Context.new user: user }
+  let(:contribution) { double :contribution }
+
+  subject { ContributionPolicy.new context, contribution }
+
+  context 'guest user' do
+    let(:user) { nil }
+
+    it { is_expected.to permit_actions %i[show] }
+    it { is_expected.to forbid_actions %i[new edit update destroy] }
+  end
+
+  context 'authenticated user' do
+    let(:user) { FactoryBot.build :user, :dispatcher }
+
+    it { is_expected.to permit_actions %i[show] }
+    it { is_expected.to forbid_actions %i[new edit update destroy] }
+  end
+
+  context 'admin user' do
+    let(:user) { FactoryBot.build :user, :admin }
+
+    it { is_expected.to permit_actions %i[show edit update destroy] }
+  end
+end