Organisers can create new Sponsors without adding in number of coache… #1803
Conversation
…s/students but it will 404
|
Hey @joaoguiIherme thank you so much for this PR. One of the codebar team will try to review it as soon as possible |
|
@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), | |||
There was a problem hiding this comment.
@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 } | |||
There was a problem hiding this comment.
| 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 } |
|
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. |
|
@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. |
|
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. |
|
@matyikriszta I tried checking out this branch to add the changes and I'm not able to I assume because it was created years ago. I suggest someone else tries checking out the branch |
|
@davidmillen50 I've been able to check out the branch (and can show you how if you'd like). To fix conflicts and make @matyikriszta's suggested changes I think that I, or someone else, will need to create a new PR from a branch in their own fork? |
|
Thanks @mikej if you can send me the git commands I'll have a go. Regarding the edits: I would hope we can make changes to this branch directly without creating a new PR |
…s/students but it will 404
This PR fixs issue #1794.
Also fixes some bugs in tests.