Skip to content

[Admin] Filter active teenagers within the last 30 days on the team page #10615

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

scooterthedev
Copy link
Member

@scooterthedev scooterthedev commented Jun 10, 2025

Summary of the problem

https://hackclub.slack.com/archives/C068U0JMV19/p1749575821982219 + @leowilkin ;)

Describe your changes

Added a filter for admins only to see active teenagers for a specific org on the teams page.

@scooterthedev scooterthedev requested review from a team as code owners June 10, 2025 22:24
@@ -274,7 +276,11 @@ def team

@all_positions = @event.organizer_positions
.joins(:user)
@all_positions = @all_positions.where(role: @filter) if @filter
if @filter == "active_teens"
@all_positions = @all_positions.select { |op| op.user.teenager? && op.user.active? } # select if user is a teenager and active (stole from the other code ;))
Copy link
Member

Choose a reason for hiding this comment

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

I think this will break as select forces the SQL query to run and then does the filtering in memory, on Line 284 though we seem to be adding more to the SQL query

Copy link
Member Author

Choose a reason for hiding this comment

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

In my testing, nothing broke. but yah, that doesn't seem good. Any suggestions on how to fix it?

Copy link
Member

Choose a reason for hiding this comment

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

I think move it to after L284

@scooterthedev scooterthedev requested a review from sampoder June 20, 2025 05:25
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