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.

Sign up for GitHub

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

Jump to bottom

[14.0][MIG] auth_signup_verify_email #270

Merged
merged 17 commits into from
Jan 21, 2022
Merged

[14.0][MIG] auth_signup_verify_email #270

merged 17 commits into from
Jan 21, 2022

Conversation

em230418
Copy link
Contributor

@em230418 em230418 commented Apr 29, 2021 •
edited
Loading

In summary:

@em230418 em230418 mentioned this pull request Sep 1, 2021
Closed
17 tasks
@pedrobaeza pedrobaeza added this to the 14.0 milestone Oct 6, 2021
@pedrobaeza pedrobaeza mentioned this pull request Oct 6, 2021
Closed
@pedrobaeza
Copy link
Member

Travis is red

@em230418 em230418 force-pushed the 14.0 branch 2 times, most recently from c34d1d7 to 9e54359 Compare October 6, 2021 10:18
@em230418
Copy link
Contributor Author

em230418 commented Oct 7, 2021

Travis is green now, should I cover lines reported by codecov?

ernestotejeda
ernestotejeda approved these changes Jan 20, 2022
View reviewed changes
Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

Code and functional review 👍🏼

@ernestotejeda
Copy link
Member

ping @Tardo @pedrobaeza

@pedrobaeza
Copy link
Member

/ocabot migration auth_signup_verify_email

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-270-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jan 21, 2022
@OCA-git-bot
Merge PR #270 into 14.0
9d1607f 
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-270-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

