users: add new fields, move fields from the patron #1695

jma · 2021-02-11T16:14:56Z

Replaces the phone field by home, business, mobile, other phone fields
in the user data model.
Adds country and gender fields in the user data model.
Adds 3 invenio-userprofiles configuration to specify the list of
countries, the default country and the read only fields.
Makes the barcode repetitive.
Removes all user fields from the patron data model.
Adds the second address, the local code, the source fields in the
patron data model.
Closes A user get an internal servor error as he wants to save changes in his profile #1724, The affiliation library disappears in some cases in the patron editor #1634, When the creation of a patron fails, the reset password email is sent anyway #1615, No feedback is given to the user if the email is not confirmed #1490, Creating a patron with a username and an email corresponding to two different invenio user cause unpredictable errors #1467, Fields name and birth date should not be editable by a user/patron #1318, Add facets in the user result list #1384.

Co-Authored-by: Johnny Mariéthoz [email protected]

Why are you opening this PR?

https://tree.taiga.io/project/rero21-reroils/us/1905?milestone=284341

Dependencies

My PR depends on the following rero-ils-ui's PR(s):

How to test?

check several scenarios of online registration and then create a patron
create a patron directly
check existing username, email, etc.

Code review check list

iGormilhit

Commit message proposition:

users: add new fields, move fields from the patron

* Replaces the phone field by home, business, mobile, other phone fields
  in the user data model.
* Adds country and gender fields in the user data model.
* Adds 3 invenio-userprofiles configuration to specify the list of
  countries, the default country and the read only fields.
* Makes the barcode repetitive.
* Removes all user fields from the patron data model.
* Adds the second address, the local code, the source fields in the
  patron data model.

iGormilhit · 2021-03-01T07:32:53Z

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

