[12.0][MIG] auth_signup_verify_email by Tardo · Pull Request #108 · OCA/server-auth

Tardo · 2019-06-04T12:57:49Z

PR superseded: #69

cc @Tecnativa

pedrobaeza · 2019-06-04T12:59:56Z

What is the PR superseeded?

Tardo · 2019-06-04T13:02:45Z

@pedrobaeza PR updated with the requested info.

pedrobaeza · 2019-06-04T14:29:58Z

@ernestotejeda please review this PR

chienandalu

I'm trying a functional test but I get this error

I'd say the problem comes from this view: https://github.com/OCA/server-auth/pull/108/files#diff-add74761757ad34e815f5cd9f9933f97R9

Tardo · 2019-06-05T13:13:56Z

@chienandalu The problem is the SMTP configuration: https://github.com/OCA/server-auth/pull/108/files#diff-b76492f8c47aaeb881c43ddd50c764ccR67
'only_passwords' is used when you have a token, this token is sent via email to set the passwords.
Try using test@ñ0.com email, you can see that the form is sent correctly...

ernestotejeda

Work great for me

Generate README.rst again after make the change i've proposed to include all the changes made in the readme fragment files
Squash to have a single migration commit

chienandalu

Sorry @Tardo But even with a right smtp config and my email I'm not able to signup on the in the instance.

Tardo · 2019-06-05T16:38:05Z

@chienandalu can reproduce it on local?

ernestotejeda · 2019-06-05T19:30:52Z

It worked well for me on the runbot

martintorresvm

LGTM technical speaking. Waiting for functional approval of @chienandalu

mohamedhagag · 2019-07-08T11:16:20Z

it's working fine with me .

pedrobaeza · 2019-07-08T11:22:07Z

/ocabot merge

OCA-git-bot · 2019-07-08T11:22:18Z

This PR looks fantastic, let's merge it!
Rebased to 12.0-ocabot-merge-pr-108-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot · 2019-07-08T11:34:05Z

@pedrobaeza Merge command aborted due to a failed check on 12.0-ocabot-merge-pr-108-by-pedrobaeza-bump-no.

pedrobaeza · 2019-07-08T11:35:39Z

@Tardo please check the problem in Travis on the rebased branch.

Tardo · 2019-07-08T16:44:30Z

@pedrobaeza it's a incompatibility with 'auth_oauth' module.. these module doesn't modify the qcontext properly... https://github.com/odoo/odoo/blob/12.0/addons/auth_oauth/controllers/main.py#L116
I made a "hack" to prevent crashes.

yajo · 2019-07-09T07:20:22Z

these module doesn't modify the qcontext properly

What's wrong there? 🤔

Tardo · 2019-07-09T08:32:38Z

@yajo the correct place to modify the qcontext is here: https://github.com/odoo/odoo/blob/11.0/addons/auth_signup/controllers/main.py#L101
If you change the xml and modify the qcontext in 'web_auth_signup' the view will broke if you don't use "super"...

yajo

I opened odoo/odoo#34690 so please link it in the hacks, and let's see if in the future we can remove the HACK.

MiquelRForgeFlow · 2019-08-14T18:03:19Z

conflicts! rebase please

* 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.

Tardo · 2019-08-14T20:31:25Z

@mreficent rebased, thanks!

pedrobaeza · 2019-08-14T23:23:44Z

@yajo, please re-review

rafaelbn

Missing technical final review of @yajo

But, functionally 👍

yajo · 2019-08-21T11:57:00Z

Please squash the migration-related commits.

Tardo · 2019-08-21T18:03:19Z

@yajo done!

* Module set to installable * Odoo don't let signup without password

…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.

* 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.

pedrobaeza · 2019-08-21T18:33:32Z

/ocabot merge

OCA-git-bot · 2019-08-21T18:33:43Z

What a great day to merge this nice PR. Let's do it!
Rebased to 12.0-ocabot-merge-pr-108-by-pedrobaeza-bump-no, awaiting test results.

Signed-off-by pedrobaeza

OCA-git-bot · 2019-08-21T19:17:00Z

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

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

Syncing from upstream OCA/server-auth (12.0)

pedrobaeza requested a review from yajo June 4, 2019 12:58

pedrobaeza added this to the 12.0 milestone Jun 4, 2019

Tardo force-pushed the 12.0-mig-auth_signup_verify_email branch from 5638970 to 026904a Compare June 4, 2019 13:20

pedrobaeza mentioned this pull request Jun 4, 2019

[MIG] auth_signup_verify_email: Migration to 12.0 #69

Closed

OCA-git-bot mentioned this pull request Jun 4, 2019

Migration to version 12.0 #38

Closed

19 tasks

chienandalu reviewed Jun 4, 2019

View reviewed changes

