345/bruteforce/event not deleteable after start #418
Conversation
ghost
assigned 
| expect(page).not_to have_css('a', :text => I18n.t('helpers.links.destroy')) | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
If we're working with cucumber now this test isn't needed anymore. Cucumber replaces all our feature tests.
spec/models/ability_spec.rb
Outdated
| @@ -145,7 +145,7 @@ | |||
| end | |||
|
|
|||
| it 'should allow users to crud events they created' do | |||
| event = Event.new(owner: @user) | |||
| event = FactoryBot.create(:event, :has_dates, owner_id: @user.id) | |||
There was a problem hiding this comment.
This is awkward because it's not really obvious why the even needs to have dates, I'd rather see this in a sperate ability test only for Tournaments and leagues, because rankinglists don't have any dates. Also adding another test with the other case (when you cannot delete the event) would be a good idea imo.
There was a problem hiding this comment.
Users can only delete events now under certain conditions so this test has to be changed. But I agree that the event object having a date here is weird. I will change this test and create a separate one for the changed delete ability.
features/events/delete_event.feature
Outdated
| Scenario: Admin can delete event any time, organizer does not | ||
| Given an event that has started | ||
| Then the admin should be able to delete it | ||
| And the organizer should not be able to delete it |
There was a problem hiding this comment.
Implement the steps for this please :) If you need help don't be afraid to ask, although my knowledge about cucumber is also still quite limited.
app/models/event.rb
Outdated
| @@ -55,6 +56,10 @@ def deadline_has_passed? | |||
| deadline < Date.current | |||
| end | |||
|
|
|||
| def startdate_has_passed? | |||
| startdate < Date.current | |||
| end | |||
There was a problem hiding this comment.
This method is only used in your tests, nowhere else. Is that intentional? I don't really see how this adds the expected behaviour yet.
There was a problem hiding this comment.
This is from an earlier implementation that is actually not needed anymore.
|
Can you merge this with dev, so we can push it into PO Review? :) |
closes #345