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

FFS-2322: Consolidate Pinwheel helpers into PinwheelDataHelper #422

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

tdooner
Copy link
Contributor

@tdooner tdooner commented Jan 28, 2025

Ticket

Resolves FFS-2322.

Changes

FFS-2322: Consolidate Pinwheel helpers into PinwheelDataHelper

This commit does three refactors:

  1. Moves all the methods from PaymentsHelper into ReportsHelper (since
    the separation between modules did not really match how we used them)
  2. Renames ReportsHelper to PinwheelDataHelper (since the helper was
    used in places unrelated to "reports")
  3. Moves the PinwheelDataHelper into "app/helpers" (since it is not an
    ActiveSupport::Concern as is conventional for classes in the
    "app/controllers/concerns" folder)

Context for reviewers

I tested by going through the flow locally with no issues.

Acceptance testing

  • No acceptance testing needed
    • This change will not affect the user experience (bugfix, dependency updates, etc.)
  • Acceptance testing prior to merge
    • This change can be verified visually via screenshots attached below or by sending a link to a local development environment to the acceptance tester
    • Acceptance testing should be done by design for visual changes, product for behavior/logic changes, or both for changes that impact both.
  • Acceptance testing after merge
    • This change is hard to test locally, so we'll test it in the demo environment (deployed automatically after merge.)
    • Make sure to notify the team once this PR is merged so we don't inadvertently deploy the unaccepted change to production. (e.g. :alert: Deploy block! @ffs-eng I just merged PR [#123] and will be doing acceptance testing in demo - please don't deploy until I'm finished!)

This commit does three refactors:
1. Moves all the methods from PaymentsHelper into ReportsHelper (since
   the separation between modules did not really match how we used them)
2. Renames `ReportsHelper` to `PinwheelDataHelper` (since the helper was
   used in places unrelated to "reports")
3. Moves the `PinwheelDataHelper` into "app/helpers" (since it is not an
   ActiveSupport::Concern as is conventional for classes in the
   "app/controllers/concerns" folder)
@tdooner tdooner changed the title FFS-2322: Encapsulate Report Data Presentation: Combine Related Helpers FFS-2322: Consolidate Pinwheel helpers into PinwheelDataHelper Jan 28, 2025
@tdooner tdooner merged commit effd2c6 into main Jan 28, 2025
14 checks passed
@tdooner tdooner deleted the td/ffs-2322-encapsulate-report-data-presentation- branch January 28, 2025 20:01
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.

2 participants