From e509673f7b304f73ca2c3dc19c64b4b2d493240c Mon Sep 17 00:00:00 2001
From: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
Date: Mon, 7 Feb 2022 10:50:53 +0100
Subject: [PATCH] interface: keep the viewcode in the urls

The change of the user's data as well as the change of
the password has been transferred to the Angular user profile.

* Removes the "edit my profile" menu entry.
* Removes the "Change password" menu entry.
* Closes #2195.

Co-Authored-by: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
---
 rero_ils/config.py                            |  9 +++++
 .../templates/rero_ils/ill_request_form.html  |  2 +-
 rero_ils/modules/ill_requests/views.py        | 12 ++++---
 .../templates/rero_ils/patron_profile.html    |  2 +-
 rero_ils/modules/patrons/views.py             | 18 +++++-----
 rero_ils/modules/users/views.py               | 36 ++++++++++++++++---
 rero_ils/theme/menus.py                       | 34 +++++-------------
 tests/api/users/test_users_rest.py            | 32 +++++++++++++----
 tests/ui/ill_requests/test_ill_requests_ui.py |  3 +-
 9 files changed, 94 insertions(+), 54 deletions(-)

diff --git a/rero_ils/config.py b/rero_ils/config.py
index 362a5c5f40..9ba333c899 100644
--- a/rero_ils/config.py
+++ b/rero_ils/config.py
@@ -257,6 +257,15 @@ def _(x):
 #: proxies) removes these headers again before sending the response to the
 #: client. Set to False, in case of doubt.
 ACCOUNTS_USERINFO_HEADERS = False
+
+# User profile
+RERO_PUBLIC_USERPROFILES_READONLY = False
+RERO_PUBLIC_USERPROFILES_READONLY_FIELDS = [
+    'first_name',
+    'last_name',
+    'birth_date'
+]
+
 # Disable User Profiles
 USERPROFILES = True
 USERPROFILES_COUNTRIES = get_profile_countries
diff --git a/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html b/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html
index fcf3f59b10..55651a03e1 100644
--- a/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html
+++ b/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html
@@ -27,7 +27,7 @@ <h4 class="alert-heading">{{ _('Caution!') }}</h4>
     <p class="mb-0">{{ _('A well detailed request is more likely to be satisfied') }}</p>
   </div>
 
-  <form id="ill-public-form" action="{{ url_for('ill_requests.ill_request_form') }}" method="POST" class="form" role="form" novalidate>
+  <form id="ill-public-form" action="{{ url_for('ill_requests.ill_request_form', viewcode=viewcode) }}" method="POST" class="form" role="form" novalidate>
     {{ form.hidden_tag() }}
     {%- if form.csrf_token and form.csrf_token.errors %}
       <div class="alert alert-danger" role="alert">
diff --git a/rero_ils/modules/ill_requests/views.py b/rero_ils/modules/ill_requests/views.py
index 0f4e446e05..5218a8eb8a 100644
--- a/rero_ils/modules/ill_requests/views.py
+++ b/rero_ils/modules/ill_requests/views.py
@@ -35,16 +35,16 @@
 blueprint = Blueprint(
     'ill_requests',
     __name__,
-    url_prefix='/ill_requests',
+    url_prefix='/<string:viewcode>',
     template_folder='templates',
     static_folder='static',
 )
 
 
-@blueprint.route('/create/', methods=['GET', 'POST'])
+@blueprint.route('/ill_requests/create/', methods=['GET', 'POST'])
 @check_user_is_authenticated(redirect_to='security.login')
 @check_logged_as_patron
-def ill_request_form():
+def ill_request_form(viewcode):
     """Return professional view."""
     form = ILLRequestForm(request.form)
     # pickup locations selection are based on app context then the choices
@@ -73,6 +73,8 @@ def get_patron(location_pid):
             _('The request has been transmitted to your library.'),
             'success'
         )
-        return redirect(url_for('patrons.profile', tab='ill_request'))
+        return redirect(url_for(
+            'patrons.profile', viewcode=viewcode, tab='ill_request'))
 
-    return render_template('rero_ils/ill_request_form.html', form=form)
+    return render_template('rero_ils/ill_request_form.html',
+                           form=form, viewcode=viewcode)
diff --git a/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html b/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html
index dde9590ac7..4bfc94fdb5 100644
--- a/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html
+++ b/rero_ils/modules/patrons/templates/rero_ils/patron_profile.html
@@ -24,7 +24,7 @@
 {%- endblock css %}
 
 {%- block body %}
-<public-patron-profile language="{{ current_i18n.locale.language[:2] }}"></public-patron-profile>
+<public-patron-profile language="{{ current_i18n.locale.language[:2] }}" viewcode="{{ viewcode }}"></public-patron-profile>
 {%- endblock body %}
 
 {%- block javascript %}
diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py
index 0b889e2a1e..c8277b9fbc 100644
--- a/rero_ils/modules/patrons/views.py
+++ b/rero_ils/modules/patrons/views.py
@@ -130,7 +130,13 @@ def logged_user():
             ),
             'librarianRoles': current_app.config.get(
                 'RERO_ILS_LIBRARIAN_ROLES', []
-            )
+            ),
+            'userProfile': {
+                'readOnly': current_app.config.get(
+                    'RERO_PUBLIC_USERPROFILES_READONLY', False),
+                'readOnlyFields': current_app.config.get(
+                    'RERO_PUBLIC_USERPROFILES_READONLY_FIELDS', []),
+            }
         }
     }
     if not current_user.is_authenticated:
@@ -138,9 +144,7 @@ def logged_user():
 
     user = User.get_by_id(current_user.id).dumpsMetadata()
     user['id'] = current_user.id
-    data = {**data, **user}
-    data['patrons'] = []
-
+    data = {**data, **user, 'patrons': []}
     for patron in Patron.get_patrons_by_user(current_user):
         organisation = patron.get_organisation()
         # TODO: need to be fixed this causes errors in production only
@@ -168,9 +172,7 @@ def logged_user():
     return jsonify(data)
 
 
-@blueprint.route('/global/patrons/profile', defaults={'viewcode': 'global'},
-                 methods=['GET', 'POST'])
-@blueprint.route('/<string:viewcode>/patrons/profile')
+@blueprint.route('/<string:viewcode>/patrons/profile', methods=['GET', 'POST'])
 @check_logged_as_patron
 @register_menu(
     blueprint,
@@ -185,7 +187,7 @@ def logged_user():
 )
 def profile(viewcode):
     """Patron Profile Page."""
-    return render_template('rero_ils/patron_profile.html')
+    return render_template('rero_ils/patron_profile.html', viewcode=viewcode)
 
 
 @blueprint.app_template_filter('format_currency')
diff --git a/rero_ils/modules/users/views.py b/rero_ils/modules/users/views.py
index 97824d220a..4ed57cbd0e 100644
--- a/rero_ils/modules/users/views.py
+++ b/rero_ils/modules/users/views.py
@@ -22,10 +22,14 @@
 import json
 from functools import wraps
 
-from flask import request
+from flask import abort, request
+from flask_login import current_user
 from invenio_rest import ContentNegotiatedMethodView
 
+from rero_ils.modules.decorators import check_logged_user_authentication
+
 from .api import User
+from ...modules.patrons.api import current_librarian
 from ...permissions import login_and_librarian
 
 
@@ -42,6 +46,24 @@ def is_logged_librarian(*args, **kwargs):
     return is_logged_librarian
 
 
+def check_user_permission(fn):
+    """Decorate to check permission access.
+
+    The access is allow when the connected user is a librarian or
+    the user id is the same of the id argument.
+    """
+    @wraps(fn)
+    def is_logged(*args, **kwargs):
+        """Decorated view."""
+        if not current_user.is_authenticated:
+            abort(401)
+        if not current_librarian and 'id' in kwargs and \
+            current_user.id != kwargs.get('id'):
+            abort(401)
+        return fn(*args, **kwargs)
+    return is_logged
+
+
 class UsersResource(ContentNegotiatedMethodView):
     """User REST resource."""
 
@@ -67,13 +89,13 @@ def __init__(self, **kwargs):
             **kwargs
         )
 
-    @check_permission
+    @check_user_permission
     def get(self, id):
         """Implement the GET."""
         user = User.get_by_id(id)
         return user.dumps()
 
-    @check_permission
+    @check_user_permission
     def put(self, id):
         """Implement the PUT."""
         user = User.get_by_id(id)
@@ -106,7 +128,7 @@ def __init__(self, **kwargs):
             **kwargs
         )
 
-    @check_permission
+    @check_user_permission
     def get(self):
         """Get user info for the professionnal view."""
         email_or_username = request.args.get('q', None).strip()
@@ -131,7 +153,11 @@ def get(self):
             user = User.get_by_username_or_email(email_or_username)
         if not user:
             return hits
-        data = user.dumps()
+        # if librarian: send all user data
+        # if patron: send only the user id
+        user_data = user.dumps()
+        data = user_data if current_librarian else \
+            {'id': user_data.get('id')}
         hits['hits']['hits'].append(data)
         hits['hits']['total']['value'] = 1
         return hits
