Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable8] backport 13740 and 15606 #16456

Merged
merged 6 commits into from Jul 23, 2015
Merged

Conversation

MorrisJobke
Copy link
Contributor

Backport of #13740 and #15606 (because the latter is a followup of the previous one)

Fixes #12190 and fixes #13533

How to test?

  • configure User Filter in the LDAP Wizard and select a group
  • on AD, the resulting filter should look like (&(|(objectclass=person))(|(|((memberof=CN=Domain Users,CN=Users,DC=madder,DC=owncloud,DC=bzoc)(primaryGroupID=513)))(|((memberof=CN=Wolfpack 8,OU=packs,DC=madder,DC=owncloud,DC=bzoc)(primaryGroupID=968988)(2 selected groups)
  • on OpenLDAP the (primaryGroupID=XXX) part should not show up!
  • in the User list: users should appear, that have the selected group as primary, but are not regular member of any of those selected
  • in the User list: Before: empty count, no users when selecting the group. Now: proper count with users.

@craigpg @sbelov1 @bboule @jnfrmarks @plastilincheg @rjaeckel @gig13 Please test. I tested this with an LDAP instance and all works like before and no errors are logged.

This is my 👍 for this, because I only cherry picked the commits of @blizzz who is on vacation.

@karlitschek
Copy link
Contributor

backport is fine 👍

@ghost
Copy link

ghost commented May 20, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/12438/
🚀 Test PASSed.🚀
chuck

@DeepDiver1975
Copy link
Member

we need testing power here @rperezb THX

@rperezb
Copy link

rperezb commented May 26, 2015

@DeepDiver1975 thanks for the reminder, yes, this is on our list, @davitol is currently checking

@MorrisJobke
Copy link
Contributor Author

@davitol I just tested if it still works with an LDAP instance.

@davitol
Copy link
Contributor

davitol commented May 27, 2015

I understood that with an AD , in the resulting filter using the wizard, (primaryGroupID=513) should appear, but it doesn't.

adbehaviour2

@MorrisJobke
Copy link
Contributor Author

@davitol For me the groupID shows up. :(

@MorrisJobke
Copy link
Contributor Author

Got it ... it is only there in the second try.

  • going to the user filter tab
  • selecting the group
  • => not present in the raw filter
  • unselect the group
  • select the group again
  • => present in the raw filter

@MorrisJobke
Copy link
Contributor Author

@cmonteroluque Either there volunteers somebody to debug this race condition or we need to delay this :( I don't think that I have time tomorrow to debug this properly.

@MorrisJobke
Copy link
Contributor Author

@cmonteroluque The blue ticket refers to a stable7 instance (where this is already fixed) So I'm fine with moving this to 8.0.5 and wait for @blizzz

@MorrisJobke
Copy link
Contributor Author

cc @DeepDiver1975 regarding release planing

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.0.5-next-maintenance, 8.0.4-current-maintenance May 28, 2015
@DeepDiver1975
Copy link
Member

@cmonteroluque The blue ticket refers to a stable7 instance (where this is already fixed) So I'm fine with moving this to 8.0.5 and wait for @blizzz

agreed

@davitol
Copy link
Contributor

davitol commented May 29, 2015

#16626

@MorrisJobke
Copy link
Contributor Author

The problem with this PR is that it is present in master and stable7 already and the issue, why it is not merged is #16626. Should we merge this now, because it is better to have a working group selection in stable8 for AD too, that has a known bug, but we know the workaround too. Or should we ship the broken behaviour in stable7 and master only and wait for the proper fix of this problem and then port the stuff from this stable8 branch to stable7 and master? cc @DeepDiver1975 @cmonteroluque

@PVince81
Copy link
Contributor

This is what I observe:

  1. Have the raw filter collapsed
  2. Select "group1", click outside
  3. Wait for all spinners to disappear
  4. Expand raw filter: group is missing
  5. Collapse raw filter
  6. Expand again: group appears

The same happens when deselecting a group.

It looks like the raw filter simply doesn't refresh properly, unless collapsed and expanded again.
Is that what you were observing ?

Also, if you expand too early while the spinners are still turning, there is a chance to mess it up too.
That link should probably be disabled while it's still working.

@MorrisJobke
Copy link
Contributor Author

@davitol I rebased this on the current stable8 (with #16627 merged). Now the groupID should show up for you. :)

@ghost
Copy link

ghost commented Jun 18, 2015

🚀 Test PASSed.🚀
chuck

@davitol
Copy link
Contributor

davitol commented Jun 19, 2015

@MorrisJobke I've tested again in stable8 -> "8.0.4.2","versionstring":"8.0.4","edition":"Enterprise"
actived3

@MorrisJobke
Copy link
Contributor Author

@MorrisJobke I've tested again in stable8 -> "8.0.4.2","versionstring":"8.0.4","edition":"Enterprise"

Did it work? It's too blurry to read anything :(

@davitol
Copy link
Contributor

davitol commented Jun 19, 2015

No, it didn't 😿

@DeepDiver1975
Copy link
Member

@MorrisJobke @blizzz looks like we need to get our hands dirty again -> moved to 8.0.6

@DeepDiver1975 DeepDiver1975 modified the milestones: 8.0.6-next-maintenance, 8.0.5-current-maintenance Jun 22, 2015
@MorrisJobke MorrisJobke changed the title Stable8 backport 13740 and 15606 [stable8] backport 13740 and 15606 Jul 9, 2015
@MorrisJobke
Copy link
Contributor Author

@pvince @blizzz Any idea how to address the issue?

@blizzz
Copy link
Contributor

blizzz commented Jul 15, 2015

@davitol

@MorrisJobke I've tested again in stable8 -> "8.0.4.2","versionstring":"8.0.4","edition":"Enterprise"

To reassure, did you use stable8 branch or this one here (stable8-backport-13740-and-15606)?

@davitol
Copy link
Contributor

davitol commented Jul 16, 2015

@blizzz Both of them with the same behaviour

@blizzz blizzz force-pushed the stable8-backport-13740-and-15606 branch from 5385262 to 4253838 Compare July 23, 2015 12:34
@blizzz
Copy link
Contributor

blizzz commented Jul 23, 2015

rebased

@ghost
Copy link

ghost commented Jul 23, 2015

🚀 Test PASSed.🚀
chuck

@davitol
Copy link
Contributor

davitol commented Jul 23, 2015

👍 It works :shipit:

@blizzz
Copy link
Contributor

blizzz commented Jul 23, 2015

🎆

blizzz added a commit that referenced this pull request Jul 23, 2015
@blizzz blizzz merged commit 385aefd into stable8 Jul 23, 2015
@blizzz blizzz deleted the stable8-backport-13740-and-15606 branch July 23, 2015 15:25
@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants