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

232 role as event option #235

Merged
merged 12 commits into from
Jul 15, 2024
Merged

232 role as event option #235

merged 12 commits into from
Jul 15, 2024

Conversation

beingmattlevy
Copy link
Collaborator

Adding event option to make role collection optional

add event model param for require_role
add event form checkbox for require role
add ticket request check for require role
@beingmattlevy beingmattlevy linked an issue Jul 11, 2024 that may be closed by this pull request
font-titillium has been removed from homebrew
github is colliding with installed github
Copy link
Collaborator

@kigster kigster left a comment

Choose a reason for hiding this comment

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

We should already have a ticket request submission spec.

I would like to see this field being sent as both false and true and verify that it saves the proper value into the DB.

I worry that the checkbox will by the default be unchecked, and that submitting a form with it unchecked would save it as a true value (which is the default).

Perhaps it's not doing that, but adding this to the spec would ensure that for certain.

Brewfile Show resolved Hide resolved
app/models/event.rb Show resolved Hide resolved
app/views/ticket_requests/_form.html.haml Show resolved Hide resolved
@beingmattlevy
Copy link
Collaborator Author

@kigster the events controller spec now has a test to ensure that the require role is true by default.
ticket requests controller spec is not the right place for the event require role db save / fetch test.
need to update events controller spec to do an update where this is turned off and then validated the right thing is in the db.

@beingmattlevy
Copy link
Collaborator Author

@kigster ready for re-review

@beingmattlevy beingmattlevy merged commit 4fb568c into main Jul 15, 2024
3 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.

role-as-event-option
2 participants