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

Ldap password renewal fixes for NC13 #9288

Merged
merged 4 commits into from
May 12, 2018

Conversation

GitHubUser4234
Copy link
Contributor

Hi,

The "Renew Password" page in user_ldap needed minor adjustments to keep it aligned with NC13 core changes:

  • Commit "adjust css to nc13 core changes"
    Since NC 13 the "Renew Password" page doesn't load the server.scss anymore, so tooltip related CSS is copied over to renewPassword.css until issue server.scss for login pages #9251 is solved. The commit also includes a minor adjustment in label position and button background color

  • Commit "remove reset password link"
    The reset password function in core has been heavily revamped, so instead of updating the "Renew Password" page with this now quite complicated function, the reset password link is now removed completely from the "Renew Password" page. This is because such feature isn't needed on the "Renew Password" page anyway, and here's why:

  1. User has to enter the correct password on the Login page before being redirected to the "Renew Password" page, so it's unlikely he suddenly forgets the password after having entered it correctly on the previous page
  2. User could click "Cancel" on the "Renew Password" page to return the Login page and perform a password reset there

@GitHubUser4234 GitHubUser4234 added the 3. to review Waiting for reviews label Apr 24, 2018
@GitHubUser4234 GitHubUser4234 added this to the Nextcloud 13.0.3 milestone Apr 24, 2018
width:0;
height:0;
border-color:transparent;
border-style:solid
Copy link
Member

Choose a reason for hiding this comment

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

Could I ask you to have them same indention level as in the previous lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah sure, done now with commit "css indent alignment"

Signed-off-by: Roger Szabo <[email protected]>
@codecov
Copy link

codecov bot commented Apr 25, 2018

Codecov Report

Merging #9288 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #9288      +/-   ##
============================================
- Coverage     51.94%   51.93%   -0.02%     
- Complexity    25383    25389       +6     
============================================
  Files          1608     1608              
  Lines         95423    95435      +12     
  Branches       1394     1394              
============================================
- Hits          49571    49563       -8     
- Misses        45852    45872      +20
Impacted Files Coverage Δ Complexity Δ
apps/user_ldap/templates/renewpassword.php 0% <ø> (ø) 0 <0> (ø) ⬇️
apps/theming/lib/ImageManager.php 72.41% <0%> (-14.26%) 24% <0%> (+14%)
apps/theming/lib/Settings/Admin.php 82.35% <0%> (-3.76%) 5% <0%> (ø)
apps/theming/lib/ThemingDefaults.php 89.2% <0%> (-2.59%) 48% <0%> (+2%)
...iles_sharing/lib/Controller/ShareAPIController.php 68.03% <0%> (-0.15%) 160% <0%> (ø)
lib/private/Server.php 82.57% <0%> (ø) 278% <0%> (ø) ⬇️
apps/theming/appinfo/routes.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
apps/theming/templates/settings-admin.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
apps/theming/lib/Controller/ThemingController.php 75.33% <0%> (+0.14%) 29% <0%> (-14%) ⬇️
lib/private/User/Database.php 79.47% <0%> (+0.41%) 43% <0%> (+1%) ⬆️
... and 1 more

@MorrisJobke
Copy link
Member

cc @nextcloud/ldap

@blizzz
Copy link
Member

blizzz commented Apr 27, 2018

The commit also includes a minor adjustment in label position and button background color

Is it specific to that page, or useful for nextcloud in general?

@GitHubUser4234
Copy link
Contributor Author

Is it specific to that page, or useful for nextcloud in general?

Specific to that page.

Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

Form looks proper, logic works

@blizzz
Copy link
Member

blizzz commented May 9, 2018

needs another reviewer

@blizzz blizzz requested a review from rullzer May 9, 2018 11:02
@rullzer rullzer merged commit d341a96 into nextcloud:master May 12, 2018
@MorrisJobke
Copy link
Member

Backport is in #9661

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants