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 appeal state association #21947

Closed
wants to merge 55 commits into from
Closed

Conversation

ThorntonMatthew
Copy link
Contributor

Resolves Jira Issue Title

Description

Please explain the changes you made here.

Acceptance Criteria

  • Code compiles correctly

Testing Plan

  1. Go to Jira Issue/Test Plan Link or list them below
  • For feature branches merging into master: Was this deployed to UAT?

Frontend

User Facing Changes

  • Screenshots of UI changes added to PR & Original Issue
BEFORE AFTER

Storybook Story

For Frontend (Presentation) Components

  • Add a Storybook file alongside the component file (e.g. create MyComponent.stories.js alongside MyComponent.jsx)
  • Give it a title that reflects the component's location within the overall Caseflow hierarchy
  • Write a separate story (within the same file) for each discrete variation of the component

Backend

Database Changes

Only for Schema Changes

  • Add typical timestamps (created_at, updated_at) for new tables
  • Update column comments; include a "PII" prefix to indicate definite or potential PII data content
  • Have your migration classes inherit from Caseflow::Migration, especially when adding indexes (use add_safe_index) (see Writing DB migrations)
  • Verify that migrate:rollback works as desired (change supported functions)
  • Perform query profiling (eyeball Rails log, check bullet and fasterer output)
  • For queries using raw sql was an explain plan run by System Team
  • Add appropriate indexes (especially for foreign keys, polymorphic columns, unique constraints, and Rails scopes)
  • Run make check-fks; add any missing foreign keys or add to config/initializers/immigrant.rb (see Record associations and Foreign Keys)
  • Add belongs_to for associations to enable the schema diagrams to be automatically updated
  • Document any non-obvious semantics or logic useful for interpreting database data at Caseflow Data Model and Dictionary

Integrations: Adding endpoints for external APIs

  • Check that Caseflow's external API code for the endpoint matches the code in the relevant integration repo
    • Request: Service name, method name, input field names
    • Response: Check expected data structure
    • Check that calls are wrapped in MetricService record block
  • Check that all configuration is coming from ENV variables
    • Listed all new ENV variables in description
    • Worked with or notified System Team that new ENV variables need to be set
  • Update Fakes
  • For feature branches: Was this tested in Caseflow UAT

Best practices

Code Documentation Updates

  • Add or update code comments at the top of the class, module, and/or component.

Tests

Test Coverage

Did you include any test coverage for your code? Check below:

  • RSpec
  • Jest
  • Other

Code Climate

Your code does not add any new code climate offenses? If so why?

  • No new code climate issues added

Monitoring, Logging, Auditing, Error, and Exception Handling Checklist

Monitoring

  • Are performance metrics (e.g., response time, throughput) being tracked?
  • Are key application components monitored (e.g., database, cache, queues)?
  • Is there a system in place for setting up alerts based on performance thresholds?

Logging

  • Are logs being produced at appropriate log levels (debug, info, warn, error, fatal)?
  • Are logs structured (e.g., using log tags) for easier querying and analysis?
  • Are sensitive data (e.g., passwords, tokens) redacted or omitted from logs?
  • Is log retention and rotation configured correctly?
  • Are logs being forwarded to a centralized logging system if needed?

Auditing

  • Are user actions being logged for audit purposes?
  • Are changes to critical data being tracked ?
  • Are logs being securely stored and protected from tampering or exposing protected data?

Error Handling

  • Are errors being caught and handled gracefully?
  • Are appropriate error messages being displayed to users?
  • Are critical errors being reported to an error tracking system (e.g., Sentry, ELK)?
  • Are unhandled exceptions being caught at the application level ?

Exception Handling

  • Are custom exceptions defined and used where appropriate?
  • Is exception handling consistent throughout the codebase?
  • Are exceptions logged with relevant context and stack trace information?
  • Are exceptions being grouped and categorized for easier analysis and resolution?

jefftmarks and others added 30 commits December 18, 2023 10:55
* APPEALS-43706 macros added to each model file for the polymorphic relationship to be setup.

* APPEALS-43706 migration made and yet to be run.

* APPEALS-43706 migrations ran and expected columns and indexes are in table.

* Fix some spec failures unrelated to this branch

* Limit schema diff

* Add a missing tab

* Fix more unrelated failures. Also prevent ACD lever seeds from overwriting spec output

* Fix last unrelated failure.

* APPEALS-43706 linting issues in va_notify_service fixed.

---------

Co-authored-by: Jonathan Cohen <cohen_jonathan@bah.com>
Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
* Quarterly Notification Job refactoring

* APPEALS-43598 Fix lint issues for QuarterlyNotificationsJob refactor

* APPEALS-43598 Fix db schema.rb file

* APPEALS-43598 Fix schema.rb file

* APPEALS-43598 Refactoring QuarterlyNotificationsJob for readability

* APPEALS-43598 Refactoring QuarterlyNotificationsJob and removing extra methods

* [APPEALS-43598] Update method name

