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

Track consent revoked by and display it on the file card #4857

Merged
merged 6 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 8 additions & 0 deletions app/controllers/clients/files_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class FilesController < ApplicationController
include ClientDependentControllers

before_action :require_window_file_access!
before_action :set_user
before_action :set_client
before_action :set_files, only: [:index]
before_action :set_window
Expand All @@ -19,6 +20,8 @@ class FilesController < ApplicationController
# before_action :require_can_manage_client_files!, only: [:update]
after_action :log_client

attr_accessor :user

def index
@consent_editable = consent_editable?
@consent_form_url = GrdaWarehouse::PublicFile.url_for_location 'client/hmis_consent'
Expand Down Expand Up @@ -249,6 +252,11 @@ def batch_params
params.require(:batch_download).permit(:file_ids)
end

def set_user
# binding.pry
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
# binding.pry

@user = current_user
end

def set_client
@client = destination_searchable_client_scope.find(params[:client_id].to_i)
end
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/clients/releases_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class ReleasesController < FilesController

skip_before_action :require_window_file_access!
before_action :require_can_use_separated_consent!
before_action :set_user

after_action :log_client

Expand Down Expand Up @@ -215,6 +216,10 @@ def set_window
@window = true
end

def set_user
@user = current_user
end

def editable_scope
scope = file_source.editable_by?(current_user).
where(client_id: @client.id)
Expand Down
15 changes: 11 additions & 4 deletions app/models/grda_warehouse/client_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ClientFile < GrdaWarehouse::File
belongs_to :vispdat, class_name: 'GrdaWarehouse::Vispdat::Base', optional: true
belongs_to :enrollment, class_name: 'GrdaWarehouse::Hud::Enrollment', optional: true
belongs_to :data_source, class_name: 'GrdaWarehouse::DataSource', optional: true
belongs_to :consent_revoked_by_user, class_name: 'User', optional: true
validates_inclusion_of :visible_in_window, in: [true, false]
validates_presence_of :expiration_date, on: :requires_expiration_date, message: 'Expiration date is required'
validates_presence_of :effective_date, on: :requires_effective_date, message: 'Effective date is required'
Expand Down Expand Up @@ -233,15 +234,21 @@ class ClientFile < GrdaWarehouse::File
####################
# Callbacks
####################
after_create_commit :notify_users, if: ->(m) { m.should_run_callbacks? }
before_save :adjust_consent_date, if: ->(m) { m.should_run_callbacks? }
after_save :note_changes_in_consent, if: ->(m) { m.should_run_callbacks? }
after_commit :set_client_consent, on: [:create, :update], if: ->(m) { m.should_run_callbacks? }
after_create_commit :notify_users, if: ->(m) { m.should_run_callbacks? } # rubocop:disable Style/SymbolProc
before_update :adjust_revoked_by, if: ->(m) { m.should_run_callbacks? && consent_revoked_at_changed? }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some reason we can't just call this from the controller? There are a bunch of callbacks, but the controller should know the user that made the change, so should be able to set consent_revoked_by_user_id: current_user.id when creating or updating the record. (There is significant complexity here, so I may be missing something.)

While we're in here, it might be worth adding has_paper_trail. I was going to suggest we pull the user out of the versions table, but it appears these aren't being logged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was mentioned in the ticket that this may be a good way to go. At the time, it felt like capturing it at a deeper level may good if we are working with files in multiple controllers, but I see what you are saying and it does feel less complex to just have the controller handle it. I'll adjust the PR.

before_save :adjust_consent_date, if: ->(m) { m.should_run_callbacks? } # rubocop:disable Style/SymbolProc
after_save :note_changes_in_consent, if: ->(m) { m.should_run_callbacks? } # rubocop:disable Style/SymbolProc
after_commit :set_client_consent, on: [:create, :update], if: ->(m) { m.should_run_callbacks? } # rubocop:disable Style/SymbolProc

def should_run_callbacks?
callbacks_skipped.nil? || ! callbacks_skipped
end

def adjust_revoked_by
revoked_by_id = user&.id if consent_revoked_at.present?
self.consent_revoked_by_user_id = revoked_by_id
end

####################
# Access
####################
Expand Down
3 changes: 3 additions & 0 deletions app/views/clients/files/_file_row.haml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@
= f.input :consent_form_confirmed, as: :boolean, input_html: { class: 'submit-on-change' }, label: Translation.translate('Consent form confirmed')
= simple_form_for file, remote: true, url: appropriate_file_path(id: file.id) do |f|
= f.input :consent_revoked_at, as: :date_picker, label: 'Revoked or Refused On', input_html: { disabled: true, class: 'submit-on-change enable-on-load' }
- if file.revoked?
%dd Revoked or Refused By: #{file.consent_revoked_by_user&.name}
- if GrdaWarehouse::Config.get(:release_duration) == 'Use Expiration Date'
= simple_form_for file, remote: true, url: appropriate_file_path(id: file.id) do |f|
= f.input :expiration_date, as: :date_picker, label: 'Expiration Date', input_html: { disabled: true, class: 'submit-on-change enable-on-load' }
Expand All @@ -86,6 +88,7 @@
%dl
- if file.revoked?
%dt Consent revoked on #{file.consent_revoked_at.to_date}
%dd Consent revoked by #{file.consent_revoked_by_user.name}
- else
%dt
= checkmark file.consent_form_confirmed
Expand Down
5 changes: 5 additions & 0 deletions db/warehouse/migrate/20241022185534_add_revoked_by_to_file.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddRevokedByToFile < ActiveRecord::Migration[7.0]
def change
add_column :files, :consent_revoked_by_user_id, :integer, null: true
end
end
Loading
Loading