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

users: display keep_history for patron #447

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

BadrAly
Copy link

@BadrAly BadrAly commented Nov 27, 2020

In the patron detailed view, the new field
keep_history is displayed.

Co-Authored-by: Aly Badr aly.badr@rero.ch

Why are you opening this PR?

https://tree.taiga.io/project/rero21-reroils/us/1422?milestone=277700

Dependencies

rero/invenio-userprofiles#2
rero/rero-ils#1512

How to test?

Case 1:

  • Login as system_librarian.
  • Find any patron with completed circ transactions
  • Edit the patron record and disable the keep_history field if enabled. (Note: if disabled already, then enable it and then disable it).
  • Go back the patron page, you will not find the the circ history. they are anonymized (not visible anymore)

Case 2:

  • Login as patron (simonetta or other).
  • Make sure you have completed circ transactions in the history tab.
  • Take a look at the Personal data tab (re-organiszed) and the new edit button to take you to the RERO_ID editor.
  • disable the keep_history field if enabled. (Note: if disabled already, then enable it and then disable it).
  • Go back the history tab, you will not find the the circ history. they are anonymized (not visible anymore)

Case 3:

  • repeat any of the above cases for a completed circulation transaction that has still fees to pay.
  • The transaction will not be anonymized
  • Pay the fees and the transaction will be anonymized with the execution of the daily job @7h00

Case 4:

  • Patron has the keep_history enabled. transaction older than 6 months will be anonymized

Case 5:

  • Patron has keep_history disabled and no transactions are visiable.
  • Patron enables the keep_history . The anonymized transactions will still be not visible.

Screenshot 2020-11-30 at 14 09 26

Screenshot 2020-11-30 at 14 08 08

Screenshot 2020-11-30 at 14 07 31

Code review check list

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

@BadrAly BadrAly self-assigned this Nov 30, 2020
@BadrAly BadrAly added the f: circulation Concerns the circulation interface or backend label Nov 30, 2020
@BadrAly BadrAly added this to the v0.15.0 milestone Nov 30, 2020
@BadrAly BadrAly marked this pull request as ready for review November 30, 2020 11:00
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.

  1. Question: do we actually want to edit the patron detailed view? Check User detailed view should be removed. rero-ils#1444
  2. Commit message proposition:
user: display keep_history for patron

* Displays the new field keep_history in the patron detailed view.

@BadrAly
Copy link
Author

BadrAly commented Nov 30, 2020

Question: do we actually want to edit the patron detailed view? Check rero/rero-ils#1444

@iGormilhit this will be removed when the task rero/rero-ils#1444 is done

@benerken
Copy link
Contributor

benerken commented Dec 1, 2020

  • First time: there is a discordance between public view and Pro view.
    Let's take Helder patron account (Public view) : he has a patron history disabled.
    image

Edit the patron record in the professional interface. The Toggle is enabled.

image

  • Second test: Update patron history (enable/disable) for Helder and it's OK now.
    Both views are correct : the toggle in the pro view + the flag in the public view.

@pronguen
Copy link
Contributor

pronguen commented Dec 1, 2020

  • Patron editor (professional)
    • "Keep history" instead of "History settings". It is a toggle so the user should be able to respond to the label by yes/no, enable/disable.
    • Description to correct: "If enabled, the loan history is saved for a maximum of six months. It is visible to the user and the library staff."
    • When adding a new patron, the "Keep history" should be disabled by default.
    • I could reproduce Benoit's problem of discordance between the detailed view and the editor (users: display keep_history for patron #447 (comment)). It is difficult to identify how to reproduce it.
  • Patron account (professional):
    • add in the Profile tab the "Keep history" info (as it is done for the patron detailed view).
    • disabling the history as a librarian does not work: indeed it removes the current history, but the new loans are displayed again in the history. To reproduce: 1. Open a user with history enabled and checkouts. 2. Disable history. 3. See that history isn't displayed. 4. Add transactions. 5. See that they appear in history.
    • If disabled, the loan history should not be shown, even for loans with fees. The US says that the corresponding tab disabled. Another option is to display a message "The loan history is disabled."
  • Patron account (public):
    • "Keep history" instead of "History settings". Don't put a label next to the green point. The account needs to have as little text as possible. See proposal below.
      image
    • If disabled, the loan history should not be shown, even for loans with fees. The US says that the corresponding tab disabled. Another option is to display a message "The loan history is disabled."
  • Patron editor (public).
    • "Keep History" > "Keep history"
    • Description to correct: "If enabled, ..."

@benerken
Copy link
Contributor

benerken commented Dec 3, 2020

  • In the public view: login with Helder
    Go to "Mon compte" -> "Données Personnelles" : the history is disabled.

image

 Click on "Edit" -> In the editor, the "Keep history" is enabled by default.
 If the patron doesn't take care, he will re-enable its history 

image

It's linked to the fact that Helder account has not yet been modified.

Mana has done the same test with Alain. She has enabled/disabled its history and it works.
So, a patron account with an history already disabled has a problem when he edits its account (the flag is enabled)

@benerken it is a minor problem. The user_profile is not up to date after executing the setup. I will fix this.
But as you noticed once the patron is edited for the first time, all goes well

@iGormilhit iGormilhit added the f: professional ui Professional interface label Dec 3, 2020
@iGormilhit iGormilhit changed the title user: display keep_history for patron users: display keep_history for patron Dec 10, 2020
* Displays the new field keep_history in the patron detailed view.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>
@BadrAly BadrAly merged commit 586dd0a into rero:dev Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: circulation Concerns the circulation interface or backend f: professional ui Professional interface f: user management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants