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

[16.0][ADD] website_recaptcha_v2_form #1076

Open
wants to merge 15 commits into
base: 16.0
Choose a base branch
from

Conversation

edescalona
Copy link

Adding recaptcha version 2 for the form snippets and also in the login, password reset and register forms.

…orm snippets and also in the login, password reset and register forms.

[FIX] Tests

[FIX] Ttests

[IMP] Add new tests

[IMP] Add new tests
@edescalona edescalona force-pushed the 16.0-add-website_recaptcha_v2_form branch from 74618c9 to 8b4f216 Compare December 6, 2024 16:27
Copy link

@Christian-RB Christian-RB left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, check my comments please and remember to squash your commits into one.

SIGN_UP_REQUEST_PARAMS.add("g-recaptcha-response")


class BinhexHome(Home):

Choose a reason for hiding this comment

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

Don't use company name in classes, functions nor variables, it should only appear in the manifest as author and the contributors readme.

Suggested change
class BinhexHome(Home):
class RecaptchaHome(Home):

Copy link
Author

Choose a reason for hiding this comment

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

Comment on lines 8 to 16
# --------------------------------------------------
# METHODS
# --------------------------------------------------
"""
Validating that the recaptcha sent is correct
@params:
kw: Data sent from the form
"""

Choose a reason for hiding this comment

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

I think you could remove this comments

Suggested change
# --------------------------------------------------
# METHODS
# --------------------------------------------------
"""
Validating that the recaptcha sent is correct
@params:
kw: Data sent from the form
"""

Comment on lines 1 to 3
* `BINHEX <https://www.binhex.cloud>`:

* Edilio Escalona Almira

Choose a reason for hiding this comment

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

Suggested change
* `BINHEX <https://www.binhex.cloud>`:
* Edilio Escalona Almira
- `Binhex <https://binhex.cloud>`__:
- Edilio Escalona Almira

Comment on lines +3 to +16
#. Go to **Website > Configuration > Settings**.

#. Search 'reCAPTCHA v2' option.

![reCaptcha v2](../static/src/img/readme/img.png)

#. Click the link [Get reCAPTCHA v2 keys](https://www.google.com/recaptcha/admin)
to generate the keys needed to use the recaptcha.

![Get reCAPTCHA v2 keys](../static/src/img/readme/img_1.png)

#. Fill in the generated **Site Key** and **Secret key**.

![reCAPTCHA v2 keys](../static/src/img/readme/img_2.png)

Choose a reason for hiding this comment

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

I think it would be better to create a link to the original README in website_recaptcha_v2 instead of duplicating the imgs and information here.

Copy link
Author

Choose a reason for hiding this comment

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

ok

Choose a reason for hiding this comment

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

Oh sorry I just saw that the website_recaptcha_v2 module doesn't contains those imgs. It's ok the way it is.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @Christian-RB , ok no problem, thanks for your comments.

# This is the 1st commit message:

[IMP] Add tests

# This is the commit message OCA#2:

[IMP] Add tests

# This is the commit message OCA#3:

[FIX] Tests

# This is the commit message OCA#4:

[FIX] Tests

[ADD] Tests

[FIX] Tests

[FIX] Tests

[ADD] Tests

[ADD] Tests

[FIX] Tests

[ADD] Tests

[ADD] Tests

[REM] Tests

[ADD] Tests

[ADD] Tests

[ADD] Tests
@edescalona edescalona force-pushed the 16.0-add-website_recaptcha_v2_form branch from cf7f6d2 to e07469c Compare December 24, 2024 03:19
Copy link

@jorgeglez1990 jorgeglez1990 left a comment

Choose a reason for hiding this comment

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

Greetings,
It has been functionally tested, and it works correctly except when, on /web/login, we log in without verifying the reCAPTCHA, which results in a 500: Internal Server Error.

image

@edescalona
Copy link
Author

Greetings, It has been functionally tested, and it works correctly except when, on /web/login, we log in without verifying the reCAPTCHA, which results in a 500: Internal Server Error.

image

hello @jorgeglez1990 I'll check it out now, thanks for your cooperation.

]
)
)
site_key_old = views_recaptcha.arch_db.split('data-sitekey="')
Copy link

@nobuQuartile nobuQuartile Jan 7, 2025

Choose a reason for hiding this comment

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

Thank you for your contribution.
Let me comment on the minor error I had when I tried to use the module you created.

When I tried to open the Setting app with this module on runboat, I got the following error.
It's because search() result ir.ui.view() is empty
I think just doing an if branch will solve this.
What do @edescalona think?

  File "/opt/odoo/odoo/http.py", line 1653, in _serve_db
    return service_model.retrying(self._serve_ir_http, self.env)
  File "/opt/odoo/odoo/service/model.py", line 133, in retrying
    result = func()
  File "/opt/odoo/odoo/http.py", line 1680, in _serve_ir_http
    response = self.dispatcher.dispatch(rule.endpoint, args)
  File "/opt/odoo/odoo/http.py", line 1884, in dispatch
    result = self.request.registry['ir.http']._dispatch(endpoint)
  File "/opt/odoo/addons/website/models/ir_http.py", line 237, in _dispatch
    response = super()._dispatch(endpoint)
  File "/opt/odoo/odoo/addons/base/models/ir_http.py", line 154, in _dispatch
    result = endpoint(**request.params)
  File "/opt/odoo/odoo/http.py", line 734, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 42, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 33, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 468, in call_kw
    result = _call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 453, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/opt/odoo/odoo/models.py", line 6633, in onchange
    record._onchange_eval(name, field_onchange[name], result)
  File "/opt/odoo/odoo/models.py", line 6344, in _onchange_eval
    method_res = method(self)
  File "/mnt/data/odoo-addons-dir/website_recaptcha_v2_form/models/res_config_settings.py", line 19, in onchange_recaptcha_v2_site_key
    site_key_old = views_recaptcha.arch_db.split('data-sitekey="')
AttributeError: 'bool' object has no attribute 'split'

The above server error caused the following client error:
null 
Suggested change
site_key_old = views_recaptcha.arch_db.split('data-sitekey="')
if views_recaptcha:
site_key_old = views_recaptcha.arch_db.split('data-sitekey="')

Copy link
Author

Choose a reason for hiding this comment

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

Hi @nobuQuartile , if it could be an option, I will review it and let you know as soon as possible, thanks for yout comment.

@edescalona
Copy link
Author

Hi @nobuQuartile , I solved the error you mentioned. When I can, please check and confirm. Thanks.

_inherit = "res.config.settings"

@api.onchange("recaptcha_v2_site_key")
def onchange_recaptcha_v2_site_key(self):
Copy link
Member

Choose a reason for hiding this comment

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

I thought it would be more appropriate to extend the write() method instead of using onchange here.

Choose a reason for hiding this comment

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

@edescalona Excuse me. There was an error I pointed out yesterday, which you corrected, but this way of doing things may be more appropriate: it is better to stop unnecessary execution occurring on change than to branch on IF. Sorry for making so many corrections.

@nobuQuartile
Copy link

Hi @nobuQuartile , I solved the error you mentioned. When I can, please check and confirm. Thanks.

Hi @edescalona, I checked and confirmed the problem is solved at the ruboat. Thanks.

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.

5 participants