Conversation
Odoo don't let signup without password
|
Hey @sharkende, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
|
Hi pedrobaeza, |
|
If you have signed the CLA, you just have to wait that at least 2 reviewers validate your work. See https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#review for more information. |
|
Ok thanks |
|
Any news on this one ? Changes in this PR aren't that big, could someone take a look to validate it? @yajo and @moylop260 were the ones that migrated it from 8.0 to 9.0. Thanks! |
|
If you want this to be reviewed, review other PRs from the same repo or others and ask in exchange to review this one. |
|
You probably have to adapt a bit the README file: for example the runbot URL and refresh with newest template. |
|
|
||
| # Remove password | ||
| values["password"] = False | ||
| values["password"] = "" |
There was a problem hiding this comment.
I cannot understand the reasoning behind this change. Could you explain please?
There was a problem hiding this comment.
Well Odoo 10.0 doesn't allow to signup without password (False) but it work with an empty password/string ("").
There was a problem hiding this comment.
But I have searched through the code and cannot see where it breaks, could you point me to it please? The field is not required after all...
There was a problem hiding this comment.
Well I have search it too and to be honest I don't remember where it breaks. I just know that it works :/
There was a problem hiding this comment.
Hmm I don't say it doesn't work, but I suspect it works also with previous implementation, and it's not a good practice to store empty strings in fields. I don't feel comfortable skipping good practices precisely in the password field 🤔, so please elaborate this need before merging.
There was a problem hiding this comment.
If we use the module as is with the password set to False, we can't signup:
Traceback (most recent call last):
File "/Users/alex/Documents/workspace/odoo/addons/10.0/server-tools/auth_signup_verify_email/controllers/main.py", line 43, in passwordless_signup
sudo_users.signup(values, qcontext.get("token"))
File "/Users/alex/Documents/workspace/odoo/odoo10/addons/auth_signup/models/res_users.py", line 71, in signup
self._signup_create_user(values)
File "/Users/alex/Documents/workspace/odoo/odoo10/addons/auth_signup/models/res_users.py", line 98, in _signup_create_user
raise SignupError(ustr(e))
SignupError: secret must be unicode or bytes, not bool
Can't find why this is raised, but, as stated in the code for the password field: "Keep empty if you don't want the user to be able to connect on the system." and the default value is '' not False
I think even for odoo 9, the password should be set to ''
There was a problem hiding this comment.
OH I see, thanks! 😊 It's because of the secret computation. It needs strings to work.
|
@elicoidal Ok for the runbot url but the newest template ? I don't see anything different. Can you explain please ? |
|
FYI continuation of this PR Vauxoo#90 |
|
The Merge of this PR seems to have broken Travis. (Commit n° 385e9b7) https://github.com/OCA/server-tools/commits/10.0 @moylop260, @sharkende Could you take a look ? thanks. |
|
@legalsylvain I don't know how travis is able to run this module's tests as they are still using openerp package. For me, to run the tests, I had to:
Also, for having the test pass in case of a running smtp server, I had to set the timeout to 20sec not sure that's relevant |
|
I think #762 may fix these tests. |
Syncing from upstream OCA/server-tools (14.0)
10 participants
Module set to installable
Odoo don't let signup without password