Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 5780 #5991

Merged
merged 28 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
077888b
add crud methods for case-specific placements
guswhitten Aug 12, 2024
13ac07d
add placement destroy action
guswhitten Aug 12, 2024
5dcd091
final functionalities added: use Modal Components, create & new actions
guswhitten Aug 13, 2024
89e38ff
cleanup unused things
guswhitten Aug 13, 2024
4f441f9
write unit tests for added placement views
guswhitten Aug 16, 2024
ac6d735
merge main
guswhitten Aug 16, 2024
f63a76b
run linter
guswhitten Aug 16, 2024
00d522d
ensure placement_types are not hardcoded but still org-specific
guswhitten Aug 16, 2024
d2c47c2
add back placement show action and placement decorators
guswhitten Aug 16, 2024
bf36ae1
undo placement_types factory change
guswhitten Aug 16, 2024
6fe1d04
fix-5780: add placement_policy_spec.rb
guswhitten Aug 16, 2024
cca8df1
undo config change
guswhitten Aug 16, 2024
c49f427
fix failing tests for casa_case.show
guswhitten Aug 17, 2024
1de908c
clean up placements, remove most added error handling and _placements…
guswhitten Aug 19, 2024
61e332b
fix placement policy on certain actions
guswhitten Aug 20, 2024
c649a51
add system specs for CRUD actions on placements
guswhitten Aug 21, 2024
4ba2aca
fix failing tests
guswhitten Aug 22, 2024
bd7237f
update placement policies
guswhitten Aug 23, 2024
a966bc5
update spec/system/placements/index_spec.rb test
guswhitten Aug 23, 2024
798eee1
flaky test fixes
guswhitten Aug 23, 2024
3979c9e
get latest main
guswhitten Aug 31, 2024
be506a9
add policy scope for placements index action
guswhitten Aug 31, 2024
ff85445
update placement policy scope
guswhitten Aug 31, 2024
9262d03
please linter
guswhitten Aug 31, 2024
4d3bcf3
Update config/environments/test.rb
guswhitten Aug 31, 2024
bb6f003
Update config/environments/test.rb
guswhitten Aug 31, 2024
38353a0
dont display placement info on casa case when org has no placement ty…
guswhitten Sep 3, 2024
630bb24
run linter
guswhitten Sep 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/assets/stylesheets/pages/casa_cases.scss
guswhitten marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,4 @@ body.casa_cases-show {
flex-direction: column;
gap: .2rem;
}
}
}
37 changes: 36 additions & 1 deletion app/controllers/placements_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,53 @@ class PlacementsController < ApplicationController
before_action :set_placement, only: %i[edit show generate update destroy]
before_action :require_organization!

rescue_from ActiveRecord::RecordNotFound, with: -> { head :not_found }
def index
authorize @casa_case
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the placement policy for this. You should be able to do something like Placement.authorize.

@placements = @casa_case.placements.includes(:placement_type).order(placement_started_at: :desc)

This comment was marked as resolved.

This comment was marked as resolved.

end

def show
authorize @casa_case
end

def new
authorize @casa_case
@placement = Placement.new

This comment was marked as resolved.

end

def edit
authorize @placement
end

def create
@placement = Placement.new(placement_params)
authorize @placement

if @placement.save
redirect_to casa_case_placements_path(@casa_case), notice: "Placement was successfully created."
else
render :new, status: :unprocessable_entity
end
end

def update
authorize @placement

if @placement.update(placement_params)
redirect_to casa_case_placements_path(@casa_case), notice: "Placement was successfully updated."
else
render :edit, status: :unprocessable_entity
end
end

def destroy
authorize @placement

if @placement.destroy
redirect_to casa_case_placements_path(@casa_case), notice: "Placement was successfully deleted."
else
render :edit, status: :unprocessable_entity
end
end

private
Expand All @@ -33,4 +61,11 @@ def set_casa_case
def set_placement
@placement = @casa_case.placements.find(params[:id])
end

def placement_params
params.require(:placement).permit(
:placement_started_at,
:placement_type_id
).merge({creator_id: current_user.id, casa_case_id: @casa_case.id})
end
end
1 change: 1 addition & 0 deletions app/models/casa_org.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class CasaOrg < ApplicationRecord
has_many :contact_topics
has_one_attached :logo
has_one_attached :court_report_template
has_many :placement_types, dependent: :destroy
guswhitten marked this conversation as resolved.
Show resolved Hide resolved

def casa_admins
CasaAdmin.in_organization(self)
Expand Down
2 changes: 2 additions & 0 deletions app/models/placement_type.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class PlacementType < ApplicationRecord
belongs_to :casa_org

validates :name, presence: true
scope :for_organization, ->(org) { where(casa_org: org).order(:name) }

This comment was marked as resolved.

end

# == Schema Information
Expand Down
19 changes: 19 additions & 0 deletions app/policies/placement_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
class PlacementPolicy < ApplicationPolicy
def allowed_to_edit_casa_case?
casa_case_policy.edit?
end