diff --git a/rero_ils/theme/menus.py b/rero_ils/theme/menus.py
index cd08f57419..c8bab85b18 100644
--- a/rero_ils/theme/menus.py
+++ b/rero_ils/theme/menus.py
@@ -128,6 +128,10 @@ def init_menu_tools():
     rero_register(
         item,
         endpoint='ill_requests.ill_request_form',
+        endpoint_arguments_constructor=lambda: dict(
+            viewcode=request.view_args.get(
+                'viewcode', current_app.config.get(
+                    'RERO_ILS_SEARCH_GLOBAL_VIEW_CODE'))),
         visible_when=lambda: bool(current_patrons),
         text=TextWithIcon(
             icon='<i class="fa fa-shopping-basket"></i>',
@@ -309,6 +313,10 @@ def init_menu_profile():
     rero_register(
         item,
         endpoint=profile_endpoint,
+        endpoint_arguments_constructor=lambda: dict(
+            viewcode=request.view_args.get(
+                'viewcode', current_app.config.get(
+                    'RERO_ILS_SEARCH_GLOBAL_VIEW_CODE'))),
         visible_when=lambda: len(current_patrons) > 0,
         text=TextWithIcon(
             icon='<i class="fa fa-book"></i>',
@@ -318,32 +326,6 @@ def init_menu_profile():
         id='profile-menu',
     )
 
-    item = current_menu.submenu('main.profile.edit_profile')
-    rero_register(
-        item,
-        endpoint='invenio_userprofiles.profile',
-        visible_when=lambda: current_user.is_authenticated,
-        text=TextWithIcon(
-            icon='<i class="fa fa-user"></i>',
-            text='Edit my profile'
-        ),
-        order=1,
-        id='profile-menu',
-    )
-
-    item = current_menu.submenu('main.profile.change_password')
-    rero_register(
-        item,
-        endpoint='security.change_password',
-        visible_when=lambda: current_user.is_authenticated,
-        text=TextWithIcon(
-            icon='<i class="fa fa-lock"></i>',
-            text='Change password'
-        ),
-        order=1,
-        id='profile-menu',
-    )
-
     # Endpoint for:
     # Application: invenio_oauth2server_settings.index
     # Security: invenio_accounts.security
diff --git a/tests/api/users/test_users_rest.py b/tests/api/users/test_users_rest.py
index 3c317625fa..404d288969 100644
--- a/tests/api/users/test_users_rest.py
+++ b/tests/api/users/test_users_rest.py
@@ -139,7 +139,7 @@ def test_users_search_api(client, librarian_martigny, patron_martigny):
     assert hits['hits']['hits'] == []
     assert hits['hits']['total']['value'] == 0
 
-    # all by username
+    # # all by username
     res = client.get(
         url_for(
             'api_users.users_list',
@@ -148,10 +148,11 @@ def test_users_search_api(client, librarian_martigny, patron_martigny):
     )
     assert res.status_code == 200
     hits = get_json(res)
-    assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id']
+    assert hits['hits']['hits'][0]['metadata']['username'] == \
+        patron_martigny['username']
     assert hits['hits']['total']['value'] == 1
 
-    # all by email
+    # # all by email
     res = client.get(
         url_for(
             'api_users.users_list',
@@ -160,7 +161,8 @@ def test_users_search_api(client, librarian_martigny, patron_martigny):
     )
     assert res.status_code == 200
     hits = get_json(res)
-    assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id']
+    assert hits['hits']['hits'][0]['metadata']['username'] == \
+        patron_martigny['username']
     assert hits['hits']['total']['value'] == 1
 
     # by username
@@ -172,7 +174,8 @@ def test_users_search_api(client, librarian_martigny, patron_martigny):
     )
     assert res.status_code == 200
     hits = get_json(res)
-    assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id']
+    assert hits['hits']['hits'][0]['metadata']['username'] == \
+        patron_martigny['username']
     assert hits['hits']['total']['value'] == 1
 
     # by email
@@ -184,7 +187,8 @@ def test_users_search_api(client, librarian_martigny, patron_martigny):
     )
     assert res.status_code == 200
     hits = get_json(res)
-    assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id']
+    assert hits['hits']['hits'][0]['metadata']['username'] == \
+        patron_martigny['username']
     assert hits['hits']['total']['value'] == 1
 
     # by uppercase email
@@ -196,5 +200,19 @@ def test_users_search_api(client, librarian_martigny, patron_martigny):
     )
     assert res.status_code == 200
     hits = get_json(res)
-    assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id']
+    assert hits['hits']['hits'][0]['metadata']['username'] == \
+        patron_martigny['username']
     assert hits['hits']['total']['value'] == 1
+
+    # Login with patron role
+    login_user_via_session(client, p_martigny.user)
+    res = client.get(
+        url_for(
+            'api_users.users_list',
+            q=patron_martigny['username']
+        )
+    )
+    assert res.status_code == 200
+    hits = get_json(res)
+    assert 'metadata' not in hits['hits']['hits'][0]
+    assert hits['hits']['hits'][0]['id'] == patron_martigny['user_id']
diff --git a/tests/ui/ill_requests/test_ill_requests_ui.py b/tests/ui/ill_requests/test_ill_requests_ui.py
index c09232fa97..4474f220f7 100644
--- a/tests/ui/ill_requests/test_ill_requests_ui.py
+++ b/tests/ui/ill_requests/test_ill_requests_ui.py
@@ -29,7 +29,8 @@ def test_ill_request_create_request_form(client, app,
                                          loc_public_martigny,
                                          patron_martigny):
     """ test ill request create form."""
-    request_form_url = url_for('ill_requests.ill_request_form')
+    request_form_url = url_for(
+        'ill_requests.ill_request_form', viewcode='global')
 
     # Not logged user don't have access to request_form. It is redirected to
     # login form