feat: Addition of fields for reCAPTCHA #6044
Conversation
app/models/setting.py
Outdated
| # CAPTCHA | ||
| # | ||
|
|
||
| #Google reCAPTCHA |
There was a problem hiding this comment.
block comment should start with '# '
|
@iamareebjamal @CosmicCoder96 @niranjan94 Please Review |
Codecov Report
@@ Coverage Diff @@
## development #6044 +/- ##
===============================================
+ Coverage 66.15% 66.17% +0.02%
===============================================
Files 285 285
Lines 14060 14069 +9
===============================================
+ Hits 9301 9310 +9
Misses 4759 4759
Continue to review full report at Codecov.
|
abhinavk96
left a comment
abhinavk96
left a comment
There was a problem hiding this comment.
Why 2 migration files? Please make them in a single file.
iamareebjamal
left a comment
iamareebjamal
left a comment
There was a problem hiding this comment.
There are many captcha services, make it explicit, -> is_google_captcha_enabled, google_captcha_site_key
niranjan94
left a comment
niranjan94
left a comment
There was a problem hiding this comment.
Rename captcha to recaptcha. Incase we start to support multiple captcha providers in the future
app/models/setting.py
Outdated
| gs_key=None, | ||
| gs_secret=None, | ||
| gs_bucket_name=None, | ||
| is_google_recaptcha_enabled=False,google_recaptcha_site_key=None,google_recaptcha_secret_key=None, |
|
@iamareebjamal @CosmicCoder96 @niranjan94 Please have a look |
migrations/versions/7388e7504caa_.py
Outdated
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.add_column('settings', sa.Column('google_recaptcha_secret_key', sa.String(), nullable=True)) | ||
| op.add_column('settings', sa.Column('google_recaptcha_site_key', sa.String(), nullable=True)) | ||
| op.add_column('settings', sa.Column('is_google_recaptcha_enabled', sa.Boolean(), nullable=False)) |
There was a problem hiding this comment.
This will fail as there's no default set
|
Solve hound issues |
|
@iamareebjamal @CosmicCoder96 @niranjan94 Please review |
|
Please first review the PRs yourself by seeing the changes |
There was a problem hiding this comment.
About the boolean columns, https://github.com/fossasia/open-event-frontend/blob/development/app/models/setting.js#L46,
@kushthedude They are usually computed properties which can be toggled on the frontend, if that helps. They were implemented in the previous gateways too, such as is_stripe_activated &is_paypal_activated which are toggle buttons. Used to disable the modules temporarily.
{{ui-checkbox class='toggle' checked=isCheckedAliPay onChange=(action (mut isCheckedAliPay))}}
|
@iamareebjamal Did the requested changes, Please review. |
migrations/versions/232b13d4e86a_.py
Outdated
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.add_column('settings', sa.Column('google_recaptcha_secret', sa.String(), nullable=True)) | ||
| op.add_column('settings', sa.Column('google_recaptcha_site', sa.String(), nullable=True)) | ||
| op.add_column('settings', sa.Column('is_google_recaptcha_enabled', sa.Boolean(), nullable=False)) |
There was a problem hiding this comment.
@iamareebjamal i specified the default value above , its not showing in migration file , what should i do?
There was a problem hiding this comment.
See previous PRs where default has been supplied
|
@iamareebjamal Please Review now , Added the |
6 participants
Fixes #6043
Checklist
developmentbranch.Short description of what this resolves:
Changes proposed in this pull request: