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 all commits
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
4 changes: 3 additions & 1 deletion app/controllers/clients/files_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ def update
attrs[:effective_date] = attrs[:consent_form_signed_on]
attrs[:consent_form_confirmed] = true if GrdaWarehouse::Config.get(:auto_confirm_consent)
end
@file.update(attrs)
@file.assign_attributes(attrs)
@file.sync_revokation_info(current_user)
@file.save
end

def show_delete_modal
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/clients/releases_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ def update
attrs[:effective_date] = attrs[:consent_form_signed_on]
attrs[:consent_form_confirmed] = true if GrdaWarehouse::Config.get(:auto_confirm_consent)
end
@file.update(attrs)
@file.assign_attributes(attrs)
@file.sync_revokation_info(current_user)
@file.save
end

def file_params
Expand Down
19 changes: 15 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,10 +234,10 @@ 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_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
Expand Down Expand Up @@ -410,6 +411,16 @@ def data_sources_for_confidential_files(user)
end
end

def sync_revokation_info(current_user)
return unless consent_revoked_at_changed?

self.consent_revoked_by_user_id = if consent_revoked_at.present?
current_user.id
else # rubocop:disable Style/EmptyElse
nil
end
end

def copy_to_s3!
return unless content.present?
return if client_file.attached? # don't re-process
Expand Down
1 change: 1 addition & 0 deletions app/models/grda_warehouse/file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
module GrdaWarehouse
class File < GrdaWarehouseBase
acts_as_paranoid
has_paper_trail
Copy link
Contributor

@gigxz gigxz Oct 29, 2024

Choose a reason for hiding this comment

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

This caused a test regression on HMIS, because duplicate versions are being created for HMIS files. https://github.com/greenriver/hmis-warehouse/actions/runs/11525170831/job/32086897713

The problem is that Hmis::File also calls has_paper_trail (and attaches some metadata).

Paper Trail has an unreleased (breaking) change that will actually cause this to fail https://github.com/paper-trail-gem/paper_trail/blob/master/CHANGELOG.md#breaking-changes / paper-trail-gem/paper_trail#1479. They acknowledge that this can create duplicates. Glad we caught it with the test first!

I don't have much context on this PR, so not sure what the best course of action is. This class is pretty lightweight, so one approach is for Hmis::File to just not subclass it. I did that here #4872 and the tests pass, but it needed some weirdness to get type set.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch @gigxz ! I believe our intent was actually to add has_paper_trail to ClientFile. Not sure we need it in the base class. I'll give that a try and see if tests still fail.

belongs_to :user, optional: true
end
end
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