Skip to content

[12.0][MIG] users_ldap_populate - #132

Merged
OCA-git-bot merged 66 commits into
OCA:12.0from
Tecnativa:12.0-mig-users_ldap_populate
Dec 11, 2019
Merged

OCA-git-bot merged 66 commits into
OCA:12.0from
Tecnativa:12.0-mig-users_ldap_populate

Conversation

@Tardo

@Tardo Tardo commented Sep 9, 2019

Copy link
Copy Markdown
Member

Migration of users_ldap_populate from OCA/server-tools 10.0

Cc @Tecnativa

dreispt and others added 30 commits August 13, 2019 19:46
In order to get visibility on https://www.odoo.com/apps the OCA board has
decided to add the OCA as author of all the addons maintained as part of the
association.
and remove use of deprecated except_orm on the way
* leftovers from old api
* python-ldap does not like Unicode strings
r-hede and others added 3 commits August 13, 2019 19:46
* [10.0] users_ldap_populate special characters
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-10.0/server-tools-10.0-users_ldap_populate
Translate-URL: https://translation.odoo-community.org/projects/server-tools-10-0/server-tools-10-0-users_ldap_populate/
@oca-clabot

Copy link
Copy Markdown

Hey @Tardo, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

Comment thread users_ldap_populate/models/populate_wizard.py
Comment thread users_ldap_populate/models/populate_wizard.py
Comment thread users_ldap_populate/models/users_ldap.py
@rafaelbn

Copy link
Copy Markdown
Member

Please @yajo revew here thanks

@hbrunn hbrunn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please also add

Comment thread users_ldap_populate/models/users_ldap.py
@@ -63,9 +62,10 @@ def test_users_ldap_populate(self):
'uid': ['fake'],
'mail': ['fake@fakery.com'],
})]):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if you add a second dict here describing to a user that you add before as inactive (demo user will do), you also test the activating inactive users codepath which probably raises coveragfe enough to let this test pass

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I can't use demo user... if the user exists but is inactive the code raises this exception: https://github.com/odoo/odoo/blob/12.0/addons/auth_ldap/models/res_company_ldap.py#L202

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hbrunn do you agree with the answer?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no. Turn on creating users in the demo config then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Tardo do you know what to do then? I confess I'm a bit lost with this stuff.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@hbrunn Can you verify that '_get_or_create_user' can return a "false" value?

  • If the user exists and is actived = The user is used
  • If the user exists and is deactivated = Exception
  • If the user not exists and creation is not enabled = Exception
  • If the user not exists and creation is enabled = Copy/Creates a new user

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the function won't return a falsy value, but that's not the point here. Just turn on the create_user flag, then you actually test what this addon does.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@hbrunn "create_user" is enabled by default: https://github.com/odoo/odoo/blob/12.0/addons/auth_ldap/models/res_company_ldap.py#L34
I don't understand you, sorry.

@hbrunn hbrunn added this to the 12.0 milestone Oct 1, 2019
@Tardo
Tardo force-pushed the 12.0-mig-users_ldap_populate branch from 7cea0e8 to 79f8d03 Compare October 7, 2019 09:20
@Tardo

Tardo commented Oct 10, 2019

Copy link
Copy Markdown
Member Author

@pedrobaeza ready to review

@pedrobaeza

Copy link
Copy Markdown
Member

@Tardo please squash together adjacent translations commits

hbrunn
hbrunn previously requested changes Oct 14, 2019
Comment thread users_ldap_populate/models/users_ldap.py
@Tardo
Tardo force-pushed the 12.0-mig-users_ldap_populate branch from 79f8d03 to 678b545 Compare October 27, 2019 18:05

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can't wait more for this module, and the concerns are only in the tests part, so I'm going to merge it right now as is, and later PRs can improve that if it's a problem.

@pedrobaeza

Copy link
Copy Markdown
Member

/ocabot merge

@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

OCA-git-bot added a commit that referenced this pull request Dec 11, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot mentioned this pull request Dec 11, 2019
19 tasks
@OCA-git-bot
OCA-git-bot merged commit 678b545 into OCA:12.0 Dec 11, 2019
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@pedrobaeza
pedrobaeza deleted the 12.0-mig-users_ldap_populate branch October 15, 2023 10:23
SiesslPhillip pushed a commit to grueneerde/OCA-server-auth that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-auth (13.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.