Skip to content

[stable13] [LDAP] The WebUI Wizard also should not assign empty config IDs - #10824

Merged
blizzz merged 1 commit into
stable13from
backport/10820/stable13
Sep 13, 2018
Merged

[stable13] [LDAP] The WebUI Wizard also should not assign empty config IDs#10824
blizzz merged 1 commit into
stable13from
backport/10820/stable13

Conversation

@blizzz

@blizzzblizzz commented Aug 23, 2018

Copy link
Copy Markdown
Member

backport of #10820

@MorrisJobke not so serious, feel free to move to 13.0.7. This should not be a blocker.

@blizzz
[LDAP] The WebUI Wizard also should not assign empty config IDs
7c33395
With 689df9a the behaviour to assign only
non-empty config IDs was introduced. Only, this was only effective for CLI
and OCS API.
Related to #3270.
The web UI creates now also a full configuration on first load. This fixes#5094.
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzzblizzz added bug 3. to review Waiting for reviews feature: ldap papercut Annoying recurring UX issue with possibly simple fix. labels Aug 23, 2018
@blizzzblizzz added this to the Nextcloud 13.0.6 milestone Aug 23, 2018
@MorrisJobke

MorrisJobke commented Aug 24, 2018

Copy link
Copy Markdown
Member

RC 1 is out -> moving to 13.0.7

@blizzz

blizzz commented Sep 3, 2018

Copy link
Copy Markdown
MemberAuthor

failing tests not related

@blizzz

blizzz commented Sep 12, 2018

Copy link
Copy Markdown
MemberAuthor

reviews, pozhaluysta? 😸

nickvergessen
nickvergessen reviewed Sep 13, 2018
View reviewed changes
Comment threadapps/user_ldap/lib/Settings/Admin.php
public function getForm() {
$helper = new Helper(\OC::$server->getConfig());
$prefixes = $helper->getServerConfigurationPrefixes();
if(count($prefixes) === 0) {

@nickvergessennickvergessenSep 13, 2018

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.

Use empty in the future ;)

@blizzzblizzzSep 13, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

it wouldn't be a big array in this case, still you're right.

nickvergessen
nickvergessen approved these changes Sep 13, 2018
View reviewed changes

@nickvergessennickvergessen 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.

🐘

@blizzz
blizzz merged commit acce19f into stable13Sep 13, 2018
@blizzz
blizzz deleted the backport/10820/stable13 branch September 13, 2018 11:11
This was referenced Oct 3, 2018
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@rullzerrullzerAwaiting requested review from rullzer
@MorrisJobkeMorrisJobkeAwaiting requested review from MorrisJobke
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr
@GitHubUser4234GitHubUser4234Awaiting requested review from GitHubUser4234
@ArtificialOwlArtificialOwlAwaiting requested review from ArtificialOwl

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsbugfeature: ldappapercutAnnoying recurring UX issue with possibly simple fix.

Projects

None yet

Milestone

Nextcloud 13.0.7

Development

Successfully merging this pull request may close these issues.

4 participants

@blizzz@MorrisJobke@nickvergessen@skjnldsv