Skip to content

Refactor Requests controller#5563

Open
stefannibrasil wants to merge 7 commits into
rubyforgood:mainfrom
hexdevs:refactor-requests-controller-5557
Open

Refactor Requests controller#5563
stefannibrasil wants to merge 7 commits into
rubyforgood:mainfrom
hexdevs:refactor-requests-controller-5557

Conversation

@stefannibrasil
Copy link
Copy Markdown
Contributor

@stefannibrasil stefannibrasil commented May 11, 2026

Resolves #5557

Description

In #5543, I added one more variable to this controller. I didn't want to refactor the controller since the PR was already too big and it's a good practice to refactor in a separate stream of work.

This PR moves instance variables used in the views to View objects. This keeps the controller slim and it's more maintainable to add methods to the view objects from now on.

Type of change

  • refactor

How Has This Been Tested?

  • Existing request and system tests pass
  • Added coverage for the business logic in the view objects being introduced
  • tested the requests index and view pages

Screenshots

Short walk trough of requests page:

requests.refactor.480.mov

Moving view instances to a View Object.
This was raising a bullet error:

GET /donations
AVOID eager loading detected
  Donation => [:product_drive_participant]
  Remove from your query: .includes([:product_drive_participant])
Moving view instances to a View Object to keep
the controller clean.

Add coverage for business logic
@stefannibrasil stefannibrasil force-pushed the refactor-requests-controller-5557 branch from d3dc2a5 to 1f08698 Compare May 11, 2026 23:39
@@ -31,7 +31,6 @@ def from_params(params:, organization:, helpers:)
.includes(:storage_location,
:donation_site,
:product_drive,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am happy to remove this change from this PR. I got here by looking at other view objects and saw the bullet's warning: 8089be6cb25bfff90506e634fc486dbaf6454e9b

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.

Refactor Requests controller: too many instance variables

1 participant