-
Notifications
You must be signed in to change notification settings - Fork 0
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
family life gift card app #5
base: initial
Are you sure you want to change the base?
Conversation
able to generate now and proper auth on the admin for issuances - no editing issuance after gift cards are created
various other fixes and progress towards issuing and displaying gift cards
store gift card type on each gift card
we don't use sidekiq but it's already set up in terraform and could be used in the future, so it's good to have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continued review, still mostly config related
…-cards into rails_7_rebase
since we're still creating official dbs
various config change suggestions Co-authored-by: Mark Knutsen <[email protected]>
fix filter on column that doesn't exist any more remove some sidekiq refs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor nits; otherwise, approved!
@@ -0,0 +1,8 @@ | |||
class MonitorsController < ApplicationController | |||
# skip_before_action :require_login, raise: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting; that skip isn't needed? In which case this comment can go away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I copied that from one-app but campaign forms doesn't have it either. We don't need it here because there is no login_required filter, because the site is only active admin stuff, and active admin has its own login filter.
app/views/login/error.html.erb
Outdated
@@ -0,0 +1,10 @@ | |||
<!-- because of https://github.com/omniauth/omniauth/wiki/Resolving-CVE-2015-9284, we need to do a post to /auth/oktaoauth, so this page will do it seamlessly without being visible to the user --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment relevant here? Looks to be copied from the new.html.erb
tweak data info comment to not have has_access boolean Co-authored-by: Mark Knutsen <[email protected]>
…-cards into rails_7_rebase
No description provided.