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

No feedback when changing passwords as administrator #25532

Closed
tfrdidi opened this Issue Jul 19, 2016 · 9 comments

Comments

Projects
None yet
5 participants
@tfrdidi

tfrdidi commented Jul 19, 2016

Steps to reproduce

  1. Go to <owncloudBaseUri>/settings/users
  2. Change password of a user
  3. Press Enter

Expected behaviour

I would expect some feedback if it worked or not.

Actual behaviour

No feedback shown.

Server configuration

Operating system: CentOS Linux release 7.2.1511 (Core)

Web server: Apache/2.4.6 (CentOS)

Database: MariaDB

PHP version: PHP 5.5.37

ownCloud version: 9.0.3 (stable)

Updated from an older ownCloud or fresh install: upgrade from 9.0.1 (stable)

Where did you install ownCloud from: yum

List of activated apps:

Enabled:

  • activity: 2.2.1
  • comments: 0.2
  • dav: 0.1.6
  • documents: 0.12.0
  • encryption: 1.2.0
  • federatedfilesharing: 0.1.0
  • files: 1.4.4
  • files_antivirus: 0.8.0.2
  • files_external: 0.5.2
  • files_pdfviewer: 0.8.1
  • files_sharing: 0.9.1
  • files_texteditor: 2.1
  • files_trashbin: 0.8.0
  • files_versions: 1.2.0
  • files_videoplayer: 0.9.8
  • firstrunwizard: 1.1
  • gallery: 14.5.0
  • notifications: 0.2.3
  • passwordpolicy: 1.1
  • provisioning_api: 0.4.1
  • systemtags: 0.2
  • templateeditor: 0.1
  • updatenotification: 0.1.0
    Disabled:
  • external
  • federation
  • user_external
  • user_ldap

The content of config/config.php:

'de', 'updatechecker' => false, 'instanceid' => '...', 'passwordsalt' => '...', 'secret' => '...', 'trusted_domains' => array ( 0 => '...', 1 => '...', ), 'datadirectory' => '/srv/owncloud-data/', 'overwrite.cli.url' => '...', 'dbtype' => 'mysql', 'version' => '9.0.3.2', 'dbname' => '...', 'dbhost' => '...', 'dbtableprefix' => '...', 'dbuser' => '...', 'dbpassword' => '...', 'logtimezone' => 'Europe/Berlin', 'installed' => true, 'mail_from_address' => 'hostmaster', 'mail_smtpmode' => 'php', 'mail_domain' => '...', 'singleuser' => false, 'appstore.experimental.enabled' => true, 'enabledPreviewProviders' => array ( 0 => 'OC\Preview\PNG', 1 => 'OC\Preview\JPEG', 2 => 'OC\Preview\GIF', 3 => 'OC\Preview\BMP', 4 => 'OC\Preview\XBitmap', 5 => 'OC\Preview\MP3', 6 => 'OC\Preview\TXT', 7 => 'OC\Preview\MarkDown', 8 => 'OC\Preview\PDF', ), 'theme' => '', 'loglevel' => 2, 'maintenance' => false, 'htaccess.RewriteBase' => '/owncloud', 'theme' => '...', 'preview_libreoffice_path' => '/usr/bin/libreoffice', ); ?>

Are you using external storage, if yes which one: local/smb/sftp/...
No

Are you using encryption: yes/no
Yes

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...
No

Client configuration

Browser:
Chrome/FF/IE

Operating system:
Win 10

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jul 21, 2016

Member

At least a yellow notification would be nice: OC.Notification.showTemporary(...)

Tagging as junior job

Member

PVince81 commented Jul 21, 2016

At least a yellow notification would be nice: OC.Notification.showTemporary(...)

Tagging as junior job

@PVince81 PVince81 added this to the 9.1.1 milestone Jul 21, 2016

@tfrdidi tfrdidi changed the title from No feedback when changeing passwords as administrator to No feedback when changing passwords as administrator Aug 3, 2016

@kavishdahekar

This comment has been minimized.

Show comment
Hide comment
@kavishdahekar

kavishdahekar Aug 9, 2016

Contributor

I have identified the solution for the above issue. Should I go ahead and create a pull request?

// file : core/settings/js/users/users.js
// line 671
if (result.status != 'success') {
    OC.Notification.showTemporary(t('admin', result.data.message));
}else{
    // shows message indicating success
    OC.Notification.showTemporary(t('admin', "Password changed successfully!"));
}
Contributor

kavishdahekar commented Aug 9, 2016

I have identified the solution for the above issue. Should I go ahead and create a pull request?

// file : core/settings/js/users/users.js
// line 671
if (result.status != 'success') {
    OC.Notification.showTemporary(t('admin', result.data.message));
}else{
    // shows message indicating success
    OC.Notification.showTemporary(t('admin', "Password changed successfully!"));
}
@tfrdidi

This comment has been minimized.

Show comment
Hide comment
@tfrdidi

tfrdidi Aug 9, 2016

I suggest to use the i18n-version of "Password changed successfully!" to allow the translation of this string. Furthermore keep in mind, that the Server might respond with a 500, e.g. when the private key for the user is missing. In this case I am quite sure, that you do not get the proper result.data.message.

tfrdidi commented Aug 9, 2016

I suggest to use the i18n-version of "Password changed successfully!" to allow the translation of this string. Furthermore keep in mind, that the Server might respond with a 500, e.g. when the private key for the user is missing. In this case I am quite sure, that you do not get the proper result.data.message.

@JackWillDavis

This comment has been minimized.

Show comment
Hide comment
@JackWillDavis

JackWillDavis Aug 9, 2016

Contributor

I have an active pull request with this fix. I would suggest not having the != success in the IF statement. If something goes wrong, we don't want to default back to displaying a success message in the ELSE.

See: https://github.com/JackWillDavis/core/pull/1

Contributor

JackWillDavis commented Aug 9, 2016

I have an active pull request with this fix. I would suggest not having the != success in the IF statement. If something goes wrong, we don't want to default back to displaying a success message in the ELSE.

See: https://github.com/JackWillDavis/core/pull/1

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Aug 10, 2016

Member

@JackWillDavis you need to send the pull request back to the fork owncloud/core (upstream). That PR is pointing to your own fork as target.

Member

PVince81 commented Aug 10, 2016

@JackWillDavis you need to send the pull request back to the fork owncloud/core (upstream). That PR is pointing to your own fork as target.

@JackWillDavis

This comment has been minimized.

Show comment
Hide comment
@JackWillDavis

JackWillDavis Aug 10, 2016

Contributor

@PVince81 ahh you're right, I'm still getting used to contributions to open source on here. I will sort this out shortly.

Contributor

JackWillDavis commented Aug 10, 2016

@PVince81 ahh you're right, I'm still getting used to contributions to open source on here. I will sort this out shortly.

@JackWillDavis

This comment has been minimized.

Show comment
Hide comment
@JackWillDavis

JackWillDavis Aug 10, 2016

Contributor

New PR: #25756

Contributor

JackWillDavis commented Aug 10, 2016

New PR: #25756

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Aug 19, 2016

Member

thanks @JackWillDavis - great job

Member

DeepDiver1975 commented Aug 19, 2016

thanks @JackWillDavis - great job

@JackWillDavis

This comment has been minimized.

Show comment
Hide comment
@JackWillDavis

JackWillDavis Aug 19, 2016

Contributor

Thanks very much!

Contributor

JackWillDavis commented Aug 19, 2016

Thanks very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment