From 992618b482b4e62954b1c9477df15f87a24547ff Mon Sep 17 00:00:00 2001 From: Sean Craig <110493538+seancva@users.noreply.github.com> Date: Fri, 20 Oct 2023 09:22:44 -0500 Subject: [PATCH 01/40] =?UTF-8?q?SeanC/APPEALS-31071=20Add=20new=20databas?= =?UTF-8?q?e=20column=20=E2=80=9Cdecision=5Fdate=5Fadded=5Fat=E2=80=9D=20t?= =?UTF-8?q?o=20table=20=E2=80=9Crequest=5Fissues=E2=80=9D=20(#19735)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Created rails migration file * got happy route working for updating decision_date_added_at * added an after_create hook to save to the decision_date_add_at field * removed unnecessary code * added the decision_date_added_at field to a test * Added decision_date_added_at to a couple more tests * Added comment to the new field in the datebase * changed the migration to inherit form Caseflow::Migration * removed version number --- app/models/request_issue.rb | 12 +++++++++++- ...3_add_decision_date_added_at_to_request_issues.rb | 5 +++++ spec/models/request_issue_spec.rb | 3 +++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20231010161703_add_decision_date_added_at_to_request_issues.rb diff --git a/app/models/request_issue.rb b/app/models/request_issue.rb index e067788565f..768670c6537 100644 --- a/app/models/request_issue.rb +++ b/app/models/request_issue.rb @@ -69,6 +69,8 @@ class RequestIssue < CaseflowRecord before_save :set_contested_rating_issue_profile_date before_save :close_if_ineligible! + after_create :set_decision_date_added_at, if: :decision_date_exists? + # amoeba gem for splitting appeal request issues amoeba do enable @@ -512,7 +514,7 @@ def save_edited_contention_text!(new_description) def save_decision_date!(new_decision_date) fail DecisionDateInFutureError, id if new_decision_date.to_date > Time.zone.today - update!(decision_date: new_decision_date) + update!(decision_date: new_decision_date, decision_date_added_at: Time.zone.now) # Special handling for claim reviews that contain issues without a decision date decision_review.try(:handle_issues_with_no_decision_date!) @@ -1018,5 +1020,13 @@ def check_for_untimely! def appeal_active? decision_review.tasks.open.any? end + + def decision_date_exists? + decision_date.present? + end + + def set_decision_date_added_at + self.decision_date_added_at = created_at + end end # rubocop:enable Metrics/ClassLength diff --git a/db/migrate/20231010161703_add_decision_date_added_at_to_request_issues.rb b/db/migrate/20231010161703_add_decision_date_added_at_to_request_issues.rb new file mode 100644 index 00000000000..bb0fa9678e7 --- /dev/null +++ b/db/migrate/20231010161703_add_decision_date_added_at_to_request_issues.rb @@ -0,0 +1,5 @@ +class AddDecisionDateAddedAtToRequestIssues < Caseflow::Migration + def change + add_column :request_issues, :decision_date_added_at, :datetime, comment: "Denotes when a decision date was added" + end +end diff --git a/spec/models/request_issue_spec.rb b/spec/models/request_issue_spec.rb index 3741bd5008f..96bdf1f5224 100644 --- a/spec/models/request_issue_spec.rb +++ b/spec/models/request_issue_spec.rb @@ -1008,6 +1008,7 @@ it do is_expected.to have_attributes( decision_date: Time.zone.today, + decision_date_added_at: subject.created_at, notes: "notes", untimely_exemption: true, untimely_exemption_notes: "untimely notes", @@ -1390,6 +1391,7 @@ context "when decision date is missing" do it "returns nil" do expect(subject).to be_nil + expect(rating_request_issue_without_contested_issue.decision_date_added_at).to be_nil end end end @@ -2302,6 +2304,7 @@ expect(review).to receive(:handle_issues_with_no_decision_date!).once subject expect(nonrating_request_issue.decision_date).to eq(new_decision_date.to_date) + expect(nonrating_request_issue.decision_date_added_at).to eq(new_decision_date) end context "when the decision date is in the future" do From 3b1f7c3afb1912a06e8f02ea9113cb0dee91e1fc Mon Sep 17 00:00:00 2001 From: Tyler Broyles <109369527+TylerBroyles@users.noreply.github.com> Date: Fri, 20 Oct 2023 10:43:37 -0400 Subject: [PATCH 02/40] TYLERB/APPEALS-31182: Create controller routes/methods to process change history report request (#19676) * Initial commit. Added expected param permissions for the change history report generation page. * Added Controller action for change history report generation and the corresponding route. Also created an initial template for the change history reporter that will be used to create the csv. * Added controller action method tests for the new generate report action. Added some error handling for the filters params and the report within filters. * Cleaned up the controller tests by removing unused tests, variables, and comments. * Code climate fixes. * Disabled reek warning for a method in the unfinished reporter. * Added a small change to force the content type of the errype of the error messaging to be application/json for the csv controller action. * Removed puts statement. * Removed the generate_report html file and just render the index for now. Updated the route to /report instead of /report_generation. * Refactored the admin check in the generate_report action to be a guard clause for readability. Move the ChangeHistoryReporter import to the controller rspec test since it is not eager loaded in the testing environment, but it is in most other environments. * Moved the /report controller route into the decision reviews route block. * Fixed a double render error caused by the guard clause refactor on the report action. --------- Co-authored-by: Craig Reese <109101548+craigrva@users.noreply.github.com> --- .../decision_reviews_controller.rb | 75 +++++++++++++--- .../change_history_reporter.rb | 29 ++++++ config/routes.rb | 1 + .../decision_reviews_controller_spec.rb | 89 +++++++++++++++++++ 4 files changed, 182 insertions(+), 12 deletions(-) create mode 100644 app/services/claim_change_history/change_history_reporter.rb diff --git a/app/controllers/decision_reviews_controller.rb b/app/controllers/decision_reviews_controller.rb index 687c6066c52..a8f53b56264 100644 --- a/app/controllers/decision_reviews_controller.rb +++ b/app/controllers/decision_reviews_controller.rb @@ -6,6 +6,7 @@ class DecisionReviewsController < ApplicationController before_action :verify_access, :react_routed, :set_application before_action :verify_veteran_record_access, only: [:show] + before_action :verify_business_line, only: [:index, :generate_report] delegate :incomplete_tasks, :incomplete_tasks_type_counts, @@ -30,19 +31,13 @@ class DecisionReviewsController < ApplicationController }.freeze def index - if business_line - respond_to do |format| - format.html { render "index" } - format.csv do - jobs_as_csv = BusinessLineReporter.new(business_line).as_csv - filename = Time.zone.now.strftime("#{business_line.url}-%Y%m%d.csv") - send_data jobs_as_csv, filename: filename - end - format.json { queue_tasks } + respond_to do |format| + format.html { render "index" } + format.csv do + jobs_as_csv = BusinessLineReporter.new(business_line).as_csv + send_data jobs_as_csv, filename: csv_filename end - else - # TODO: make index show error message - render json: { error: "#{business_line_slug} not found" }, status: :not_found + format.json { queue_tasks } end end @@ -69,6 +64,24 @@ def update end end + def generate_report + return requires_admin_access_redirect unless business_line.user_is_admin?(current_user) + + respond_to do |format| + format.html { render "index" } + format.csv do + filter_params = change_history_params + + fail ActionController::ParameterMissing.new(:report), report_missing_message unless filter_params[:report] + + events_as_csv = ChangeHistoryReporter.new([], filter_params.to_h).as_csv + send_data events_as_csv, filename: csv_filename, type: "text/csv", disposition: "attachment" + end + end + rescue ActionController::ParameterMissing => error + render json: { error: error.message }, status: :bad_request, content_type: "application/json" + end + def business_line_slug allowed_params[:business_line_slug] || allowed_params[:decision_review_business_line_slug] end @@ -184,6 +197,12 @@ def verify_access redirect_to "/unauthorized" end + def verify_business_line + unless business_line + render json: { error: "#{business_line_slug} not found" }, status: :not_found + end + end + def verify_veteran_record_access if task.type == VeteranRecordRequest.name && !task.appeal.veteran&.accessible? render(Caseflow::Error::ActionForbiddenError.new( @@ -192,6 +211,21 @@ def verify_veteran_record_access end end + def csv_filename + Time.zone.now.strftime("#{business_line.url}-%Y%m%d.csv") + end + + def report_missing_message + "param is missing or the value is empty: report" + end + + def requires_admin_access_redirect + Rails.logger.info("User without admin access to the business line #{business_line} "\ + "couldn't access #{request.original_url}") + session["return_to"] = request.original_url + redirect_to "/unauthorized" + end + def allowed_params params.permit( :decision_review_business_line_slug, @@ -209,6 +243,23 @@ def allowed_params ) end + def change_history_params + params.require(:filters).permit( + :report, + events: [], + timing: [], + statuses: [], + conditions: { + days_waiting: [], + review_type: [], + issue_type: [], + disposition: [], + personnel: [], + facility: [] + } + ) + end + def power_of_attorney_data { representative_type: task.appeal&.representative_type, diff --git a/app/services/claim_change_history/change_history_reporter.rb b/app/services/claim_change_history/change_history_reporter.rb new file mode 100644 index 00000000000..1feef95573d --- /dev/null +++ b/app/services/claim_change_history/change_history_reporter.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class ChangeHistoryReporter + attr_reader :business_line + + CHANGE_HISTORY_COLUMNS = %w[].freeze + + def initialize(events = [], filters = {}) + @events = events + @filters = filters + end + + # :reek:FeatureEnvy + def as_csv + CSV.generate do |csv| + csv << format_filters_row + csv << CHANGE_HISTORY_COLUMNS + @events.each do |event| + csv << event.to_csv_row + end + end + end + + private + + def format_filters_row + @filters.to_a + end +end diff --git a/config/routes.rb b/config/routes.rb index 6532a38ac1c..dc771a5a03c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -257,6 +257,7 @@ patch :update_power_of_attorney end end + get "report", to: "decision_reviews#generate_report", on: :member, as: :report, format: false end match '/decision_reviews/:business_line_slug' => 'decision_reviews#index', via: [:get] diff --git a/spec/controllers/decision_reviews_controller_spec.rb b/spec/controllers/decision_reviews_controller_spec.rb index 1c13f1adb3f..3e59f478839 100644 --- a/spec/controllers/decision_reviews_controller_spec.rb +++ b/spec/controllers/decision_reviews_controller_spec.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +# This is needed for the generate_report action for a csv format since the testing environment does not eager load files +require Rails.root.join("app", "services", "claim_change_history", "change_history_reporter.rb") + describe DecisionReviewsController, :postgres, type: :controller do before do Timecop.freeze(Time.utc(2018, 1, 1, 12, 0, 0)) @@ -678,6 +681,92 @@ end end + describe "#generate_report" do + context "business-line-slug is not found" do + it "returns 404" do + get :generate_report, params: { business_line_slug: "foobar" } + + expect(response.status).to eq 404 + end + end + + context "user is not an org admin" do + it "returns unauthorized" do + get :generate_report, params: { business_line_slug: non_comp_org.url } + + expect(response.status).to eq 302 + expect(response.body).to match(/unauthorized/) + end + end + + context "user is an org admin" do + let(:generate_report_filters) do + { + report: "event", + events: ["claim_created"], + timing: { + end_date: Time.zone.now, + start_date: Time.zone.now, + range: 3 + }, + conditions: { + days_waiting: { + range: "between", + number_of_days: 3, + start_date: Time.zone.now, + end_date: Time.zone.now + }, + review_type: %w[higher_level_review supplemental_claim], + issue_type: ["Beneficiary Travel"], + disposition: %w[blank granted denied], + personnel: ["ACBAUERVVHAH"], + facility: [668, 669] + } + } + end + + before do + OrganizationsUser.make_user_admin(user, non_comp_org) + end + + it "renders the report generation template in HTML format" do + get :generate_report, format: :html, params: { business_line_slug: non_comp_org.url } + + expect(response).to have_http_status(:success) + expect(response.headers["Content-Type"]).to eq("text/html; charset=utf-8") + end + + it "renders the report generation action in a css format" do + get :generate_report, format: :csv, + params: { business_line_slug: non_comp_org.url, filters: generate_report_filters } + + expect(response.headers["Content-Type"]).to eq("text/csv") + expect(response.headers["Content-Disposition"]).to match(/^attachment; filename=\"nca-20180101.csv\"/) + end + + context "missing filter parameters" do + it "raises a param is missing error when filters are missing" do + get :generate_report, format: :csv, params: { business_line_slug: non_comp_org.url } + expect(response).to have_http_status(:bad_request) + expect(response.content_type).to eq("application/json") + json_response = JSON.parse(response.body) + expect(json_response["error"]).to eq("param is missing or the value is empty: filters") + end + end + + context "missing report parameter" do + it "raises a param is missing error when report type is missing from filters" do + params = { business_line_slug: non_comp_org.url, filters: generate_report_filters.except(:report) } + get :generate_report, format: :csv, params: params + expect(response).to have_http_status(:bad_request) + expect(response.content_type).to eq("application/json") + json_response = JSON.parse(response.body) + expect(json_response["error"]).to eq("param is missing or the value is empty: report") + end + end + end + end + def task_ids_from_response_body(response_body) response_body["tasks"]["data"].map { |task| task["id"].to_i } end From 3da326c66db62ef59197c297204b3ae29e180f8a Mon Sep 17 00:00:00 2001 From: Brandon Lee Dorner Date: Fri, 20 Oct 2023 14:31:27 -0500 Subject: [PATCH 03/40] APPEALS/30242 - Create `ReportPage` for Generate Task Report Epic (#19695) * Refactor NonComp pages for more design flexibility The NonComp pages have been refactored to allow for buttons outside of the `AppSegment` component. Throughout the app we are moving to this design pattern and want the flexibility to allow for that without any hacky css in these pages. This will be followed immediately by a commit for: Jira: https://jira.devops.va.gov/browse/APPEALS-30242 This commit will add the `ReportPage` which calls for a design where the buttons are located outside the `AppSegment` component. * Update the `decision_reviews` routing After discussion we decided it was better to do the exact routing through React Router on the frontend. Instead of adding in a new exact path for each new decision_review page we create we can point all decision_review pages to the index view which will display the main DecisionReview component on the frontend. From there we can get more granular and break up exact paths into separate components. * Create general structure for Generate Task Report For: https://jira.devops.va.gov/browse/APPEALS-30242 Create the basic structure for ReportPage that will be expanded on as more stories are completed. * prevent non-VHA business lines from accessing /report * ran migration, added test for report page, added controller testing gem * Revert "ran migration, added test for report page, added controller testing gem" This reverts commit 7065de898b71779850be0cc641b0bd5e26c44d91. * moved test to feature * fix test --------- Co-authored-by: Craig Reese --- .../decision_reviews_controller.rb | 1 + app/models/organizations/business_line.rb | 4 ++ app/models/organizations/vha_business_line.rb | 4 ++ .../app/nonComp/components/NonCompLayout.jsx | 23 ++++++ client/app/nonComp/index.jsx | 38 +++++----- client/app/nonComp/pages/ReportPage.jsx | 65 +++++++++++++++++ client/app/nonComp/pages/ReviewPage.jsx | 58 ++++++++------- client/app/nonComp/pages/TaskPage.jsx | 71 ++++++++++--------- .../test/app/nonComp/pages/ReportPage.test.js | 44 ++++++++++++ .../__snapshots__/ReportPage.test.js.snap | 52 ++++++++++++++ config/routes.rb | 4 +- .../decision_reviews_controller_spec.rb | 4 +- spec/feature/non_comp/reviews_spec.rb | 7 ++ 13 files changed, 293 insertions(+), 82 deletions(-) create mode 100644 client/app/nonComp/components/NonCompLayout.jsx create mode 100644 client/app/nonComp/pages/ReportPage.jsx create mode 100644 client/test/app/nonComp/pages/ReportPage.test.js create mode 100644 client/test/app/nonComp/pages/__snapshots__/ReportPage.test.js.snap diff --git a/app/controllers/decision_reviews_controller.rb b/app/controllers/decision_reviews_controller.rb index a8f53b56264..d58a090c0f5 100644 --- a/app/controllers/decision_reviews_controller.rb +++ b/app/controllers/decision_reviews_controller.rb @@ -65,6 +65,7 @@ def update end def generate_report + return render "errors/404" unless business_line.can_generate_claim_history return requires_admin_access_redirect unless business_line.user_is_admin?(current_user) respond_to do |format| diff --git a/app/models/organizations/business_line.rb b/app/models/organizations/business_line.rb index cc3a4f66ff0..d7a2a04799d 100644 --- a/app/models/organizations/business_line.rb +++ b/app/models/organizations/business_line.rb @@ -509,6 +509,10 @@ def locate_issue_type_filter(filters) end end end + + def can_generate_claim_history + false + end end require_dependency "vha_business_line" diff --git a/app/models/organizations/vha_business_line.rb b/app/models/organizations/vha_business_line.rb index af4b9a98a2f..f45b3c4ffb6 100644 --- a/app/models/organizations/vha_business_line.rb +++ b/app/models/organizations/vha_business_line.rb @@ -16,4 +16,8 @@ def tasks_query_type completed: "recently_completed" } end + + def can_generate_claim_history + true + end end diff --git a/client/app/nonComp/components/NonCompLayout.jsx b/client/app/nonComp/components/NonCompLayout.jsx new file mode 100644 index 00000000000..84c9a5bd7af --- /dev/null +++ b/client/app/nonComp/components/NonCompLayout.jsx @@ -0,0 +1,23 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import AppSegment from '@department-of-veterans-affairs/caseflow-frontend-toolkit/components/AppSegment'; + +const NonCompLayout = ({ buttons, children }) => { + return ( +
+ +
+ {children} +
+ + {buttons ? buttons : null} +
+ ); +}; + +NonCompLayout.propTypes = { + buttons: PropTypes.node, + children: PropTypes.node, +}; + +export default NonCompLayout; diff --git a/client/app/nonComp/index.jsx b/client/app/nonComp/index.jsx index bb5034bcff8..7b819ae117d 100644 --- a/client/app/nonComp/index.jsx +++ b/client/app/nonComp/index.jsx @@ -5,13 +5,13 @@ import NavigationBar from '../components/NavigationBar'; import { BrowserRouter } from 'react-router-dom'; import PageRoute from '../components/PageRoute'; import AppFrame from '../components/AppFrame'; -import AppSegment from '@department-of-veterans-affairs/caseflow-frontend-toolkit/components/AppSegment'; import { LOGO_COLORS } from '../constants/AppConstants'; import Footer from '@department-of-veterans-affairs/caseflow-frontend-toolkit/components/Footer'; import { FlashAlerts } from './components/Alerts'; import ReviewPage from './pages/ReviewPage'; import TaskPage from './pages/TaskPage'; +import ReportPage from './pages/ReportPage'; import { nonCompReducer, mapDataToInitialState } from './reducers'; class NonComp extends React.PureComponent { @@ -37,23 +37,25 @@ class NonComp extends React.PureComponent { defaultUrl={`/${this.props.serverNonComp.businessLineUrl}`} > - - {this.props.flash && } -
- - -
- + {this.props.flash && } + + +