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

Medal Distribution tool #521

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Medal Distribution tool #521

wants to merge 8 commits into from

Conversation

coreobs
Copy link

@coreobs coreobs commented Apr 5, 2024

No description provided.

Copy link
Collaborator

@BenjaminSchaaf BenjaminSchaaf left a comment

Choose a reason for hiding this comment

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

This should also have tests.

config/routes.rb Outdated Show resolved Hide resolved
@league = League.includes(:tiebreakers).find(params[:id])
end

before_action only: [:medals] do
@league = League.includes(:tiebreakers).find(params[:league_id])
Copy link
Collaborator

Choose a reason for hiding this comment

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

With the route change this shouldn't be needed. :medals shouldn't be excepted from the previous before_action.

Copy link
Author

Choose a reason for hiding this comment

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

Unable to find league ID when I removed the exception and those two lines, re-adding them it works as expected.

image

app/controllers/leagues_controller.rb Show resolved Hide resolved
@coreobs
Copy link
Author

coreobs commented Apr 15, 2024

This should also have tests.

Will need to learn to do that, but can.

@seanbudd
Copy link

seanbudd commented May 3, 2024

Tests prepared here: coreobs#3

@coreobs
Copy link
Author

coreobs commented May 3, 2024

Tests prepared here: coreobs#3

Legend, thank you!

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.

3 participants