---------

Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com>
…terans (#21481)

* APPEALS-44007: Writing tests to ensure adding the 'failure due to deceased' information is properly set and added to the db

* updates to pr

* pushing changes for tonight

* updating spec file to handle enabling and disabling feature flags

* Update format_message_status

* Lint roll

* fixing conflicts

* Updates to send notification spec

* Fix one of the legacy test cases

* Remove accidental push

* Fix remaining test

---------

Co-authored-by: Noah Hansen <nhansen@Noahs-MacBook-Pro.local>
Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com>
…ter All eFolder Upload Attempts (#21557)

* Cleanup files after uploading to VBMS

* Only delete file if it exists

* Resolve linter issue

* Update specs

* Trigger CodeClimate

---------

Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
… New Notifications (#21575)

* APPEALS-45346: Populate Polymorphic Association Columns When Creating New Notifications

* fixing some errors

* updating factory

* updating tests

* updating tests and factory

* rubocop

---------

Co-authored-by: Noah Hansen <nhansen@Noahs-MacBook-Pro.local>
Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com>
…es (#21596)

* APPEALS-43714: Refactor VANotify Modules to Not Rely on Class Variables

* pushing changes

* updating tests

* rubocop

* rubocop'n

* deleting repeat constants

* removing file in favor of EVENT_TYPE_FILTERS.json

* removing instances of using VA_NOTIFY_TEMPLATE_NAMES and using EVENT_TYPE_FILTERS instead

* replacing all instances of strings with constants

* fixing bug

* rubocop'n

* rubocop'n

* rubocop

---------

Co-authored-by: Noah Hansen <nhansen@Noahs-MacBook-Pro.local>
Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com>
)

Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
…tificationsJob` to Decrease Runtime (#21643)

* Group notification types as scopes

* Add new job

* Fix typo

* Add back batching

* Batch send messages

* Multithread job queueing

* Reconfigure bulk enqueueing to utilize send_notifications.fifo queue

* Update the comment on QuaterlyNotificationsJob#perform

to adhere to the RDoc standard

* Use constants for template name

* Provide a more descriptive var name

* Remove unused env var

* Fix linting issues

* Prevent error from being thrown early in init job

* Add RDoc comments to appeal_state

* Add rdoc comment to init job

* Add another rdoc comment to the init job

* Use a more specific error class whenever an appeal

cannot be located within the init job

* Add a more specific error to CaseflowJob#enqueue_batch_of_jobs

* Add test for SendNotificationJob.queue_name_suffix

* Update QuarterlyNotificationsJob tests

* Update appeal_state_spec.rb to cover new scopes

* Add spec file for NotificationInitializationJob

* Add CaseflowJob specs

* Add CaseflowJob specs

* Address rubocop issue (by ignoring it)

* Kicking off CodeClimate

* Fix some lint issues

* Freeze constant

---------

Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
ThorntonMatthew and others added 25 commits May 16, 2024 16:09
* APPEALS-43599 Refactoring SendNotificationJob

* APPEALS-43599 Remove comments

* APPEALS-43599 Suppress code smells from reek

* APPEALS-43599 Suppress code smells from reek
* APPEALS-43727 Branch created. Extracted all the 'when' conditions in the case statement within appellant_notification.rb update_appeal_state method.

* APPEALS-43727 appellant_notification_service is created, logic from crowded method in appellant_notificaion.rb placed in newly created file, allowing redability.

* APPEALS-43727 added logic to appeal_state.rb, appellant_notification.rb updated with less logic, readable code through well named methods.

* APPEALS-43727 commit before merge from teammates work

* APPEALS-43727 removed freeze method on default status hash in appeal_state.rb. removed some rubocop disabling in appellant_notificaion.rb

* APPEALS-43727 extracted more logic from the appellant_notification module and placed it into the appeal_state class. Also got rid of rubocop declarations in app_notif and app_state as well. finally removed feature_toggle calls in appell_notif.

* APPEALS-43727 tests passing, code works

* APPEALS-43727 adding tests to appeal_state_spec to test functionality

* APPEALS-43727 rspec tests in appeal_state.

* APPEALS-43727 tests pass.

* Fixing code climate duplicate code issue

* fixing case statement

* code climate things

* rubocop

* APPEALS-43727 work completed.

* APPEALS-43727 PR comments applied. tests not in a working state.

* APPEALS-43727 PR comments applied. appeal_state_spec in working state.

* APPEALS-43727 PR comments applied. appellant_notification_spec.rb all green.

* Adjust spacing of comments in appeal_state.rb

* Utilize #external_id method over ternary

* Swap or conditional for blank?

* Fix another whitespace gap

* APPEALS-43727 PR comments applied. again after reset.

* APPEALS-43727 PR comments applied. flipped conditionals in appeal_state.rb

* Update app/models/concerns/appeal_concern.rb

* APPEALS-43727 code/test edits

* APPEALS-43727 bulk_task_reasssignment_spec reverted to orginal state, with the exception of an expectation that was on line 98

* APPEALS-43727 lint fix

---------

Co-authored-by: Jonathan Cohen <cohen_jonathan@bah.com>
Co-authored-by: Noah Hansen <nhansen@Noahs-MacBook-Pro.local>
Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com>
* APPEALS-46179 Create JobExecutionTimes table

* APPEALS-46179 Add comment

* APPEALS-46179 Update schema
* APPEALS-46193 Add record to JobExecutionTime table when job is performed

* APPEALS-46193 Updating JobExecutionTimes table with an upsert query

* APPEALS-46193 Ignore JobExecutionTimes for certain jobs

* APPEALS-46193 Update ignore_job_execution_time? method

* APPEALS-46193 Adding tests
* Add conditional for FOIA and Privacy Act mail

task to receives 'Privacy Act Pending' notifications

* Update spec

* Adjust some specs

* Add missing param

---------

Co-authored-by: Matthew Thornton <ThorntonMatthew@users.noreply.github.com>
* initial commit

* refactoring methods to have constants instead of methods

* adding comments back in

---------

Co-authored-by: Noah Hansen <nhansen@Noahs-MacBook-Pro.local>
Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com>
Co-authored-by: nhansen3 <noah.hansen1323@gmail.com>
Copy link

codeclimate bot commented Jun 18, 2024

Code Climate has analyzed commit 497acda and detected 0 issues on this pull request.

View more on Code Climate.

@ThorntonMatthew ThorntonMatthew deleted the fix_appeal_state_association branch June 18, 2024 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants