@svenseeberg can you resolve conflicts :)

PS: since its a pull from an org we maintainer cant apply code suggestions or resolve conflicts. if you need help just tell us.

@svenseeberg can you resolve conflicts :)

PS: since its a pull from an org we maintainer cant apply code suggestions or resolve conflicts. if you need help just tell us.

Right, should not be an issue. We will take care of rebasing on the current main branch.

I'll also look into the linting errors.

make fmt

you dont have to hurry we are currently in feature-freeze ... :)

& we need some tests for this code

we need some tests for this code

Are there any specific requirements for the tests? Mocking an LDAP server is somewhat complicated ;-) However, we could easily test the functions that do not interact with the LDAP server.

well we have unit tests who test selve contained functions or easy to moke on ...
example: https://github.com/go-gitea/gitea/blob/main/modules/auth/openid/discovery_cache_test.go

and we have integration tests:
example: https://github.com/go-gitea/gitea/blob/main/integrations/auth_ldap_test.go

and we have a running ldap to test against (https://drone.gitea.io/go-gitea/gitea/41501/2/5)

I guess we can work with that :-)

We have some LDAP tests already which use a docker container for the LDAP.

Codecov Report

Merging #16299 (a75516d) into main (212e81f) will increase coverage by 0.39%.
The diff coverage is 70.12%.

❗ Current head a75516d differs from pull request most recent head 0d402cc. Consider uploading reports for the commit 0d402cc to get more accurate results

@@            Coverage Diff             @@
##             main   #16299      +/-   ##
==========================================
+ Coverage   45.74%   46.14%   +0.39%     
==========================================
  Files         831      839       +8     
  Lines       92178    92563     +385     
==========================================
+ Hits        42171    42712     +541     
+ Misses      43249    43066     -183     
- Partials     6758     6785      +27     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f910924...0d402cc. Read the comment docs.

@6543 we updated our pull request an included tests. Can you please review?

Thank you for your review @6543
I'm sorry for the long response time, but now I have removed the funk package in 3a032cc
Unfortunately I could not find existing functions to use instead, so I added some to the utils module for these three use cases:

1. get all keys from a map
2. get the intersection of two slices
3. get the difference of two slices

Thanks in advance for your feedback

@svenseeberg can you resolve conflicts :)

(just do a merge of main and resolve - rebase or squash not required since pull's are squash-merged anyway)

@wxiaoguang wana have a look at the ui part?

I proposed a PR netzbegruenung#4 (from wxiaoguang@6ef197e)

Reason:

1. the LDAP group support of admin CLI is incomplete, it doesn't support group options so we do not need add our new options there.
2. remove TeamGroupMapEnabled, now GroupsEnabled controls both GroupFilter and our new GroupTeamMap
3. the UI is rewritten completely, see the screenshot, it's more simple and consistent.

@svenseeberg can you cherry-pick netzbegruenung@6ef197e ? I thinkt that's the cleanset solution ... & merge upstream in first

@svenseeberg can you cherry-pick netzbegruenung@6ef197e ? I thinkt that's the cleanset solution ... & merge upstream in first

Oh sorry, I read this comment after I merged the PR.

I added @wxiaoguang and @6543 as collaborators to our repo.

Since the main branch has changed a lot (including the lint rules), there are some new work to do. Give me some more time.

Hmm... CI fails, maybe I did something wrong during the refactoring. @svenseeberg could you help to take a look?

Fixing ...

@wxiaoguang yes you did not adjust the tests appropriate

ok two things need to be done:

1. test if this pull works in current state for you @svenseeberg @gtudan
2. update inteagration tests

@svenseeberg thanks - that will help :)

Just tried the latest build and I can confirm that it works

🚀

Thank you all for your work on this!