Skip to content

IIRR-32: Add feature to filter the archived list by low success rate#76

Merged
JuanVqz merged 1 commit into
mainfrom
IIRR-32-add-low-success-rate-filter
May 18, 2026
Merged

IIRR-32: Add feature to filter the archived list by low success rate#76
JuanVqz merged 1 commit into
mainfrom
IIRR-32-add-low-success-rate-filter

Conversation

@arielj
Copy link
Copy Markdown
Contributor

@arielj arielj commented May 18, 2026

This PR adds a new filter for the Archived table to only show puzzles with a success answer rate lower than or equal to 80%.

I also updated the correct_answer_percentage to reuse the already loaded association when possible to avoid extra DB queries.

@arielj arielj requested a review from JuanVqz May 18, 2026 13:33
Comment thread test/fixtures/puzzles.yml
explanation: "This is a test puzzle"
link: "https://example.com"
state: "approved"
sent_at: <%= 1.day.ago %>
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.

approved puzzles shouldn't have a sent_at value, they are not sent yet

Copy link
Copy Markdown
Member

@JuanVqz JuanVqz left a comment

Choose a reason for hiding this comment

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

It is working; left some comments, but they are optional, we can fix them later if needed.

Comment thread app/models/puzzle.rb
end

def self.only_low_success_rate
includes(:answers).to_a.select { |ans| ans.correct_answer_percentage <= 80 }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I propose changing from ans to p because is a puzzle sending correct_answer_percentage not answer.

Suggested change
includes(:answers).to_a.select { |ans| ans.correct_answer_percentage <= 80 }
includes(:answers).to_a.select { |p| p.correct_answer_percentage <= 80 }

And one question: Is this equivalent to a scope or should we use the scope syntax here?

@JuanVqz JuanVqz merged commit 908d603 into main May 18, 2026
4 checks passed
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