yajo requested changes Jun 5, 2019

View reviewed changes

Comment thread auth_signup_verify_email/readme/CONFIGURE.rst Outdated

Comment thread auth_signup_verify_email/readme/INSTALL.rst Outdated

ernestotejeda reviewed Jun 5, 2019

View reviewed changes

Comment thread auth_signup_verify_email/readme/CONFIGURE.rst

chienandalu reviewed Jun 5, 2019

View reviewed changes

martintorresvm approved these changes Jun 13, 2019

View reviewed changes

Tardo force-pushed the 12.0-mig-auth_signup_verify_email branch from ce8890a to bfd4ca2 Compare July 8, 2019 16:52

yajo mentioned this pull request Jul 9, 2019

[FIX] auth_oauth: Override qcontext in the right place odoo/odoo#34690

Closed

yajo requested changes Jul 9, 2019

View reviewed changes

Tardo force-pushed the 12.0-mig-auth_signup_verify_email branch from b01b8e5 to 749cb9d Compare July 16, 2019 16:23

Tardo force-pushed the 12.0-mig-auth_signup_verify_email branch from 749cb9d to a148f1e Compare August 14, 2019 20:27

rafaelbn approved these changes Aug 16, 2019

View reviewed changes

yajo approved these changes Aug 21, 2019

View reviewed changes

OCA-git-bot added the approved label Aug 21, 2019

MiquelRForgeFlow reviewed Aug 21, 2019

View reviewed changes

Comment thread auth_signup_verify_email/__manifest__.py

Tardo force-pushed the 12.0-mig-auth_signup_verify_email branch from a148f1e to 3738c55 Compare August 21, 2019 18:02

sharkende and others added 6 commits August 21, 2019 20:31

[MIG][10] auth_signup_verify_email

c3cbc40

* Module set to installable * Odoo don't let signup without password

OCA Transbot updated translations from Transifex

5d84eb5

[MIG][11] auth_signup_verify_email

34e5b1e

[MIG] auth_signup_verify_email: Migration to 12.0

64a3827

pedrobaeza force-pushed the 12.0-mig-auth_signup_verify_email branch from 3738c55 to 64a3827 Compare August 21, 2019 18:33

OCA-git-bot added a commit that referenced this pull request Aug 21, 2019

Merge PR #108 into 12.0

df1f6d9

Signed-off-by pedrobaeza

OCA-git-bot merged commit 64a3827 into OCA:12.0 Aug 21, 2019

OCA-git-bot added the merged 🎉 label Aug 21, 2019

yajo deleted the 12.0-mig-auth_signup_verify_email branch August 29, 2019 07:42

SiesslPhillip pushed a commit to grueneerde/OCA-server-auth that referenced this pull request Nov 20, 2024

Merge pull request OCA#108 from OCA/12.0

3bde658

Syncing from upstream OCA/server-auth (12.0)

Uh oh!

Conversation

Tardo commented Jun 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedrobaeza commented Jun 4, 2019

Uh oh!

Tardo commented Jun 4, 2019

Uh oh!

pedrobaeza commented Jun 4, 2019

Uh oh!

chienandalu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Tardo commented Jun 5, 2019

Uh oh!

ernestotejeda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chienandalu left a comment

Choose a reason for hiding this comment

Uh oh!

Tardo commented Jun 5, 2019

Uh oh!

ernestotejeda commented Jun 5, 2019 • edited by yajo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martintorresvm left a comment

Choose a reason for hiding this comment

Uh oh!

mohamedhagag commented Jul 8, 2019

Uh oh!

pedrobaeza commented Jul 8, 2019

Uh oh!

OCA-git-bot commented Jul 8, 2019

Uh oh!

OCA-git-bot commented Jul 8, 2019

Uh oh!

pedrobaeza commented Jul 8, 2019

Uh oh!

Tardo commented Jul 8, 2019

Uh oh!

yajo commented Jul 9, 2019

Uh oh!

Tardo commented Jul 9, 2019

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MiquelRForgeFlow commented Aug 14, 2019

Uh oh!

Tardo commented Aug 14, 2019

Uh oh!

pedrobaeza commented Aug 14, 2019

Uh oh!

rafaelbn left a comment

Choose a reason for hiding this comment

Uh oh!

yajo commented Aug 21, 2019

Uh oh!

Uh oh!

Tardo commented Aug 21, 2019

Uh oh!

pedrobaeza commented Aug 21, 2019

Uh oh!

OCA-git-bot commented Aug 21, 2019

Uh oh!

OCA-git-bot commented Aug 21, 2019

Uh oh!

Reviewers

Assignees

Tardo commented Jun 4, 2019 •

edited

Loading

ernestotejeda commented Jun 5, 2019 •

edited by yajo

Loading