yajo and others added 12 commits January 21, 2022 08:52
@yajo @pedrobaeza
[9.0][MIG][auth_signup_verify_email] Migration. (OCA#436)
b5e3151 
* Add module auth_signup_verify_email.

* Import module following guidelines.

* README typos.

* OCA Transbot updated translations from Transifex

* Credit creator.

* author name correction

* [9.0][MIG][auth_signup_verify_email] Migration.

Migrate to v9.

* [FIX] auth_signup_verify_email: Python library requirement

* Add tests, fix xml tags, fix credits.

* Fix test.
@sharkende @pedrobaeza
[MIG][10] auth_signup_verify_email
3fd2089 
* Module set to installable
* Odoo don't let signup without password
@oca-transbot @pedrobaeza
OCA Transbot updated translations from Transifex
f806deb
@simahawk @pedrobaeza
[MIG][11] auth_signup_verify_email
fc447df
@yajo @pedrobaeza
[FIX+REF] auth_signup_verify_email: Fix tests + Skip mail send in tes…
95bb3cd 
…ts and use email_validator (OCA#29)

This addon introduced an integration conflict when tested in a database that had `mail_tracking_mass_mailing` installed, producing this failure:

    Traceback (most recent call last):
    File "/opt/odoo/auto/addons/auth_signup_verify_email/controllers/main.py", line 44, in passwordless_signup
        sudo_users.reset_password(values.get("login"))
    File "/usr/lib/python2.7/contextlib.py", line 24, in __exit__
        self.gen.next()
    File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 419, in savepoint
        self.execute('RELEASE SAVEPOINT "%s"' % name)
    File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 154, in wrapper
        return f(self, *args, **kwargs)
    File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 231, in execute
        res = self._obj.execute(query, params)
    InternalError: no such savepoint

Which in turn produced the following in the next test:

    InternalError: current transaction is aborted, commands ignored until end of transaction block

The problem comes from the fact that one cannot rollback to a nested savepoint if the parent savepoint was released. It became a problem because that's the strategy that both this addon and upstream's `TestCursor` follow.

To avoid that, tests now mock the `send_mail` method. This results also in having a predictable outcome from the test `test_good_email`, so it is more meaningful now.

Besides, previously we were using the `validate_email` package, which is currently a dead project that can silently fail under certain environments, as seen in syrusakbary/validate_email#80.

There's the `email_validator` package, freely available, supported, and it provides a human-readable error message whenever some format from the email fails.

As such, here I'm switching the dependency, while still adding a backwards compatibility layer for preexisting installations.
@zaoral @pedrobaeza
[FIX] auth_signup_verify_email: Improve signup error messages
a032cd7 
* When try to login with a not valid email domain it raised a 500 server
  error. Now we shwt the proper message "The domain name <domain> does not
  exist."
* If there is any error we do not have catched when we do the email
  validation then we manage it and show the error message to the user instead
  of 500 server error.
* When we try to register with a already registered email it raise "Something
  went wrong, please try again later or contact us". Now shows the message
  as odoo regular signup does: "Another user is already registered using this
  email address" which is more specific and clear for this case.
@Tardo @pedrobaeza
[MIG] auth_signup_verify_email: Migration to 12.0
63d94a5
@pedrobaeza
Translated using Weblate (Portuguese (Brazil))
5500ae3 
Currently translated at 100.0% (4 of 4 strings)

Translation: server-auth-12.0/server-auth-12.0-auth_signup_verify_email
Translate-URL: https://translation.odoo-community.org/projects/server-auth-12-0/server-auth-12-0-auth_signup_verify_email/pt_BR/
@badbole @pedrobaeza
Translated using Weblate (Croatian)
eca5d5d 
Currently translated at 100.0% (4 of 4 strings)

Translation: server-auth-12.0/server-auth-12.0-auth_signup_verify_email
Translate-URL: https://translation.odoo-community.org/projects/server-auth-12-0/server-auth-12-0-auth_signup_verify_email/hr/
@Tardo @pedrobaeza
[IMP] auth_signup_verify_email: Remove hack
40d65a9
@pedrobaeza
[FIX] auth_signup_verify_email: make tests compatible with website
b7a44b7 
This commit fixes 2 problems:

1.  `auth_signup.allow_uninvited` ICP is no longer used.
    The new equivalent is `auth_signup.invitation_scope=b2c`.
2.  If running tests in an environment where `website` is installed,
    the website needs to get public signup enabled or tests here would
    fail with an error like this:

        2020-02-19 12:36:31,686 35 INFO prod odoo.modules.module: odoo.addons.auth_signup_verify_email.tests.test_verify_email running tests.
        2020-02-19 12:36:31,686 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: test_bad_email (odoo.addons.auth_signup_verify_email.tests.test_verify_email.UICase)
        2020-02-19 12:36:31,686 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ` Test rejection of bad emails.
        2020-02-19 12:36:31,735 35 INFO ? odoo.http: HTTP Configuring static files
        2020-02-19 12:36:31,738 35 INFO prod odoo.addons.base.models.ir_http: Generating routing map
        2020-02-19 12:36:34,227 35 INFO prod werkzeug: 127.0.0.1 - - [19/Feb/2020 12:36:34] "GET /web/signup HTTP/1.1" 404 - 763 0.262 2.227
        2020-02-19 12:36:34,231 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ERROR
        2020-02-19 12:36:34,231 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: test_good_email (odoo.addons.auth_signup_verify_email.tests.test_verify_email.UICase)
        2020-02-19 12:36:34,231 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ` Test acceptance of good emails.
        2020-02-19 12:36:36,293 35 INFO prod werkzeug: 127.0.0.1 - - [19/Feb/2020 12:36:36] "GET /web/signup HTTP/1.1" 404 - 759 0.258 1.797
        2020-02-19 12:36:36,296 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ERROR
        2020-02-19 12:36:36,297 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ======================================================================
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ERROR: test_bad_email (odoo.addons.auth_signup_verify_email.tests.test_verify_email.UICase)
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ` Test rejection of bad emails.
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: Traceback (most recent call last):
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `   File "/opt/odoo/auto/addons/auth_signup_verify_email/tests/test_verify_email.py", line 20, in setUp
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `     "csrf_token": self.csrf_token(),
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `   File "/opt/odoo/auto/addons/auth_signup_verify_email/tests/test_verify_email.py", line 33, in csrf_token
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `     return doc.xpath("//input[@name='csrf_token']")[0].get("value")
        2020-02-19 12:36:36,297 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ` IndexError: list index out of range
        2020-02-19 12:36:36,297 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ======================================================================
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ERROR: test_good_email (odoo.addons.auth_signup_verify_email.tests.test_verify_email.UICase)
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ` Test acceptance of good emails.
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: Traceback (most recent call last):
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `   File "/opt/odoo/auto/addons/auth_signup_verify_email/tests/test_verify_email.py", line 20, in setUp
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `     "csrf_token": self.csrf_token(),
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `   File "/opt/odoo/auto/addons/auth_signup_verify_email/tests/test_verify_email.py", line 33, in csrf_token
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: `     return doc.xpath("//input[@name='csrf_token']")[0].get("value")
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: ` IndexError: list index out of range
        2020-02-19 12:36:36,298 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: Ran 2 tests in 4.611s
        2020-02-19 12:36:36,298 35 ERROR prod odoo.addons.auth_signup_verify_email.tests.test_verify_email: FAILED
        2020-02-19 12:36:36,298 35 INFO prod odoo.addons.auth_signup_verify_email.tests.test_verify_email:  (errors=2)
        2020-02-19 12:36:36,298 35 ERROR prod odoo.modules.module: Module auth_signup_verify_email: 0 failures, 2 errors

@Tecnativa TT21005
@em230418 @pedrobaeza
[IMP] auth_signup_verify_email: black, isort, prettier
d8cda28
@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-270-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3fc3b6e into OCA:14.0 Jan 21, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 546c969. Thanks a lot for contributing to OCA. ❤️

SiesslPhillip pushed a commit to grueneerde/OCA-server-auth that referenced this pull request Nov 20, 2024
@bt-admin
Merge pull request OCA#270 from OCA/14.0
cd41c20 
Syncing from upstream OCA/server-auth (14.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ernestotejeda ernestotejeda ernestotejeda approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
approved merged 🎉
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.