+    "secound_address": {
+      "title": "Secound address",


Suggested change

"secound_address": {

"title": "Secound address",

"second_address": {

"title": "Second address",

iGormilhit · 2021-03-01T07:33:06Z

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

@@ -6,10 +6,13 @@
  "additionalProperties": false,
  "propertiesOrder": [
    "user_id",
+    "secound_address",


Suggested change

"secound_address",

"second_address",

iGormilhit · 2021-03-01T07:35:47Z

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

@@ -6,10 +6,13 @@
  "additionalProperties": false,
  "propertiesOrder": [
    "user_id",
+    "secound_address",


Suggested change

"secound_address",

"second_address",

iGormilhit · 2021-03-01T07:36:10Z

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

+    "secound_address": {
+      "title": "Secound address",


Suggested change

"secound_address": {

"title": "Secound address",

"second_address": {

"title": "Second address",

iGormilhit · 2021-03-01T07:38:49Z

rero_ils/modules/patrons/mappings/v7/patrons/patron-v0.0.1.json

+      "country": {
+        "type": "text"
+      },
+      "secound_address": {


Suggested change

"secound_address": {

"second_address": {

rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json

iGormilhit · 2021-03-01T07:43:08Z

rero_ils/modules/users/jsonschemas/users/user-v0.0.1.json

+      "title": "business phone number",
+      "description": "business phone number with the international prefix, without spaces.",


Suggested change

"title": "business phone number",

"description": "business phone number with the international prefix, without spaces.",

"title": "Business phone number",

"description": "Business phone number with the international prefix, without spaces.",

* Adds a new 'user' resource to manage the user profiles. It contains the user personal data as opposed to the patron data that are link to a specific organisation. * Writes a REST API: * A GET, POST, PUT methods to create, update and retrieves the patron personal information. * A search endpoint to retrieve the patron personal information given an email or a username. * Creates units testing. * Centralizes several views and methods in a specific `User` python class. * Disables the validation email for freshly created user. * Uses the standard ngx-formly `widget` field for the user JSON schema. Co-Authored-by: Aly Badr <[email protected]> Co-Authored-by: Johnny Mariéthoz <[email protected]>

iGormilhit

Commit messages approved.

benerken · 2021-03-02T07:31:18Z

the patron account screen is "slim" : is it possible to use a larger part of the screen ?
The message doesn't change from " The loan history is not saved." even if I modified to keep the history.
The status button of the ILL requests is not aligned correctly. Ex: https://ilsdev.test.rero.ch/global/patrons/profile?tab=ill_request

pronguen

In the professional view
- (!) I am not able to add a new user (personal data).
- When creating a new user, it is very difficult to find Switzerland in the list of countries. Ideally put Switzerland as default value (this should then be different for UCL). Also sort the list alphabetically (as in provisionActivity in the document editor), or put the few preferred values on top.
In the public view
- In account/settings/profile/, some field labels should not have uppercase letter for the 2nd word (Postal Code, Birth Date)
- In account/settings/profile/, if I update my profile, and then change the page language to English (for example), I land on /lang/?lang_code=en and receive a Page not found.
- From RERO ILS homepage, when I click on "My account", it lasts long before displaying the account. A spinner would be great.
- Some responsive problems: in the ILL requests tab
- Some responsive problems: in the fees tab

jma · 2021-03-03T06:37:51Z

In the public view

In account/settings/profile/, if I update my profile, and then change the page language to English (for example), I land on /lang/?lang_code=en and receive a Page not found.

This is an Invenio bug, present also on SONAR. An issue has been open: inveniosoftware/invenio-userprofiles#119

All of the following problem will be done in an other US (patron profile using Angular):

From RERO ILS homepage, when I click on "My account", it lasts long before displaying the account. A spinner would be great.

Some responsive problems: in the ILL requests tab

Some responsive problems: in the fees tab

[] The status button of the ILL requests is not aligned correctly. Ex: https://ilsdev.test.rero.ch/global/patrons/profile?tab=ill_request

BadrAly · 2021-03-03T06:31:29Z

rero_ils/modules/items/cli.py

@@ -331,7 +331,7 @@ def get_patrons_barcodes():
    barcodes = []
    for uuid in patrons_ids:
        patron = Patron.get_record_by_id(uuid)
-        barcode = patron.patron.get('barcode')
+        patron_barcodes = patron.patron.get('barcode')
        if barcode:


if patron_barcodes ?

BadrAly · 2021-03-03T06:36:19Z

rero_ils/modules/loans/cli.py

@@ -394,8 +394,8 @@ def get_random_patron(exclude_this_barcode):
        .filter('term', organisation__pid=org_pid)\
        .source(['patron']).scan()
    for patron in patrons:
-        if patron.patron.barcode != exclude_this_barcode:
-            return Patron.get_patron_by_barcode(barcode=patron.patron.barcode)
+        if patron.patron.barcode[0] != exclude_this_barcode:


since a patron can have multiple barcodes, this check is not valid anymore. Need to check that the exclude_this_barcode is not in the list patron.patron.barcode

BadrAly · 2021-03-03T06:37:46Z

rero_ils/modules/patrons/cli.py

@@ -122,6 +122,7 @@ def import_users(infile, append, verbose, password, lazy, dont_stop_on_error,
            patron.reindex()
            pids.append(patron.pid)
        except Exception as err:
+            raise(err)


need to comment this line?

BadrAly · 2021-03-03T06:45:48Z

rero_ils/modules/utils.py

@@ -303,7 +314,7 @@ def generate_item_barcode(data=None):
        data['barcode'] = 'f-{}'.format(
            datetime.now().strftime('%Y%m%d%I%M%S%f'))
    if data.get('patron') and not data.get('patron', {}).get('barcode'):
-        data['patron']['barcode'] = 'f-{}'.format(
+        data['patron']['barcode'][0] = 'f-{}'.format(


data['patron']['barcode'] = ['f-{}'.format(
datetime.now().strftime('%Y%m%d%I%M%S%f'))]

You are right. But as the barcode is required for a patron, this case should never happens! Can I remove this?

lauren-d · 2021-03-03T07:50:36Z

rero_ils/modules/patrons/templates/rero_ils/_patron_profile_head.html

@@ -21,6 +21,6 @@
    <i class="fa fa-user fa-5x"></i>
  </div>
  <hgroup class="col-sm-10 col-lg-11 align-self-center">
-    <h1 class="mb-0">{{ record.first_name }} {{ record.last_name }}</h1>
+    <h1 class="mb-0">{{ record.dumps().first_name }} {{ record.dumps().last_name }}</h1>


I think this is better to store result of dumps into a local variable because dumps() do a call to database (see patrons.api.dumps())
Maybe that could be done in the parent template (patron_profile.html) or as argument when calling the template

lauren-d · 2021-03-03T07:53:42Z

rero_ils/modules/patrons/templates/rero_ils/_patron_profile_personal.html

    <div class="row pl-2 pr-2">
      <div class="col-lg-3 font-weight-bold">
        {{_('Keep history')}}:
        </a>
      </div>
      <div class="col-lg-9">
        <i class="fa fa-circle mr- text-{{ 'success' if record.patron.keep_history else 'danger' }}"></i>
-        {% if record.patron.keep_history %}
+        {% if record.dumps().keep_history %}


same before

lauren-d · 2021-03-03T07:54:39Z

rero_ils/modules/patrons/templates/rero_ils/patron_profile.html

@@ -59,7 +59,7 @@
            <span class="badge badge-danger font-weight-normal">{{ fees.open.total_amount | format_currency(fees.open.currency) }}</span>
          </a>
        </li>
-        {% if record.patron.keep_history %}
+        {% if record.dumps().keep_history %}


same before

lauren-d · 2021-03-03T08:05:42Z

tests/api/test_permissions_librarian.py


        res, _ = postdata(
            client,
            post_entrypoint,
            data
        )
+        print(res.json)


could be removed

* Replaces the phone field by home, business, mobile, other phone fields in the user data model. * Adds country and gender fields in the user data model. * Adds 3 invenio-userprofiles configuration to specify the list of countries, the default country and the read only fields. * Makes the barcode repetitive. * Removes all user fields from the patron data model. * Adds the second address, the local code, the source fields in the patron data model. * Closes rero#1724, rero#1634, rero#1615, rero#1490, rero#1467, rero#1318, rero#1384, rero#1670. Co-Authored-by: Johnny Mariéthoz <[email protected]>

jma force-pushed the maj-user-patron-add-new-fields branch 5 times, most recently from 5956da6 to 30ea20e Compare February 24, 2021 13:58

jma mentioned this pull request Feb 24, 2021

patrons: edit the user personal data in a modal rero/rero-ils-ui#529

Merged

7 tasks

jma force-pushed the maj-user-patron-add-new-fields branch from 30ea20e to c249e2b Compare February 24, 2021 14:19

jma changed the title ~~in progress~~ user: add new fields Feb 24, 2021

jma force-pushed the maj-user-patron-add-new-fields branch from c249e2b to b042219 Compare February 24, 2021 14:26

jma marked this pull request as ready for review February 24, 2021 14:31

jma force-pushed the maj-user-patron-add-new-fields branch 6 times, most recently from 98d13ee to 35ce713 Compare March 1, 2021 07:04

iGormilhit suggested changes Mar 1, 2021

View reviewed changes

jma force-pushed the maj-US1889-patron-edit-personnal-data branch from 729a78f to ea9b133 Compare March 1, 2021 08:22

jma force-pushed the maj-user-patron-add-new-fields branch 2 times, most recently from 1828eae to 4b77046 Compare March 1, 2021 08:31

jma requested a review from iGormilhit March 1, 2021 08:38

jma force-pushed the maj-user-patron-add-new-fields branch 2 times, most recently from 377ebb6 to fdf434b Compare March 1, 2021 09:40

iGormilhit changed the title ~~user: add new fields~~ users: add new fields, move fields from the patron Mar 1, 2021

iGormilhit approved these changes Mar 1, 2021

View reviewed changes

jma force-pushed the maj-user-patron-add-new-fields branch 3 times, most recently from 3cc4c88 to 702696c Compare March 1, 2021 13:18

jma requested a review from BadrAly March 1, 2021 13:28

jma requested a review from lauren-d March 1, 2021 13:28

pronguen self-requested a review March 2, 2021 11:03

pronguen suggested changes Mar 2, 2021

View reviewed changes

BadrAly suggested changes Mar 3, 2021

View reviewed changes

jma force-pushed the maj-user-patron-add-new-fields branch from 702696c to a9f6dc7 Compare March 3, 2021 07:03

jma requested a review from BadrAly March 3, 2021 07:04

lauren-d approved these changes Mar 3, 2021

View reviewed changes

jma force-pushed the maj-user-patron-add-new-fields branch from a9f6dc7 to fb8f162 Compare March 3, 2021 08:20

jma merged commit dd8ef3f into rero:maj-US1889-patron-edit-personnal-data Mar 3, 2021

jma deleted the maj-user-patron-add-new-fields branch January 13, 2022 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

users: add new fields, move fields from the patron #1695

users: add new fields, move fields from the patron #1695

jma commented Feb 11, 2021 •

edited

Loading

iGormilhit left a comment

iGormilhit Mar 1, 2021

iGormilhit Mar 1, 2021

iGormilhit Mar 1, 2021

iGormilhit Mar 1, 2021

iGormilhit Mar 1, 2021

iGormilhit Mar 1, 2021

iGormilhit left a comment

benerken commented Mar 2, 2021 •

edited by jma

Loading

pronguen left a comment •

edited by jma

Loading

jma commented Mar 3, 2021

BadrAly Mar 3, 2021

BadrAly Mar 3, 2021

BadrAly Mar 3, 2021

BadrAly Mar 3, 2021

jma Mar 3, 2021

lauren-d Mar 3, 2021

lauren-d Mar 3, 2021

lauren-d Mar 3, 2021

lauren-d Mar 3, 2021

		"title": "business phone number",
		"description": "business phone number with the international prefix, without spaces.",

users: add new fields, move fields from the patron #1695

users: add new fields, move fields from the patron #1695

Conversation

jma commented Feb 11, 2021 • edited Loading

Why are you opening this PR?

Dependencies

How to test?

Code review check list

iGormilhit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iGormilhit left a comment

Choose a reason for hiding this comment

benerken commented Mar 2, 2021 • edited by jma Loading

pronguen left a comment • edited by jma Loading

Choose a reason for hiding this comment

jma commented Mar 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jma commented Feb 11, 2021 •

edited

Loading

benerken commented Mar 2, 2021 •

edited by jma

Loading

pronguen left a comment •

edited by jma

Loading