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

Perf: Minor dashboard index page refactor #4874

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

coalest
Copy link
Collaborator

@coalest coalest commented Dec 20, 2024

Doesn't resolve any issue.

Description

I noticed an N+1 on this page, so I went to fix it.
Then I noticed an unused instance variable and removed one extra query regarding outstanding requests.

Type of change

  • Internal refactor

How Has This Been Tested?

Behavior hasn't changed, so just the automated test suite.

@coalest coalest changed the title Perf: Refactor dashboard index page Perf: Minor dashboard index page refactor Dec 20, 2024
@@ -4,9 +4,13 @@ class DashboardController < ApplicationController

def index
@org_stats = OrganizationStats.new(current_organization)
@total_inventory = current_organization.total_inventory
Copy link
Collaborator Author

@coalest coalest Dec 20, 2024

Choose a reason for hiding this comment

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

This instance variable doesn't appear to be used anywhere.

How removing this line feels:
Screenshot from 2024-12-20 13-18-13

@coalest
Copy link
Collaborator Author

coalest commented Dec 20, 2024

I believe there is also an N+1 regarding partners awaiting review on this page which could be fixed.

@dorner
Copy link
Collaborator

dorner commented Dec 20, 2024

Thanks for this!

@dorner dorner merged commit 22d5a14 into rubyforgood:main Dec 20, 2024
11 checks passed
Copy link
Contributor

@coalest: Your PR Perf: Minor dashboard index page refactor is part of today's Human Essentials production release: 2024.12.22.
Thank you very much for your contribution!

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