Skip to content

Conversation

@rvalyi
Copy link
Member

@rvalyi rvalyi commented Aug 15, 2016

cc @legalsylvain
@pedrobaeza sorry I didn't master the API wizardry enough, but I finally got it an migrated res_users.py too. I amended the push and pushed again.
Travis didn't start in the old PR so I closed it an created a new one.

@pedrobaeza
Copy link
Member

Thanks for the changes. Please check Travis errors (some are related, although I think the main build isn't).

@rvalyi rvalyi force-pushed the 8.0-auth_admin_passkey-new-api branch 3 times, most recently from 1ba0081 to 009106a Compare August 16, 2016 06:41
@rvalyi rvalyi force-pushed the 8.0-auth_admin_passkey-new-api branch from 009106a to 8025a5d Compare August 16, 2016 07:00
@rvalyi
Copy link
Member Author

rvalyi commented Aug 16, 2016

@pedrobaeza I amended the commit and fixed the style. As you can see the Travis error on fetchmail is unrelated, so tests are passing.

@rvalyi
Copy link
Member Author

rvalyi commented Aug 18, 2016

@pedrobaeza hey I won the heisenbug Travis lottery after a few attempts: all green. So if you could review again... The idea is I migrated the API just before an other v9 PR port and also the feature extension we discussed a year ago...

@rvalyi rvalyi mentioned this pull request Aug 18, 2016
from openerp import exceptions
from openerp.osv.orm import Model
from openerp import models, api
from openerp.tools.translate import _
Copy link
Member

Choose a reason for hiding this comment

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

This can be also imported in openerp namespace directly

@rvalyi
Copy link
Member Author

rvalyi commented Aug 18, 2016

@pedrobaeza done. How does it look now?

@pedrobaeza
Copy link
Member

Thanks for the changes.

👍

@lasley
Copy link
Contributor

lasley commented Aug 18, 2016

LGTM as well 👍

'auth_admin_passkey_send_to_user': safe_eval(icp.get_param(
cr, uid, 'auth_admin_passkey.send_to_user', 'True')),
'auth_admin_passkey_send_to_user':
self.env["ir.config_parameter"].get_param(
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi,
Why did you removed safe_eval here, and not here :
8025a5d#diff-70199c6feb20b249a2794f26b51b5105R35

@legalsylvain legalsylvain added this to the 8.0 milestone Aug 22, 2016
@legalsylvain
Copy link
Contributor

Hi raphael,
Thanks a lot for porting this module. Just one question in the first commit about safe_eval.
Otherwise LGTM. Thanks !

@sbidoul
Copy link
Member

sbidoul commented Aug 24, 2016

@rvalyi since you are busy refactoring this module, do you have an opinion on #529 ?

@dvdhinesh
Copy link
Member

I am facing an Internal server error when testing this PR on runbot.

Issue:
When a user login with the same password as admin, it tries to send an email for intimation.

Traceback:

Traceback (most recent call last):
result = dispatch(method, params)
return fn(*params)
return res_users.authenticate(db, login, password, user_agent_env)
self._send_email_same_password(cr, login)
return old_api(self, *args, **kwargs)
TypeError: _send_email_same_password() takes exactly 2 arguments (3 given)

PR available @: akretion#6

@pedrobaeza pedrobaeza merged commit 58ec085 into OCA:8.0 Nov 22, 2016
Fenkiou pushed a commit to Fenkiou/server-tools that referenced this pull request Apr 1, 2017
Fenkiou pushed a commit to Fenkiou/server-tools that referenced this pull request May 2, 2017
Fenkiou pushed a commit to Fenkiou/server-tools that referenced this pull request May 2, 2017
Fenkiou pushed a commit to Fenkiou/server-tools that referenced this pull request Jun 25, 2017
lasley pushed a commit to LasLabs/server-tools that referenced this pull request Aug 29, 2017
remihb pushed a commit to osiell/server-tools that referenced this pull request Jan 3, 2019
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (12.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants