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

Add insiders settings #7151

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Add insiders settings #7151

wants to merge 23 commits into from

Conversation

dem4ron
Copy link
Member

@dem4ron dem4ron commented Nov 8, 2024

  • Add InsiderBenefitsForm
  • Add component
  • Add migration
  • Progress
  • Sort things out better
  • Permit editing hide thingy
  • Pass down props
  • Disable form for non insiders
  • Remove unused import
  • Create an insiders settings page
  • Add new components
  • Add API endpoints

Comment on lines 6 to 7
return unless current_user.insider?
return if current_user.bootcamp_affiliate_coupon_code.present?
Copy link
Member

Choose a reason for hiding this comment

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

I think these are better places into the command below (the one is already there and I'll add the insiders guard too). I prefer keeping all the logic in one place.

Also, you can't just "return" here as you need to render json, so it should be return render(json: {}) to be a valid response for a JSON API. You could also raise an error of some description like raise "Not insider" which would get caught by your block, but as we have standardised error messages in the API, that's probably not ideal.

So I would make this method:

code = User::GenerateBootcampAffiliateCouponCode.(current_user)
render json: { coupon_code: code }

or if you want to raise an error if the code is missing:

code = User::GenerateBootcampAffiliateCouponCode.(current_user)
if code 
  render json: { coupon_code: code }
else
  render_403(:could_not_generate_coupon_code)

And then you need to add translations for could_not_generate_coupon_code (see config/locales/api/en.yml)

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.

2 participants