Skip to content

Commit

Permalink
Expend assigned user selection filter for all the site (#34)
Browse files Browse the repository at this point in the history
* first version of input blood tests

* necessary permission fixes in lab_tests_controller

* release manual input blood tests

* manual input blood tests results

* tests part

* Fix errors and pass tests for RuboCop

* updated bundle and passed rubocop tests

* bundle updated, passed rubocop tests

* found and fixed bug with a colors in tests. Passed rubocop and rspec.

* fixed by recommendations by Anton

* pass tests

* fixes for rubocop tests pass

* Fixed a few errors, but I have trouble with tests passing for negative value.

* new lab test input fixed

* manual input with test

* factory users added

* rubocop rules added

* merged with a develop branch and fixed rubocop offenses

* some fixes

* answers for comments and small fixes

* merged with develop branch, changed user selection same as Anton in lab tests table filtration

* all tests is passed

* moved set_biomarkers to before_action

* small fixes related with comments

* renamed method from determine_user to set_user and all test is passed

* DS_Store deleted from git

* fixed policy_scope in reference ranges

* removed addtional reinit lab_test in lab_tests_controller

* removed @Biomarker from index of reference ranges

* edited message to correct grammatically view

* edit refenrence ranges controller, index method, back var biamarker and changed policy_scope from all to reference_ranges

* Add params comment for assigned_users_list_for_select

* Add rubocop-capybara

* Remove turbo from lab_test show view

* Remove unused code

* Add rubocop-capybara to GH actions workflow to fix pipeline

* fixed error with empty list of assigned users and creation new lab test with a user rights

* fixed testing - when the user has no assigned users, changed to recieving array with a current user

* Add TODO

* Expend assigned user selection filter for all the site

---------

Co-authored-by: Volodymyr Shynkarov <[email protected]>
  • Loading branch information
aksafan and VoldemLive authored Jan 6, 2025
1 parent 618132b commit b85daee
Show file tree
Hide file tree
Showing 16 changed files with 232 additions and 68 deletions.
27 changes: 25 additions & 2 deletions app/controllers/admin/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ def update_assigned_users

respond_to do |format|
if @user.assign_users?(@user_params_user_ids)
delete_user_from_session if should_remove_user_from_session

format.html { redirect_to admin_user_url(@user), notice: t('.success') }
format.json { render :show, status: :ok, location: @user }
else
Expand All @@ -90,9 +92,31 @@ def update_assigned_users
end
end

# POST /switch_user or /switch_user.json
def switch_user
if params['user_id']
session[:user_id] = params['user_id'].to_i

reload_page notice: t('.success')
else
reload_page alert: t('.failure')
end
end

private

# Use callbacks to share common setup or constraints between actions.
def reload_page(message)
redirect_to(request.referer || admin_users_path, message)
end

def delete_user_from_session
session.delete(:user_id)
end

def should_remove_user_from_session
@user_params_user_ids.exclude?(session[:user_id])
end

def set_user
@user = User.find(params[:id])
end
Expand All @@ -109,7 +133,6 @@ def set_user_params_user_ids
@user_params_user_ids = user_params[:users].compact_blank
end

# Only allow a list of trusted parameters through.
def user_params
params.require(:user).permit(:first_name, :last_name, :email, roles: [], users: [])
end
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ class ApplicationController < ActionController::Base

before_action :configure_permitted_parameters, if: :devise_controller?

before_action :set_filter_by_user_id

rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized
rescue_from ActiveRecord::RecordNotFound, with: :record_not_found

Expand Down Expand Up @@ -37,4 +39,8 @@ def record_not_found(error)

render file: "#{::Rails.root}/public/404.html", status: :not_found
end

def set_filter_by_user_id
@chosen_user_id = session.fetch(:user_id, current_user&.id)
end
end
2 changes: 1 addition & 1 deletion app/controllers/health_records_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class HealthRecordsController < ApplicationController
# GET /health_records or /health_records.json
def index
authorize HealthRecord
@health_records = policy_scope(HealthRecord.all)
@health_records = policy_scope(HealthRecord).where(user_id: @chosen_user_id)
end

# GET /health_records/1 or /health_records/1.json
Expand Down
18 changes: 1 addition & 17 deletions app/controllers/lab_tests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class LabTestsController < ApplicationController
before_action :build_lab_test, only: %i[create]
before_action :set_filter_by_user_id, only: %i[index]
before_action :set_biomarkers, only: %i[index new edit create]

# GET /lab_tests or /lab_tests.json
def index
authorize LabTest
Expand All @@ -17,14 +18,6 @@ def index
@biomarkers = policy_scope(Biomarker)
.includes(:reference_ranges, :lab_tests)
.where(lab_tests: { user_id: @chosen_user_id })

respond_to do |format|
format.html
format.turbo_stream do
render partial: 'lab_tests/index_table',
locals: { recordables: @recordables, biomarkers: @biomarkers }
end
end
end

# GET /lab_tests/1 or /lab_tests/1.json
Expand Down Expand Up @@ -141,15 +134,6 @@ def build_lab_test
@lab_test = current_user.lab_tests.build(lab_test_params)
end

def set_filter_by_user_id
current_user_selected = filter_params.empty? ||
!filter_params[:user_id] ||
# To exclude not assigned users selecting. TODO: move this to Pundit policy.
current_user.assigned_users.map(&:id).exclude?(filter_params[:user_id].to_i)

@chosen_user_id = current_user_selected ? current_user.id : filter_params[:user_id]
end

# Only allow a list of trusted parameters through.
def lab_test_params
params
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/measurements_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class MeasurementsController < ApplicationController
# GET /measurements or /measurements.json
def index
authorize Measurement
@measurements = policy_scope(Measurement.all)
@measurements = policy_scope(Measurement).where(user_id: @chosen_user_id)
end

# GET /measurements/1 or /measurements/1.json
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/users_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def assigned_user_ids_list(user)
# Adds current user pair if it was not selected - TODO: rethought/rewrite this to make behavior more consistent
# Or empty array if there are no assigned users
def assigned_users_list_for_select(user, current_user)
return [] if user.assigned_users.empty?
return [current_user.full_name] if user.assigned_users.empty?

current_user_included = false
assigned_users = user.assigned_users.map do |assigned_user|
Expand All @@ -54,6 +54,6 @@ def users_list_for_select(user)
end

def assigned_users?
current_user.full_access_roles_can? && !current_user.assigned_users.empty?
(current_user&.full_access_roles_can? && !current_user.assigned_users.empty?) || false
end
end
4 changes: 4 additions & 0 deletions app/policies/user_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ def update_assigned_users?
user.admin? && record.full_access_roles_can?
end

def switch_user
user.full_access_roles_can?
end

class Scope < ApplicationPolicy::Scope
def resolve
if user.admin?
Expand Down
46 changes: 11 additions & 35 deletions app/views/lab_tests/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,37 +1,13 @@
<section>
<%= turbo_frame_tag "lab_tests", class: "shadow overflow-hidden rounded border-b border-gray-200" do %>

<header class="tw-my-2 tw-flex tw-flex-center tw-justify-between tw-items-center">
<h1 class="tw-text-lg">Lab Tests</h1>

<% if assigned_users? %>
<div class="flex justify-end mb-1">
<%= form_with url: lab_tests_path, method: :get, data: { controller: "search-form", search_form_target: "form", turbo_frame: "lab_tests" } do |form| %>
<%= form.select :user_id,
options_for_select(
assigned_users_list_for_select(current_user, current_user),
@chosen_user_id
),
{ include_blank: false },
class: "border-blue-500 rounded",
data: {
action: "change->search-form#search"
}
%>
<% end %>
</div>
<% end %>

<%= button_to new_lab_test_path,
data: { turbo_frame: "_top" }, # This will load in the full page
method: :get,
class: "btn btn-primary" do %>
New Lab Test
<% end %>
</header>

<%= render "index_table", recordables: @recordables, biomarkers: @biomarkers %>

<% end %>

<header class="tw-my-2 tw-flex tw-flex-center tw-justify-between tw-items-center">
<h1 class="tw-text-lg">Lab Tests</h1>
<%= link_to "New Lab Test",
new_lab_test_path,
method: :get,
class: "btn btn-primary",
data: { turbo_frame: "_top" }
%>
</header>

<%= render "index_table", recordables: @recordables, biomarkers: @biomarkers %>
</section>
16 changes: 8 additions & 8 deletions app/views/lab_tests/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@

<div class="tw-mb-4" data-controller="biomarker-select">
<%= form.label :biomarker_id, class: "tw-block tw-text-sm tw-font-medium tw-text-gray-700" %>
<%= form.collection_select :biomarker_id,
<%= form.collection_select :biomarker_id,
@biomarkers,
:id,
:name,
{ prompt: "Select biomarker", required: true },
{ class: "tw-mt-1 tw-block tw-w-full tw-rounded-md #{@lab_test.errors[:biomarker_id].present? ? 'tw-border-red-500' : 'tw-border-gray-300'}",
data: {
data: {
biomarker_select_target: "select",
action: "change->biomarker-select#updateReferenceRange"
}
Expand All @@ -59,7 +59,7 @@
<p class="tw-text-sm tw-text-gray-600">
<strong>Reference Range:</strong>
<span data-biomarker-select-target="referenceRange">
<%= @selected_biomarker&.reference_ranges&.first ?
<%= @selected_biomarker&.reference_ranges&.first ?
"#{@selected_biomarker.reference_ranges.first.min_value} - #{@selected_biomarker.reference_ranges.first.max_value}" :
"Select biomarker first" %>
</span>
Expand All @@ -72,21 +72,21 @@
</p>
</div>

<%= form.hidden_field :reference_range_id,
<%= form.hidden_field :reference_range_id,
value: @selected_biomarker&.reference_ranges&.first&.id,
data: { biomarker_select_target: "referenceRangeId" } %>
<%= form.hidden_field :unit,
<%= form.hidden_field :unit,
value: @selected_biomarker&.reference_ranges&.first&.unit,
data: { biomarker_select_target: "unitField" } %>
</div>

<div class="tw-mb-4">
<%= form.label :value, class: "tw-block tw-text-sm tw-font-medium tw-text-gray-700" %>
<div class="tw-flex tw-items-center">
<%= form.number_field :value,
<%= form.number_field :value,
step: :any,
required: true,
class: "tw-mt-1 tw-block tw-w-full tw-rounded-md #{@lab_test.errors[:value].present? ? 'tw-border-red-500' : 'tw-border-gray-300'}"
class: "tw-mt-1 tw-block tw-w-full tw-rounded-md #{@lab_test.errors[:value].present? ? 'tw-border-red-500' : 'tw-border-gray-300'}"
%>
</div>

Expand All @@ -97,7 +97,7 @@
<%= form.text_area :notes, class: "tw-mt-1 tw-block tw-w-full tw-rounded-md tw-border-gray-300" %>
</div>

<%= form.submit "Create #{params[:test_type]&.titleize || 'Lab'} Test",
<%= form.submit "Create #{params[:test_type]&.titleize || 'Lab'} Test",
class: "tw-bg-blue-500 tw-hover:bg-blue-700 tw-text-white tw-font-bold tw-py-2 tw-px-4 tw-rounded" %>
<% end %>

Expand Down
17 changes: 17 additions & 0 deletions app/views/layouts/_navbar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@
<%= link_to "Users", admin_users_path, class: "nav-link #{current_page?(admin_users_path) ? 'active' : ''}" %>
<% end %>
</li>
<li class="nav-item">
<% if assigned_users? %>
<div class="flex justify-end mb-1">
<%= form_with url: switch_user_admin_user_path(current_user), method: :post do |form| %>
<%= form.select :user_id,
options_for_select(
assigned_users_list_for_select(current_user, current_user),
@chosen_user_id
),
{ include_blank: false },
class: "tw-border-blue-500 rounded"
%>
<%= form.submit "Switch client", class: "tw-bg-blue-500 tw-hover:bg-blue-700 tw-text-white tw-font-bold tw-py-2 tw-px-4 tw-rounded" %>
<% end %>
</div>
<% end %>
</li>
</ul>
<% if user_signed_in? %>
<span> Welcome <%= current_user.full_name %> </span>
Expand Down
3 changes: 3 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ en:
success: "User roles were successfully updated."
update_assigned_users:
success: "Users were successfully assigned."
switch_user:
success: "Client was switched"
failure: "Client was not switched"

application:
user_not_authorized:
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
post :update_roles
get :edit_assigned_users
post :update_assigned_users
post :switch_user
end
end
end
Expand Down
58 changes: 56 additions & 2 deletions spec/helpers/users_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,63 @@
context 'when the user has no assigned users' do
let(:user_with_no_assigned_users) { instance_double(User, assigned_users: []) }

it 'returns an empty array' do
it 'returns an array with the single current user' do
result = helper.assigned_users_list_for_select(user_with_no_assigned_users, current_user)
expect(result).to eq([])
expect(result).to eq([current_user.full_name])
end
end
end

describe '#assigned_users?' do
let(:current_user) { instance_double(User) }

before do
allow(helper).to receive(:current_user).and_return(current_user)
end

context 'when current_user has full access roles and assigned users' do
before do
allow(current_user).to receive_messages(full_access_roles_can?: true, assigned_users: [instance_double(User)])
end

it 'returns true' do
expect(helper.assigned_users?).to be true
end
end

context 'when current_user has full access roles but no assigned users' do
before do
allow(current_user).to receive_messages(full_access_roles_can?: true, assigned_users: [])
end

it 'returns false' do
expect(helper.assigned_users?).to be false
end
end

context 'when current_user does not have full access roles' do
before do
allow(current_user).to receive(:full_access_roles_can?).and_return(false)
end

it 'returns false, regardless of assigned users' do
allow(current_user).to receive(:assigned_users).and_return([instance_double(User)])
expect(helper.assigned_users?).to be false
end

it 'returns false when there are no assigned users' do
allow(current_user).to receive(:assigned_users).and_return([])
expect(helper.assigned_users?).to be false
end
end

context 'when current_user is nil' do
before do
allow(helper).to receive(:current_user).and_return(nil)
end

it 'returns false' do
expect(helper.assigned_users?).to be false
end
end
end
Expand Down
Loading

0 comments on commit b85daee

Please sign in to comment.