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

Organisers can create new Sponsors without adding in number of coache… #1803

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joaoguiIherme
Copy link
Contributor

…s/students but it will 404

This PR fixs issue #1794.
Also fixes some bugs in tests.

@KimberleyCook
Copy link
Contributor

Hey @joaoguiIherme thank you so much for this PR. One of the codebar team will try to review it as soon as possible

@KimberleyCook
Copy link
Contributor

@matyikriszta when you get some time could you review this PR please?

@@ -7,7 +7,7 @@
workshop: workshop,
sponsor: Fabricate(:sponsor,
seats: transients[:student_count] || 10,
number_of_coaches: transients[:coach_count || 10]),
number_of_coaches: transients[:coach_count] || 10),
Copy link
Contributor

Choose a reason for hiding this comment

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

@joaoguiIherme do we need the closing parenthesis at the end of this line? I think the next line (11) closes the parenthesis opened on line 8.

@@ -34,6 +34,8 @@ class Sponsor < ActiveRecord::Base
validates :level, inclusion: { in: Sponsor.levels.keys }
validates :name, :address, :avatar, :website, :level, presence: true
validate :website_is_url, if: :website?
validates :number_of_coaches, presence: true, numericality: { greater_than_or_equal_to: 0, only_integer: true }
validates :seats, presence: true, numericality: { greater_tha_or_equal_to: 0, only_integer: true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
validates :seats, presence: true, numericality: { greater_tha_or_equal_to: 0, only_integer: true }
validates :seats, presence: true, numericality: { greater_than_or_equal_to: 0, only_integer: true }

@matyikriszta
Copy link
Contributor

Sorry for the delay on reviewing this @joaoguiIherme, I left two small comments, but overall it looks good. Thanks for fixing up the tests too.

@matyikriszta
Copy link
Contributor

@joaoguiIherme will you be able to make the updates I requested or I'd be happy to make them myself, I'd be keen to get this PR merged in.

@github-actions
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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