alias_method :index?, :admin_or_supervisor_or_volunteer_same_org?
alias_method :show?, :admin_or_supervisor_or_volunteer_same_org?
alias_method :edit?, :allowed_to_edit_casa_case?
alias_method :update?, :allowed_to_edit_casa_case?
alias_method :new?, :admin_or_supervisor_or_volunteer_same_org?
alias_method :create?, :allowed_to_edit_casa_case?
alias_method :destroy?, :admin_or_supervisor?

private

def casa_case_policy
CasaCasePolicy.new(user, record.casa_case)
end
end
27 changes: 14 additions & 13 deletions app/views/casa_cases/_placements.html.erb
guswhitten marked this conversation as resolved.
Show resolved Hide resolved
guswhitten marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
<% placements = casa_case.placements %>
<label>Placements:</label>
<% if placements.empty? %>
No Placements
<% else %>
<ul>
<% placements.each do |pcd| %>
<p>
<%= link_to(pcd.decorate.placement_info, casa_case_placement_path(casa_case, pcd)) %>
</p>
<% end %>
</ul>
<% end %>
<% current_placement = casa_case.placements.order(placement_started_at: :desc).first %>

<h6>
<strong>Current Placement:</strong>

<% if current_placement %>
<%= current_placement.placement_type.name %>
<p>Placed since: <%= current_placement.decorate.formatted_date %></p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<%= current_placement.placement_type.name %>
<span><%= current_placement.placement_type.name %></span>

wrap with p or span some kind of html element. same with "Unknown" below.

I don't love all of these paragraphs being wrapped by a heading (h6) element. Should that h6 just be for "Current Placement"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to keep it similar to other casa_case partials. the h6 includes Current Placement and the placement_type, but now excludes the link to "See All Placements" (though its CSS was being overridden anyway)

<% else %>
Unknown
<% end %>

<p><%= link_to "See All Placements", casa_case_placements_path(casa_case), class: 'text-primary hover-underline' %></p>
</h6>
18 changes: 18 additions & 0 deletions app/views/placements/_fields.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<div class="input-style-1">
<%= form.label :placement_started_at, "Placement Started At" %>
<%= form.date_field :placement_started_at,
value: placement.placement_started_at&.to_date || Time.zone.now,
class: "form-control" %>
</div>
<div class="select-style-1">
<%= form.label :placement_type_id, "Placement Type" %>
<div class="select-position">
<%= form.collection_select(
:placement_type_id,
PlacementType.for_organization(current_organization),
:id, :name,
{include_hidden: false, include_blank: "-Select Placement Type-"},
{class: "form-control"}
) %>
</div>
</div>
29 changes: 29 additions & 0 deletions app/views/placements/_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<div class="card-style mb-30">
<div class="alert-box danger-alert">
<%= render "/shared/error_messages", resource: placement %>
</div>
<%= form_with(model: placement, url: [casa_case, placement], local: true,
data: { controller: "placement-form", nested_form_wrapper_selector_value: ".nested-form-wrapper" }) do |form| %>
<div class="row align-items-center">
<div class="col-md-6">
<h6><strong>Case Number:</strong> <%= link_to casa_case.case_number, casa_case %></h6>
</div>
<div class="col-md-6">
<div class="breadcrumb-wrapper">
<span class="top-page-actions ml-5">
<%= button_tag(type: "submit", class: "btn-sm main-btn primary-btn btn-hover") do %>
<% if placement.persisted? %>
<i class="lni lni-pencil-alt"></i> Update
<% else %>
<i class="lni lni-plus"></i> Create
<% end %>
<% end %>
</span>
</div>
</div>
</div>
<div class="row align-items-center">
<%= render 'placements/fields', placement: placement, form: form, casa_case: casa_case %>
</div>
<% end %>
</div>
11 changes: 11 additions & 0 deletions app/views/placements/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div class="title-wrapper pt-30">
<div class="row align-items-center">
<div class="col-md-6">
<div class="title mb-30">
<h1>Editing Placement</h1>
</div>
</div>
</div>
</div>
Comment on lines +1 to +9

This comment was marked as outdated.


<%= render 'form', casa_case: @casa_case, placement: @placement %>
69 changes: 69 additions & 0 deletions app/views/placements/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<div class="title-wrapper pt-30">
<div class="row align-items-center">
<div class="col-md-6">
<div class="title mb-30">
<h1>Placement History for
<%= link_to "#{@casa_case.case_number}", casa_case_path(@casa_case) %>
</h1>
</div>
</div>
<div class="col-md-6">
<div class="breadcrumb-wrapper mb-30">
<%= link_to new_casa_case_placement_path(@casa_case), class: "main-btn btn-sm primary-btn btn-hover ml-3" do %>
<i class="lni lni-plus"></i>
New Placement
<% end %>
</div>
</div>
</div>
</div>
<div class="col-lg-12">
<div class="card-style mb-30">
<table class="table">
<thead>
<tr>
<th>Placement Type</th>
<th>Date</th>
<th></th>
<th></th>
</tr>
</thead>
<tbody>
<% @placements.each_with_index do |placement, idx| %>
<tr>
<td><%= placement.placement_type.name %></td>
<td>
<%= placement.decorate.formatted_date %>
-
<% if idx.zero? %>
Present
<% elsif @placements[idx - 1].placement_started_at %>
<%= (@placements[idx - 1].placement_started_at - 1.day).strftime('%B %d, %Y') %>
<% end %>
</td>
<td><%= link_to edit_casa_case_placement_path(@casa_case, placement), class: "btn-sm main-btn primary-btn-outline btn-hover ms-auto" do %>
<i class="lni lni-pencil-alt"></i>
Edit
<% end %>
</td>
<td>
<%= render(Modal::OpenLinkComponent.new(target: placement.id, klass: "btn-sm main-btn danger-btn-outline btn-hover ms-auto")) do %>
<i class="lni lni-trash-can"></i>
Delete
<% end %>
</td>
</tr>
<%= render(Modal::GroupComponent.new(id: placement.id)) do |component| %>
<% component.with_header(text: "Delete Placement?", id: placement.id) %>
<% component.with_footer do %>
<%= link_to casa_case_placement_path(@casa_case, placement), method: :delete, class: "btn-sm main-btn danger-btn btn-hover ms-auto" do %>
<i class="lni lni-trash-can"></i>
Confirm
<% end %>
<% end %>
<% end %>
<% end %>
</tbody>
</table>
</div>
</div>
11 changes: 11 additions & 0 deletions app/views/placements/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div class="title-wrapper pt-30">
<div class="row align-items-center">
<div class="col-md-6">
<div class="title mb-30">
<h1>New Placement</h1>
</div>
</div>
</div>
</div>

<%= render 'form', casa_case: @casa_case, placement: @placement %>
2 changes: 1 addition & 1 deletion app/views/placements/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
</div>
<div class="col-md-6">
<div class="breadcrumb-wrapper mb-30">
<%= link_to '#', class: "btn-sm main-btn primary-btn btn-hover" do %>
<%= link_to edit_casa_case_placement_path(@casa_case, @placement), class: "btn-sm main-btn primary-btn btn-hover" do %>
<i class="lni lni-pencil-alt mr-10"></i>
Edit
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@

resources :court_dates, only: %i[create edit new show update destroy]

resources :placements, only: %i[create edit new show update destroy]
resources :placements

member do
patch :deactivate
Expand Down
10 changes: 10 additions & 0 deletions spec/factories/casa_orgs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,15 @@
trait :with_logo do
logo { Rack::Test::UploadedFile.new(Rails.root.join("spec", "fixtures", "org_logo.jpeg")) }
end

trait :with_placement_types do
transient { placement_types { ["Reunification", "Adoption", "Foster Care", "Kinship"] } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
transient { placement_types { ["Reunification", "Adoption", "Foster Care", "Kinship"] } }
transient { placement_names { ["Reunification", "Adoption", "Foster Care", "Kinship"] } }

Confusing, these aren't placement types, they are just type names.


after(:create) do |org, evaluator|
evaluator.placement_types.each do |name|
org.placement_types.create!(name: name)
end
end
Copy link
Contributor

@thejonroberts thejonroberts Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think the placement_names change should be made at the least:

Suggested change
after(:create) do |org, evaluator|
evaluator.placement_types.each do |name|
org.placement_types.create!(name: name)
end
end
after(:create) do |org, evaluator|
evaluator.placement_names.each do |name|
org.placement_types.create!(name: name)
end
end

end
end
end
2 changes: 1 addition & 1 deletion spec/factories/placements.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
FactoryBot.define do
factory :placement do
association :creator, factory: :user
casa_case
placement_type
placement_started_at { DateTime.now }
creator { association :user }
end
end
45 changes: 45 additions & 0 deletions spec/policies/placement_policy_spec.rb

This comment was marked as resolved.

Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require "rails_helper"

RSpec.describe PlacementPolicy do
subject { described_class }

let(:casa_org) { create(:casa_org) }
let(:diff_org) { create(:casa_org) }

let(:placement) { create(:placement, casa_case:) }
let(:casa_case) { create(:casa_case, casa_org:) }

let(:casa_admin) { create(:casa_admin, casa_org:) }
let(:volunteer) { create(:volunteer, casa_org:) }
let(:supervisor) { create(:supervisor, casa_org:) }

permissions :index?, :new?, :create?, :edit?, :update?, :show? do
it { is_expected.to permit(casa_admin, placement) }

context "when a supervisor belongs to the same org as the case" do
it { expect(subject).to permit(supervisor, placement) }
end

context "when a supervisor does not belong to the same org as the case" do
let(:casa_case) { create(:casa_case, casa_org: diff_org) }

it { expect(subject).not_to permit(supervisor, placement) }
end

context "when volunteer is assigned" do
before { volunteer.casa_cases << casa_case }

it { is_expected.to permit(volunteer, placement) }
end

context "when volunteer is not assigned" do
it { is_expected.not_to permit(volunteer, placement) }
end
end

permissions :destroy? do
it { is_expected.to permit(casa_admin, placement) }
it { is_expected.to permit(supervisor, placement) }
it { is_expected.not_to permit(volunteer, placement) }
end
end
Loading
Loading