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

patron: allows librarians to update user password #411

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

jma
Copy link
Contributor

@jma jma commented Nov 4, 2020

  • Adds missing license in some files.
  • Fixes some spelling.
  • Adds a button in the circulation patron profile to update the
    password.
  • Adds a edit button in the circulation patron profile.

Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch

Why are you opening this PR?

Dependencies

How to test?

  • Go to a circulation user profile and try to change the password.
  • Log with the patron and test the new password.
  • Log with a librarian, try to change the password of a system librarian with a patron role. You should not be able to do it!

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?

@jma jma force-pushed the maj-patron-change-password branch from 1a37f11 to 37160c6 Compare November 4, 2020 08:28
@jma jma marked this pull request as ready for review November 4, 2020 08:39
@jma jma force-pushed the maj-patron-change-password branch from 37160c6 to 8ed6e06 Compare November 4, 2020 09:12
this.closeModal();
},
(resp) => {
console.log('Error: Update Patron Password', resp);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove console.log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it is to leave at is as it is displayed only when an error occurs. This can be useful. But if you want I can remove it.

@@ -15,6 +15,10 @@
 along with this program. If not, see <http://www.gnu.org/licenses/>.
-->
<article *ngIf="currentPatron$ | async as patron">
<div *ngIf="canUpdate()" class="float-right">
<button [routerLink]="['/records','patrons','edit', patron.pid]" class="btn btn-sm btn-primary ml-1 ng-star-inserted" id="profile-edit-button" title="Edit"><i class="fa fa-pen mr-1"></i>{{ 'Edit' | translate }}</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

COSMETIC: Too long. Multiple 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.

This is my default editor configuration... but I will do it.

</ng-container>
</dd>
</dl>
<button *ngIf="canUpdate()" class="btn btn-sm btn-outline-primary mt-2" id="profile-change-password-button" (click)="updatePatronPassword(patron)" title="Change Password">{{ 'Change Password' | translate }}</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

COSMETIC: Too long. Multiple lines ?

<div *ngIf="canUpdate()" class="float-right">
<button [routerLink]="['/records','patrons','edit', patron.pid]" class="btn btn-sm btn-primary ml-1 ng-star-inserted" id="profile-edit-button" title="Edit"><i class="fa fa-pen mr-1"></i>{{ 'Edit' | translate }}</button>
</div>
<h3 class="mb-3">{{ patron.last_name }} {{ patron.first_name }}</h3>
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional case management for first name.

Copy link
Contributor Author

@jma jma Nov 4, 2020

Choose a reason for hiding this comment

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

Yes but for the moment the first_name is a required property. A lot of changes will be needed if we decide to make it optional.

@jma jma force-pushed the maj-patron-change-password branch 2 times, most recently from 1336ad7 to 5e070cb Compare November 4, 2020 11:42
@@ -46,11 +46,17 @@ export class UserService {
return this._allowInterfaceAccess;
}

/** API Prefix, i.e. /api. */
private _apiPrefix = '';

constructor(
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing docstring for constructor

Copy link
Contributor

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

I think you can mention rero/rero-ils#1364 in your commit message.

Commit proposition, but check if it's correct:

patron: allows librarians to update user password

* Adds a button in the circulation patron profile to update the
  password.
* Adds an edit button in the circulation patron profile.
* Displays patron role in the patron information of the circulation
  module and in the patron detailed view.
* Allows searching patron by name in the checkout view, by searching in
  all fields.

@iGormilhit iGormilhit added this to the v0.14.0 milestone Nov 5, 2020
@jma jma force-pushed the maj-patron-change-password branch from 5e070cb to 76f2b1b Compare November 5, 2020 09:34
@jma jma force-pushed the maj-patron-change-password branch from 76f2b1b to 50b4fb6 Compare November 5, 2020 14:53
Copy link
Contributor

@AoNoOokami AoNoOokami left a comment

Choose a reason for hiding this comment

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

Tested and approved

@iGormilhit iGormilhit changed the title patron: a professional can change a user password patron: allows librarians to update user password Nov 9, 2020
Copy link
Contributor

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

Commit message approved.

* Adds a button in the circulation patron profile to update the
  password.
* Adds an edit button in the circulation patron profile.
* Displays patron role in the patron information of the circulation
  profile and in the patron detailed view.
* Sets a default value for the patron expiration date to now + 3 years.
* Allows searching patron by name in the checkout view, by searching in
  all fields, closes rero/rero-ils#1364.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
@jma jma force-pushed the maj-patron-change-password branch from a080c43 to 50fab80 Compare November 12, 2020 10:34
@jma jma merged commit 23b2f8d into rero:dev Nov 12, 2020
@jma jma deleted the maj-patron-change-password branch February 11, 2021 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants