From e71c5c2efb955f6e56dcbb87aa39cbd2ab1df6bb Mon Sep 17 00:00:00 2001 From: ayumi-nishida Date: Wed, 15 Oct 2025 17:56:11 +0900 Subject: [PATCH 01/19] =?UTF-8?q?GakuNin=20mAP=E3=82=B0=E3=83=AB=E3=83=BC?= =?UTF-8?q?=E3=83=97=E3=81=A8WEKO3=E3=82=B0=E3=83=AB=E3=83=BC=E3=83=97?= =?UTF-8?q?=E3=81=AE=E7=B5=B1=E5=90=88=E5=8C=96=E3=81=AE=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../invenio_accounts/admin.py | 20 ++- .../invenio_accounts/models.py | 20 +++ modules/invenio-accounts/tests/test_admin.py | 78 ++++++++++- modules/invenio-accounts/tests/test_models.py | 39 ++++++ .../invenio_communities/admin.py | 15 +- .../invenio_communities/models.py | 22 +++ .../weko-index-tree/weko_index_tree/api.py | 15 +- .../weko-index-tree/weko_index_tree/utils.py | 131 ++++++++++++++---- modules/weko-workflow/weko_workflow/admin.py | 4 +- 9 files changed, 307 insertions(+), 37 deletions(-) diff --git a/modules/invenio-accounts/invenio_accounts/admin.py b/modules/invenio-accounts/invenio_accounts/admin.py index 23956d75de..3bb6545068 100644 --- a/modules/invenio-accounts/invenio_accounts/admin.py +++ b/modules/invenio-accounts/invenio_accounts/admin.py @@ -88,7 +88,10 @@ def scaffold_form(self): form_class = super(UserView, self).scaffold_form() form_class.role = QuerySelectMultipleField( 'Roles', - query_factory=lambda: Role.query.filter(~Role.name.like('%_groups_%')).all(), + query_factory=lambda: Role.query.filter( + ~Role.name.like('%_groups_%'), + ~Role.name.like('jc%roles%') + ).all(), get_label='name', widget=Select2Widget(multiple=True) ) @@ -331,6 +334,21 @@ def after_model_change(self, form, model, is_created): current_app.logger.error(str(ex)) db.session.rollback() + def get_query(self): + """ + Get the query for filtering roles. + """ + return super().get_query().filter(Role.name.in_( + [role.name for role in Role.query.all() if role.info_display] + )) + + def get_count_query(self): + """ + Get the count query for filtering roles. + """ + return super().get_count_query().filter(Role.name.in_( + [role.name for role in Role.query.all() if role.info_display] + )) class SessionActivityView(ModelView): """Admin view for user sessions.""" diff --git a/modules/invenio-accounts/invenio_accounts/models.py b/modules/invenio-accounts/invenio_accounts/models.py index b7fb56ea85..ab123da84d 100644 --- a/modules/invenio-accounts/invenio_accounts/models.py +++ b/modules/invenio-accounts/invenio_accounts/models.py @@ -47,6 +47,26 @@ def __str__(self): """Return the name and description of the role.""" return '{0.name} - {0.description}'.format(self) + @property + def info_display(self): + """Return organized role info as dict.""" + roles_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + role_mapping = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_mapping"] + role_name = self.name + # roles_keyやrole_mappingに含まれる場合は渡さない + suffix = None + if roles_key in role_name: + suffix = role_name.split(roles_key + '_')[-1] + if (roles_key in role_name and suffix and suffix not in role_mapping.keys()) or (roles_key not in role_name): + return { + "id": self.id, + "name": role_name, + "description": self.description + } + + # それ以外はNone + return None + class User(db.Model, UserMixin): """User data model.""" diff --git a/modules/invenio-accounts/tests/test_admin.py b/modules/invenio-accounts/tests/test_admin.py index 88826344a3..fca7042f27 100644 --- a/modules/invenio-accounts/tests/test_admin.py +++ b/modules/invenio-accounts/tests/test_admin.py @@ -19,7 +19,7 @@ from invenio_accounts.cli import users_create from invenio_accounts.models import SessionActivity, User, Role from invenio_accounts.testutils import create_test_user, login_user_via_view -from invenio_accounts.admin import SessionActivityView, UserView +from invenio_accounts.admin import SessionActivityView, UserView, RoleView _datastore = LocalProxy( lambda: current_app.extensions['security'].datastore @@ -283,3 +283,79 @@ def test_userview_edit_form(app, users): view = UserView(User, db.session) form = view.edit_form() assert form.data['active'] is False + +# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_scaffold_form -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp +def test_scaffold_form(app): + """Test scaffold_form method of UserView.""" + with app.app_context(): + db.session.add(Role(id=1, name='Contributor', description=None)) + db.session.add(Role(id=2, name='jc_xxx_roles_contributor', description=None)) + db.session.add(Role(id=3, name='jc_xxx_groups_yyy', description=None)) + db.session.commit() + + view = UserView(User, db.session) + form_class = view.scaffold_form() + + roles = form_class.role.kwargs['query_factory']() + groups = form_class.group.kwargs['query_factory']() + + role_names = [r.name for r in roles] + group_names = [g.name for g in groups] + assert 'Contributor' in role_names + assert 'jc_xxx_groups_yyy' in group_names + +# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_roleview_get_query -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp +def test_roleview_get_query(app): + """Test RoleView.get_query method.""" + with app.app_context(): + app.config.update(dict( + WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT= + { + "role_keyword": "roles", + "role_mapping": { + "repoadm": "Repository Administrator", + "comadm": "Community Administrator", + "contributor": "Contributor", + } + } + )) + role1 = Role(id=1, name="Contributor", description=None) + role2 = Role(id=2, name="jc_xxx_roles_contributor", description=None) + role3 = Role(id=3, name="jc_xxx_groups_yyy", description=None) + db.session.add_all([role1, role2, role3]) + db.session.commit() + + view = RoleView(Role, db.session) + # get_queryの実行 + query = view.get_query() + result_names = [r.name for r in query.all()] + # info_displayがNoneでないロールのみ取得できること + assert "Contributor" in result_names + assert "jc_xxx_groups_yyy" in result_names + assert "jc_xxx_roles_contributor" not in result_names + +# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_roleview_get_count_query -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp +def test_roleview_get_count_query(app): + """Test RoleView.get_count_query method.""" + with app.app_context(): + app.config.update(dict( + WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT= + { + "role_keyword": "roles", + "role_mapping": { + "repoadm": "Repository Administrator", + "comadm": "Community Administrator", + "contributor": "Contributor", + } + } + )) + role1 = Role(id=1, name="Contributor", description=None) + role2 = Role(id=2, name="jc_xxx_roles_contributor", description=None) + role3 = Role(id=3, name="jc_xxx_groups_yyy", description=None) + db.session.add_all([role1, role2, role3]) + db.session.commit() + + view = RoleView(Role, db.session) + query = view.get_count_query() + count = query.scalar() + assert count == 2 diff --git a/modules/invenio-accounts/tests/test_models.py b/modules/invenio-accounts/tests/test_models.py index 6866b888ec..d71152d4b5 100644 --- a/modules/invenio-accounts/tests/test_models.py +++ b/modules/invenio-accounts/tests/test_models.py @@ -73,3 +73,42 @@ def test_get_email_by_id(app, users): with app.test_client() as client: lst = User.get_email_by_id(1) assert len(lst) > 0 + +# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_models.py::test_info_display -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp +def test_info_display(app): + """Test Role.info_display property.""" + from invenio_accounts.models import Role + with app.app_context(): + app.config.update(dict( + WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT= + { + "role_keyword": "roles", + "role_mapping": { + "repoadm": "Repository Administrator", + "comadm": "Community Administrator", + "contributor": "Contributor", + } + } + )) + role1 = Role(id=1, name="Contributor", description=None) + role2 = Role(id=2, name="jc_xxx_roles_contributor", description=None) + role3 = Role(id=3, name="jc_xxx_groups_yyy", description=None) + db.session.add_all([role1, role2, role3]) + db.session.commit() + + # role_keywordを含み、role_mappingに含まれないsuffix + assert role1.info_display == { + "id": 1, + "name": "Contributor", + "description": None + } + + # role_keywordを含み、role_mappingに含まれるsuffix + assert role2.info_display is None + + # role_keywordを含まない + assert role3.info_display == { + "id": 3, + "name": "jc_xxx_groups_yyy", + "description": None + } diff --git a/modules/invenio-communities/invenio_communities/admin.py b/modules/invenio-communities/invenio_communities/admin.py index dc1913abbc..ee21ba1065 100644 --- a/modules/invenio-communities/invenio_communities/admin.py +++ b/modules/invenio-communities/invenio_communities/admin.py @@ -83,6 +83,11 @@ class CommunityModelView(ModelView): column_searchable_list = ('id', 'title', 'description') edit_template = "invenio_communities/admin/edit.html" + column_formatters = { + 'owner': lambda v, c, m, p: m.owner_display, + 'owner.name': lambda v, c, m, p: m.owner_display + } + @expose('/new/', methods=['GET', 'POST']) def create_view(self): """Create a new model. @@ -99,7 +104,7 @@ def create_view(self): if(request.method == 'POST'): try: form_data = request.form.to_dict() - + # Validate community ID self.validate_community_id(form_data['id']) # Validate title length @@ -224,7 +229,7 @@ def edit_view(self, id): form.id.data = model.id or '' form.cnri.data = model.cnri or '' form.title.data = model.title or '' - form.owner.data = model.owner or '' + form.owner.data = model.owner_display or '' form.index.data = model.index or '' form.group.data = model.group or '' form.description.data = model.description or '' @@ -555,6 +560,12 @@ def validate_community_id(self, community_id): "title": { "validators": [Length(max=255)] }, + 'owner': { + 'allow_blank': False, + 'query_factory': lambda: db.session.query(Role).filter( + ~Role.name.like('jc%roles%') + ).all(), + }, 'group': { 'allow_blank': False, 'query_factory': lambda: db.session.query(Role).filter(Role.name.like("%_groups_%")).all() diff --git a/modules/invenio-communities/invenio_communities/models.py b/modules/invenio-communities/invenio_communities/models.py index d79504f978..84b532f9ee 100644 --- a/modules/invenio-communities/invenio_communities/models.py +++ b/modules/invenio-communities/invenio_communities/models.py @@ -582,6 +582,28 @@ def version_id(self): return hashlib.sha1('{0}__{1}'.format( self.id, self.updated).encode('utf-8')).hexdigest() + @property + def owner_display(self): + """ + Get the display name of the community owner. + + This property returns a user-friendly owner name for the community. + If the owner's name contains a role keyword defined in the system configuration, + it replaces the suffix with a mapped display name for clarity. + Otherwise, it returns the owner's name as-is. + + Returns: + str: Display name of the community owner. + """ + if self.owner and hasattr(self.owner, 'name'): + roles_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + role_mapping = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_mapping"] + owner_name = self.owner.name + if owner_name and roles_key in owner_name: + suffix = owner_name.split(roles_key + '_')[-1] + if suffix in role_mapping.keys(): + owner_name = role_mapping[suffix] + return owner_name class FeaturedCommunity(db.Model, Timestamp): """Represent a featured community.""" diff --git a/modules/weko-index-tree/weko_index_tree/api.py b/modules/weko-index-tree/weko_index_tree/api.py index 0b40d57b8e..46c6209e43 100644 --- a/modules/weko-index-tree/weko_index_tree/api.py +++ b/modules/weko-index-tree/weko_index_tree/api.py @@ -886,10 +886,13 @@ def _get_allow_deny(allow, role, browse_flag=False): while role: tmp = role.pop(0) if tmp["name"] not in current_app.config['WEKO_PERMISSION_SUPER_ROLE_USER']: - if str(tmp["id"]) in allow: - alw.append(tmp) - else: - deny.append(tmp) + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + if tmp["name"] not in role_key and not (tmp["name"].startswith(prefix)): + if str(tmp["id"]) in allow: + alw.append(tmp) + else: + deny.append(tmp) return alw, deny def _get_group_allow_deny(allow_group_id=[], groups=[]): @@ -2185,11 +2188,11 @@ def get_handle_index_url(cls, indexid): @classmethod def bind_roles_including_permission(cls, roles, permission): """Bind roles including permissions. - + Args: roles (list): List of roles. permission (bool): Permission of default browsing or contribute. - + Returns: list: List of roles what binded. """ diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index 6e868164ef..6d5e12099c 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -126,7 +126,6 @@ def can_user_access_index(lst): bool: True if the user has access, False otherwise. """ from weko_records_ui.utils import is_future - result, roles = get_user_roles(is_super_role=True) groups = get_user_groups() @@ -307,24 +306,77 @@ def get_user_groups(): groups = Group.query_by_user(current_user, eager=False) for group in groups: grps.append(group.id) - return grps +def check_roles(user_role, roles, params): + """ + Check whether the user has access to an index based on roles and role groups. + + This function determines if a user can access an index by evaluating: + - Whether the user is an administrator (always True) + - Whether the user's roles or role groups match those set for the index + - Whether guest access is allowed (when '-99' is set and no role groups are configured) + - If neither roles nor role groups are set for the index, internal group check is performed + + Args: + user_role (tuple): (is_admin, [role_id, ...]) where is_admin is a bool. + roles (str or list): List or comma-separated string of role IDs set for the index. + params (dict): Dictionary containing: + - "role_groups": list of role group IDs set for the index + - "groups": list of group IDs the user belongs to + - "access_group_ids": group IDs set for the index + + Returns: + bool: True if the user has access to the index, False otherwise. + """ + + is_can = False + group_perm = False + role_perm = False -def check_roles(user_role, roles): - """Check roles.""" - is_can = True - if isinstance(roles, str): - roles = roles.split(',') if not user_role[0]: + roles_list = sorted(roles if isinstance(roles, list) else (roles.split(',') if roles else [])) + set_index_group = any(int(r) in params["role_groups"] for r in roles_list) + set_index_role = any(int(r) not in params["role_groups"] for r in roles_list) if current_user.is_authenticated: self_role = user_role[1] or ['-98'] - for role in self_role: - if str(role) not in (roles or []): + if set_index_group: + index_role_group = [int(r) for r in roles_list if int(r) in params["role_groups"]] + if index_role_group: + # Groups(role) are set for the index. + group_perm = any( + str(rg) in [str(r) for r in roles_list] and str(rg) in [str(sr) for sr in self_role] + for rg in index_role_group + ) + if set_index_role: + # If the index has both role groups and roles. + role_perm = any( + str(role) in [str(r) for r in roles_list] and str(role) in [str(sr) for sr in self_role] + for role in self_role + ) + is_can = group_perm and role_perm + else: + # If the index has role groups but no roles. + is_can = group_perm + else: + # No role groups are set for the index, but the user has role groups. is_can = False - break - elif roles and "-99" not in roles: - is_can = False + elif set_index_role: + # Roles are set for the index, but no role groups are configured. + for role in self_role: + if str(role) in roles_list: + is_can = True + break + else: + # No roles or role groups are set for the index, so proceed to internal group check. + is_can = check_groups(params["groups"], params["access_group_ids"]) + + # Guest users can view when '-99' is set and no role groups are configured. + elif roles and "-99" in roles and not set_index_group: + is_can = True + # Administrators always have access. + else: + is_can = True return is_can @@ -335,10 +387,8 @@ def check_groups(user_group, groups): group = [x for x in user_group if str(x) in (groups or [])] if group: is_can = True - return is_can - def filter_index_list_by_role(index_list): """Filter index list by role.""" def _check(index_data, roles, groups): @@ -348,8 +398,8 @@ def _check(index_data, roles, groups): if roles[0]: can_view = True else: - if check_roles(roles, index_data.browsing_role) \ - or check_groups(groups, index_data.browsing_group): + params = set_params(groups, index_data.browsing_group) + if check_roles(roles, index_data.browsing_role, params): if index_data.public_state \ and (index_data.public_date is None or not is_future(index_data.public_date)): @@ -364,6 +414,36 @@ def _check(index_data, roles, groups): result_list.append(i) return result_list +def set_params(groups, index_group): + """ + Generate a parameter dictionary for index permission checks. + + This function collects all role group IDs from the Role table whose name contains 'group', + and combines them with the specified user groups and index groups to create a parameter + dictionary for permission checking functions. + + Args: + groups (list): List of group IDs the user belongs to. + index_group (list): List of group IDs set for the index. + + Returns: + dict: Dictionary containing: + - "role_groups": list of all role group IDs in the system + - "groups": list of group IDs the user belongs to + - "access_group_ids": list of group IDs set for the index + """ + from invenio_accounts.models import Role + + role_groups = [] + for lst in Role.query.all(): + if "group" in lst.name.lower(): + role_groups.append(lst.id) + params = { + "role_groups": role_groups, + "groups": groups, + "access_group_ids": index_group + } + return params def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): """Reduce index by.""" @@ -392,9 +472,8 @@ def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): # browsing role and group check if browsing_role: - if check_roles(roles, brw_role) \ - or check_groups(groups, brw_group): - + params = set_params(groups, brw_group) + if check_roles(roles, brw_role, params): if public_state and \ (public_date is None or not is_future(public_date)): @@ -408,8 +487,8 @@ def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): tree.pop(i) # contribute role and group check else: - if check_roles(roles, contribute_role) or \ - check_groups(groups, contribute_group): + params = set_params(groups, contribute_group) + if check_roles(roles, contribute_role, params): lst['disabled'] = False plst = plst or [] @@ -424,7 +503,6 @@ def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): reduce_index_by_role( children, roles, groups, False, plst) i += 1 - else: children.clear() tree.pop(i) @@ -793,7 +871,7 @@ def _check_index_permission(index_data) -> bool: """ from weko_records_ui.utils import is_future in_admin_view_scope = False - role_names = [role.name for role in current_user.roles] + role_names = [role.name for role in current_user.roles] if roles[0]: # In case admin role. in_admin_view_scope = True @@ -802,9 +880,12 @@ def _check_index_permission(index_data) -> bool: in_admin_view_scope = _check_community_admin_permission(index_data) is_content_public = False + params = set_params(groups, index_data.browsing_group) if index_data.public_state: - check_user_role = check_roles(roles, index_data.browsing_role) or \ - check_groups(groups, index_data.browsing_group) + check_user_role = check_roles(roles, index_data.browsing_role, params) + can_view = True + elif index_data.public_state: + check_user_role = check_roles(roles, index_data.browsing_role, params) check_public_date = \ not is_future(index_data.public_date) \ if index_data.public_date else True diff --git a/modules/weko-workflow/weko_workflow/admin.py b/modules/weko-workflow/weko_workflow/admin.py index 092c14ee02..bd143e3c16 100644 --- a/modules/weko-workflow/weko_workflow/admin.py +++ b/modules/weko-workflow/weko_workflow/admin.py @@ -70,7 +70,7 @@ def flow_detail(self, flow_id='0'): :return: """ users = User.query.filter_by(active=True).all() - roles = Role.query.all() + roles = Role.query.filter(~Role.name.like('%roles%')).all() if set(role.name for role in current_user.roles) & \ set(current_app.config['WEKO_PERMISSION_SUPER_ROLE_USER']): repositories = [{"id": "Root Index"}] + Community.query.all() @@ -357,7 +357,7 @@ def workflow_detail(self, workflow_id='0'): index_list = Index().get_all() location_list = Location.query.order_by(Location.id.asc()).all() hide = [] - role = Role.query.all() + role = Role.query.filter(~Role.name.like('%roles%')).all() display_label = self.get_language_workflows("display") hide_label = self.get_language_workflows("hide") display_hide = self.get_language_workflows("display_hide") From fe42f6031e610e57aa85cf85e7e1206ec81dd70f Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Fri, 17 Oct 2025 17:56:42 +0900 Subject: [PATCH 02/19] fix roles filter --- .../invenio_accounts/admin.py | 11 +++++++++- .../invenio_communities/admin.py | 5 ++++- modules/weko-workflow/weko_workflow/admin.py | 20 +++++++++++++++---- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/modules/invenio-accounts/invenio_accounts/admin.py b/modules/invenio-accounts/invenio_accounts/admin.py index 3bb6545068..f9fe5eddbd 100644 --- a/modules/invenio-accounts/invenio_accounts/admin.py +++ b/modules/invenio-accounts/invenio_accounts/admin.py @@ -86,11 +86,20 @@ class UserView(ModelView): def scaffold_form(self): form_class = super(UserView, self).scaffold_form() + try: + config = current_app.config.get('WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT', {}) + role_key = config.get('role_keyword', '') + prefix = config.get('prefix', '') + except RuntimeError: + role_key = '' + prefix = '' form_class.role = QuerySelectMultipleField( 'Roles', query_factory=lambda: Role.query.filter( ~Role.name.like('%_groups_%'), - ~Role.name.like('jc%roles%') + ~( + Role.name.like(f"%{role_key}%") & Role.name.like(f"%{prefix}%") + ) ).all(), get_label='name', widget=Select2Widget(multiple=True) diff --git a/modules/invenio-communities/invenio_communities/admin.py b/modules/invenio-communities/invenio_communities/admin.py index ee21ba1065..7f0f54f277 100644 --- a/modules/invenio-communities/invenio_communities/admin.py +++ b/modules/invenio-communities/invenio_communities/admin.py @@ -563,7 +563,10 @@ def validate_community_id(self, community_id): 'owner': { 'allow_blank': False, 'query_factory': lambda: db.session.query(Role).filter( - ~Role.name.like('jc%roles%') + ~( + Role.name.like(f"%{current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('role_keyword','')}%") & + Role.name.like(f"%{current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('prefix','')}%") + ) ).all(), }, 'group': { diff --git a/modules/weko-workflow/weko_workflow/admin.py b/modules/weko-workflow/weko_workflow/admin.py index bd143e3c16..1719551048 100644 --- a/modules/weko-workflow/weko_workflow/admin.py +++ b/modules/weko-workflow/weko_workflow/admin.py @@ -46,7 +46,7 @@ from .config import WEKO_WORKFLOW_SHOW_HARVESTING_ITEMS from .models import WorkflowRole from .utils import recursive_get_specified_properties, check_activity_settings - +from sqlalchemy import and_ class FlowSettingView(BaseView): @expose('/', methods=['GET']) @@ -70,7 +70,11 @@ def flow_detail(self, flow_id='0'): :return: """ users = User.query.filter_by(active=True).all() - roles = Role.query.filter(~Role.name.like('%roles%')).all() + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + roles = Role.query.filter( + ~and_(Role.name.like(f"%{role_key}%"), Role.name.like(f"%{prefix}%")) + ).all() if set(role.name for role in current_user.roles) & \ set(current_app.config['WEKO_PERMISSION_SUPER_ROLE_USER']): repositories = [{"id": "Root Index"}] + Community.query.all() @@ -318,7 +322,11 @@ def index(self): """ workflow = WorkFlow() workflows = workflow.get_workflow_list(user=current_user) - role = Role.query.all() + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + role = Role.query.filter( + ~and_(Role.name.like(f"%{role_key}%"), Role.name.like(f"%{prefix}%")) + ).all() for wf in workflows: index_tree = Index().get_index_by_id(wf.index_tree_id) wf.index_tree = index_tree @@ -357,7 +365,11 @@ def workflow_detail(self, workflow_id='0'): index_list = Index().get_all() location_list = Location.query.order_by(Location.id.asc()).all() hide = [] - role = Role.query.filter(~Role.name.like('%roles%')).all() + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + role = Role.query.filter( + ~and_(Role.name.like(f"%{role_key}%"), Role.name.like(f"%{prefix}%")) + ).all() display_label = self.get_language_workflows("display") hide_label = self.get_language_workflows("hide") display_hide = self.get_language_workflows("display_hide") From 047d95659b019c9a339f3cf8cb6de523be38a10f Mon Sep 17 00:00:00 2001 From: ayumi-nishida Date: Tue, 21 Oct 2025 14:36:07 +0900 Subject: [PATCH 03/19] =?UTF-8?q?=E8=A8=AD=E8=A8=88=E6=9B=B8=E3=81=AE?= =?UTF-8?q?=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E3=81=AB?= =?UTF-8?q?=E5=9F=BA=E3=81=A5=E3=81=8D=E3=82=B3=E3=83=BC=E3=83=89=E3=82=92?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../invenio_accounts/admin.py | 21 +------------------ .../invenio_communities/models.py | 3 +++ .../weko-index-tree/weko_index_tree/utils.py | 3 --- modules/weko-workflow/weko_workflow/admin.py | 2 +- 4 files changed, 5 insertions(+), 24 deletions(-) diff --git a/modules/invenio-accounts/invenio_accounts/admin.py b/modules/invenio-accounts/invenio_accounts/admin.py index 3bb6545068..36f5ea212a 100644 --- a/modules/invenio-accounts/invenio_accounts/admin.py +++ b/modules/invenio-accounts/invenio_accounts/admin.py @@ -88,10 +88,7 @@ def scaffold_form(self): form_class = super(UserView, self).scaffold_form() form_class.role = QuerySelectMultipleField( 'Roles', - query_factory=lambda: Role.query.filter( - ~Role.name.like('%_groups_%'), - ~Role.name.like('jc%roles%') - ).all(), + query_factory=lambda: Role.query.filter(~Role.name.like('%_groups_%')).all(), get_label='name', widget=Select2Widget(multiple=True) ) @@ -334,22 +331,6 @@ def after_model_change(self, form, model, is_created): current_app.logger.error(str(ex)) db.session.rollback() - def get_query(self): - """ - Get the query for filtering roles. - """ - return super().get_query().filter(Role.name.in_( - [role.name for role in Role.query.all() if role.info_display] - )) - - def get_count_query(self): - """ - Get the count query for filtering roles. - """ - return super().get_count_query().filter(Role.name.in_( - [role.name for role in Role.query.all() if role.info_display] - )) - class SessionActivityView(ModelView): """Admin view for user sessions.""" diff --git a/modules/invenio-communities/invenio_communities/models.py b/modules/invenio-communities/invenio_communities/models.py index 84b532f9ee..86e42c8553 100644 --- a/modules/invenio-communities/invenio_communities/models.py +++ b/modules/invenio-communities/invenio_communities/models.py @@ -598,7 +598,10 @@ def owner_display(self): if self.owner and hasattr(self.owner, 'name'): roles_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] role_mapping = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_mapping"] + sysadm_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["sysadm_group"] owner_name = self.owner.name + if owner_name == sysadm_key: + return "System Administrator" if owner_name and roles_key in owner_name: suffix = owner_name.split(roles_key + '_')[-1] if suffix in role_mapping.keys(): diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index 6d5e12099c..6da1c2adec 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -358,9 +358,6 @@ def check_roles(user_role, roles, params): else: # If the index has role groups but no roles. is_can = group_perm - else: - # No role groups are set for the index, but the user has role groups. - is_can = False elif set_index_role: # Roles are set for the index, but no role groups are configured. for role in self_role: diff --git a/modules/weko-workflow/weko_workflow/admin.py b/modules/weko-workflow/weko_workflow/admin.py index bd143e3c16..2e363b3a7f 100644 --- a/modules/weko-workflow/weko_workflow/admin.py +++ b/modules/weko-workflow/weko_workflow/admin.py @@ -318,7 +318,7 @@ def index(self): """ workflow = WorkFlow() workflows = workflow.get_workflow_list(user=current_user) - role = Role.query.all() + role = Role.query.filter(~Role.name.like('%roles%')).all() for wf in workflows: index_tree = Index().get_index_by_id(wf.index_tree_id) wf.index_tree = index_tree From 6bbb8ce2c6455b0bca3794d7634afc384813fde2 Mon Sep 17 00:00:00 2001 From: ayumi-nishida Date: Tue, 21 Oct 2025 15:22:57 +0900 Subject: [PATCH 04/19] =?UTF-8?q?=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC?= =?UTF-8?q?=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../weko-index-tree/weko_index_tree/utils.py | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index 6da1c2adec..48870f4f87 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -342,22 +342,15 @@ def check_roles(user_role, roles, params): self_role = user_role[1] or ['-98'] if set_index_group: index_role_group = [int(r) for r in roles_list if int(r) in params["role_groups"]] - if index_role_group: - # Groups(role) are set for the index. - group_perm = any( - str(rg) in [str(r) for r in roles_list] and str(rg) in [str(sr) for sr in self_role] - for rg in index_role_group - ) - if set_index_role: - # If the index has both role groups and roles. - role_perm = any( - str(role) in [str(r) for r in roles_list] and str(role) in [str(sr) for sr in self_role] - for role in self_role - ) - is_can = group_perm and role_perm - else: - # If the index has role groups but no roles. - is_can = group_perm + # Groups(role) are set for the index. + group_perm = any(str(rg) in [str(sr) for sr in self_role] for rg in index_role_group) + if set_index_role: + # If the index has both role groups and roles. + role_perm = any(str(role) in [str(r) for r in roles_list] for role in self_role) + is_can = group_perm and role_perm + else: + # If the index has role groups but no roles. + is_can = group_perm elif set_index_role: # Roles are set for the index, but no role groups are configured. for role in self_role: @@ -879,9 +872,6 @@ def _check_index_permission(index_data) -> bool: is_content_public = False params = set_params(groups, index_data.browsing_group) if index_data.public_state: - check_user_role = check_roles(roles, index_data.browsing_role, params) - can_view = True - elif index_data.public_state: check_user_role = check_roles(roles, index_data.browsing_role, params) check_public_date = \ not is_future(index_data.public_date) \ From e926df0960b38f587538e4887e6184f6dca4cab7 Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Sun, 19 Oct 2025 16:22:39 +0900 Subject: [PATCH 05/19] Unit Test for Hiding mAP Roles --- modules/invenio-accounts/tests/test_admin.py | 76 ----- .../invenio-communities/tests/test_admin.py | 38 ++- .../invenio-communities/tests/test_models.py | 49 +++ modules/weko-workflow/tests/conftest.py | 289 +++++++++--------- modules/weko-workflow/tests/test_admin.py | 114 ++++++- 5 files changed, 337 insertions(+), 229 deletions(-) diff --git a/modules/invenio-accounts/tests/test_admin.py b/modules/invenio-accounts/tests/test_admin.py index fca7042f27..db834cc7da 100644 --- a/modules/invenio-accounts/tests/test_admin.py +++ b/modules/invenio-accounts/tests/test_admin.py @@ -283,79 +283,3 @@ def test_userview_edit_form(app, users): view = UserView(User, db.session) form = view.edit_form() assert form.data['active'] is False - -# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_scaffold_form -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp -def test_scaffold_form(app): - """Test scaffold_form method of UserView.""" - with app.app_context(): - db.session.add(Role(id=1, name='Contributor', description=None)) - db.session.add(Role(id=2, name='jc_xxx_roles_contributor', description=None)) - db.session.add(Role(id=3, name='jc_xxx_groups_yyy', description=None)) - db.session.commit() - - view = UserView(User, db.session) - form_class = view.scaffold_form() - - roles = form_class.role.kwargs['query_factory']() - groups = form_class.group.kwargs['query_factory']() - - role_names = [r.name for r in roles] - group_names = [g.name for g in groups] - assert 'Contributor' in role_names - assert 'jc_xxx_groups_yyy' in group_names - -# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_roleview_get_query -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp -def test_roleview_get_query(app): - """Test RoleView.get_query method.""" - with app.app_context(): - app.config.update(dict( - WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT= - { - "role_keyword": "roles", - "role_mapping": { - "repoadm": "Repository Administrator", - "comadm": "Community Administrator", - "contributor": "Contributor", - } - } - )) - role1 = Role(id=1, name="Contributor", description=None) - role2 = Role(id=2, name="jc_xxx_roles_contributor", description=None) - role3 = Role(id=3, name="jc_xxx_groups_yyy", description=None) - db.session.add_all([role1, role2, role3]) - db.session.commit() - - view = RoleView(Role, db.session) - # get_queryの実行 - query = view.get_query() - result_names = [r.name for r in query.all()] - # info_displayがNoneでないロールのみ取得できること - assert "Contributor" in result_names - assert "jc_xxx_groups_yyy" in result_names - assert "jc_xxx_roles_contributor" not in result_names - -# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_roleview_get_count_query -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp -def test_roleview_get_count_query(app): - """Test RoleView.get_count_query method.""" - with app.app_context(): - app.config.update(dict( - WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT= - { - "role_keyword": "roles", - "role_mapping": { - "repoadm": "Repository Administrator", - "comadm": "Community Administrator", - "contributor": "Contributor", - } - } - )) - role1 = Role(id=1, name="Contributor", description=None) - role2 = Role(id=2, name="jc_xxx_roles_contributor", description=None) - role3 = Role(id=3, name="jc_xxx_groups_yyy", description=None) - db.session.add_all([role1, role2, role3]) - db.session.commit() - - view = RoleView(Role, db.session) - query = view.get_count_query() - count = query.scalar() - assert count == 2 diff --git a/modules/invenio-communities/tests/test_admin.py b/modules/invenio-communities/tests/test_admin.py index 47bc7f3e1b..cb5ee318f4 100644 --- a/modules/invenio-communities/tests/test_admin.py +++ b/modules/invenio-communities/tests/test_admin.py @@ -79,6 +79,36 @@ def setup_view_community(app,db,users): # class CommunityModelView(ModelView): # .tox/c1/bin/pytest --cov=invenio_communities tests/test_admin.py::TestInclusionRequestModelView -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-communities/.tox/c1/tmp class TestCommunityModelView(): + # .tox/c1/bin/pytest --cov=invenio_communities tests/test_admin.py::TestCommunityModelView::test_owner_query_factory_exclude_roles -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-communities/.tox/c1/tmp + def test_owner_query_factory_exclude_roles(self, app, db): + app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'] = { + 'role_keyword': 'roles', + 'prefix': 'jc' + } + from invenio_accounts.models import Role + + role_key = app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('role_keyword', '') + prefix = app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('prefix', '') + + role_both = Role(name=f"{prefix}abc{role_key}") # Contains both → should be excluded + role_only_key = Role(name=f"abc{role_key}") # Contains only one → should be included + role_only_prefix = Role(name=f"{prefix}abc") # Contains only one → should be included + role_none = Role(name="abc") # Contains neither → should be included + db.session.add_all([role_both, role_only_key, role_only_prefix, role_none]) + db.session.commit() + + from invenio_communities.models import Community + view = CommunityModelView(Community, db.session) + # List of role names obtained by query_factory + owner_names = [r.name for r in view.form_args['owner']['query_factory']()] + + # Exclude roles that contain both + assert role_both.name not in owner_names + # Include roles that contain only one or neither + assert role_only_key.name in owner_names + assert role_only_prefix.name in owner_names + assert role_none.name in owner_names + def test_index_view_acl_guest(self,app,setup_view_community,client): url = url_for('community.index_view') res = client.get(url) @@ -736,7 +766,7 @@ def test_on_model_delete(self,setup_view_community,users,mocker,db): model.cnri = None model.thumbnail_path = None CommunityModelView.on_model_delete(self, model) - + # .tox/c1/bin/pytest --cov=invenio_communities tests/test_admin.py::TestCommunityModelView::test_on_model_change_sets_id_user -vv -s -- def test_on_model_change_sets_id_user(self, setup_view_community, mocker): _, _, _, _, view = setup_view_community @@ -771,7 +801,7 @@ def test_get_json_schema(self,setup_view_community,users,mocker,db): with patch("invenio_communities.admin.json.load", return_value={}): res = client.get(url) assert res.status_code == 200 - + # .tox/c1/bin/pytest --cov=invenio_communities tests/test_admin.py::TestCommunityModelView::test_get_schema_form -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-communities/.tox/c1/tmp def test_get_schema_form(self,setup_view_community,users,mocker,db): app, _, _, user, _ = setup_view_community @@ -783,10 +813,10 @@ def test_get_schema_form(self,setup_view_community,users,mocker,db): with patch("invenio_communities.admin.db.session.execute", side_effect=BaseException()): res = client.get(url) assert res.status_code == 500 - + @pytest.mark.parametrize("input_id,expected_error", [ ("1abc", "The first character cannot"), - ("-123", "Cannot set negative number"), + ("-123", "Cannot set negative number"), ("abc def", "Don't use space or special"), ]) # .tox/c1/bin/pytest --cov=invenio_communities tests/test_admin.py::TestCommunityModelView::test_validate_input_id_error -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-communities/.tox/c1/tmp diff --git a/modules/invenio-communities/tests/test_models.py b/modules/invenio-communities/tests/test_models.py index 21ccd97ed9..384e9fb1aa 100644 --- a/modules/invenio-communities/tests/test_models.py +++ b/modules/invenio-communities/tests/test_models.py @@ -359,3 +359,52 @@ def test_oaiset_url(self, app, communities): # db.DateTime, nullable=False, default=datetime.utcnow) # @classmethod # def get_featured_or_none(cls, start_date=None): + +from invenio_accounts.models import Role +from flask import Flask +from invenio_communities.models import Community + +@pytest.fixture +def app(): + app = Flask(__name__) + app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'] = { + 'role_keyword': 'roles', + 'role_mapping': { + "repoadm":"Repository Administrator", + "comadm":"Community Administrator", + }, + "sysadm_group": "jc_roles_sysadm" + } + return app + +# .tox/c1/bin/pytest --cov=invenio_communities tests/test_models.py::test_owner_display -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-communities/.tox/c1/tmp +def test_owner_display(app): + with app.app_context(): + # If owner_name contains role_keyword and is mapped by role_mapping + owner1 = Role(name="roles_repoadm") + comm1 = Community() + comm1.owner = owner1 + assert comm1.owner_display == "Repository Administrator" + + # If owner_name matches sysadm_group + owner2 = Role(name="jc_roles_sysadm") + comm2 = Community() + comm2.owner = owner2 + assert comm2.owner_display == "System Administrator" + + # If owner_name contains role_keyword but is not found in role_mapping + owner3 = Role(name="roles_unknown") + comm3 = Community() + comm3.owner = owner3 + assert comm3.owner_display == "roles_unknown" + + # If owner_name does not contain role_keyword + owner4 = Role(name="admin") + comm4 = Community() + comm4.owner = owner4 + assert comm4.owner_display == "admin" + + # If owner is None + comm_none = Community() + comm_none.owner = None + assert comm_none.owner_display is None or comm_none.owner_display == '' diff --git a/modules/weko-workflow/tests/conftest.py b/modules/weko-workflow/tests/conftest.py index d3939be970..bf03a4af59 100644 --- a/modules/weko-workflow/tests/conftest.py +++ b/modules/weko-workflow/tests/conftest.py @@ -233,6 +233,10 @@ def base_app(instance_path, search_class, cache_config): CACHE_REDIS_DB='0', CACHE_REDIS_HOST="redis", REDIS_PORT='6379', + WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT={ + 'role_keyword': 'roles', + 'prefix': 'jc' + }, ACCOUNTS_SESSION_REDIS_DB_NO = 1, WEKO_RECORDS_UI_LICENSE_DICT=[ { @@ -568,52 +572,53 @@ def base_app(instance_path, search_class, cache_config): ) app_.testing = True - Babel(app_) - InvenioI18N(app_) - Menu(app_) - # InvenioTheme(app_) - OAuth2Provider(app_) - InvenioAccess(app_) - InvenioAccounts(app_) - InvenioFilesREST(app_) - InvenioCache(app_) - InvenioDB(app_) - InvenioDeposit(app_) - InvenioStats(app_) - InvenioAssets(app_) - InvenioAdmin(app_) - InvenioPIDRelations(app_) - InvenioJSONSchemas(app_) - InvenioPIDStore(app_) - InvenioRecords(app_) - InvenioRecordsUI(app_) - InvenioREST(app_) - InvenioOAuth2Server(app_) - InvenioOAuth2ServerREST(app_) - WekoRecordsUI(app_) - search = InvenioSearch(app_, client=MockEs()) - search.register_mappings(search_class.Meta.index, 'mock_module.mappings') - # InvenioCommunities(app_) - # WekoAdmin(app_) - WekoSearchUI(app_) - WekoWorkflow(app_) - WekoUserProfiles(app_) - WekoDeposit(app_) - WekoItemsUI(app_) - WekoAdmin(app_) - InvenioOAuth2Server(app_) - WekoLoggingUserActivity(app_) - WekoNotifications(app_) - # WekoRecordsUI(app_) - # app_.register_blueprint(invenio_theme_blueprint) - app_.register_blueprint(invenio_communities_blueprint) - # app_.register_blueprint(invenio_admin_blueprint) - # app_.register_blueprint(invenio_accounts_blueprint) - # app_.register_blueprint(weko_theme_blueprint) - # app_.register_blueprint(weko_admin_blueprint) - app_.register_blueprint(weko_workflow_blueprint) - WekoWorkflowREST(app_) - app_.register_blueprint(oauth2server_settings_blueprint) + with app_.app_context(): + Babel(app_) + InvenioI18N(app_) + Menu(app_) + # InvenioTheme(app_) + OAuth2Provider(app_) + InvenioAccess(app_) + InvenioAccounts(app_) + InvenioFilesREST(app_) + InvenioCache(app_) + InvenioDB(app_) + InvenioDeposit(app_) + InvenioStats(app_) + InvenioAssets(app_) + InvenioAdmin(app_) + InvenioPIDRelations(app_) + InvenioJSONSchemas(app_) + InvenioPIDStore(app_) + InvenioRecords(app_) + InvenioRecordsUI(app_) + InvenioREST(app_) + InvenioOAuth2Server(app_) + InvenioOAuth2ServerREST(app_) + WekoRecordsUI(app_) + search = InvenioSearch(app_, client=MockEs()) + search.register_mappings(search_class.Meta.index, 'mock_module.mappings') + # InvenioCommunities(app_) + # WekoAdmin(app_) + WekoSearchUI(app_) + WekoWorkflow(app_) + WekoUserProfiles(app_) + WekoDeposit(app_) + WekoItemsUI(app_) + WekoAdmin(app_) + InvenioOAuth2Server(app_) + WekoLoggingUserActivity(app_) + WekoNotifications(app_) + # WekoRecordsUI(app_) + # app_.register_blueprint(invenio_theme_blueprint) + app_.register_blueprint(invenio_communities_blueprint) + # app_.register_blueprint(invenio_admin_blueprint) + # app_.register_blueprint(invenio_accounts_blueprint) + # app_.register_blueprint(weko_theme_blueprint) + # app_.register_blueprint(weko_admin_blueprint) + app_.register_blueprint(weko_workflow_blueprint) + WekoWorkflowREST(app_) + app_.register_blueprint(oauth2server_settings_blueprint) return app_ @@ -696,7 +701,7 @@ def users(app, db): user = User.query.filter_by(email='user@test.org').one_or_none() if not user: user = create_test_user(email='user@test.org') - + contributor = User.query.filter_by(email='user@test.org').one_or_none() if not contributor: contributor = create_test_user(email='contributor@test.org') @@ -708,7 +713,7 @@ def users(app, db): repoadmin = User.query.filter_by(email='repoadmin@test.org').one_or_none() if not repoadmin: repoadmin = create_test_user(email='repoadmin@test.org') - + sysadmin = User.query.filter_by(email='sysadmin@test.org').one_or_none() if not sysadmin: sysadmin = create_test_user(email='sysadmin@test.org') @@ -748,15 +753,15 @@ def users(app, db): general_role = Role.query.filter_by(name='General').one_or_none() if not general_role: general_role = ds.create_role(name='General') - + originalrole = Role.query.filter_by(name='Original Role').one_or_none() if not originalrole: originalrole = ds.create_role(name='Original Role') - + studentrole = Role.query.filter_by(name='Student').one_or_none() if not studentrole: studentrole = ds.create_role(name='Student') - + ds.add_role_to_user(sysadmin, sysadmin_role) ds.add_role_to_user(repoadmin, repoadmin_role) ds.add_role_to_user(contributor, contributor_role) @@ -832,7 +837,7 @@ def users(app, db): index = Index(id=1,parent=0,position=0,index_name="com_index",display_no=5,public_state=True) db.session.add(index) db.session.commit() - + comm = Community.query.filter_by(id="comm01").one_or_none() if not comm: @@ -878,7 +883,7 @@ def users_1(app, db): sysadmin = create_test_user(email='sysadmin@test.org') else: sysadmin = User.query.filter_by(email='sysadmin@test.org').first() - + role_count = Role.query.filter_by(name='System Administrator').count() if role_count != 1: sysadmin_role = ds.create_role(name='System Administrator') @@ -907,7 +912,7 @@ def users_1(app, db): index = Index(id=1,parent=0,position=0,index_name="com_index",display_no=5,public_state=True) db.session.add(index) db.session.commit() - + comm = Community.query.filter_by(id="comm01").one_or_none() if not comm: comm = Community.create(community_id="comm01", role_id=sysadmin_role.id, @@ -1084,7 +1089,7 @@ def item_type_usage_report(db): item_type_schema = dict() with open("tests/data/item_type/itemtype_schema_31003.json", "r") as f: item_type_schema = json.load(f) - + item_type_form = dict() with open("tests/data/item_type/itemtype_form_31003.json", "r") as f: item_type_form = json.load(f) @@ -1369,7 +1374,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): flow_name='Delete Approval Flow', flow_user=1, flow_type='2') - + with db.session.begin_nested(): db.session.add(flow_define) db.session.add(del_flow_define) @@ -1388,7 +1393,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2018/07/28 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + flow_action2 = FlowAction.query.filter_by(flow_id=flow_define.flow_id, action_id=3, action_order=2).one_or_none() if not flow_action2: flow_action2 = FlowAction(status='N', @@ -1400,7 +1405,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2018/07/28 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + flow_action3 = FlowAction.query.filter_by(flow_id=flow_define.flow_id, action_id=5).one_or_none() if not flow_action3: flow_action3 = FlowAction(status='N', @@ -1412,7 +1417,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2018/07/28 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + flow_action4 = FlowAction.query.filter_by(flow_id=flow_define.flow_id, action_id=3, action_order=1).one_or_none() if not flow_action4: flow_action4 = FlowAction(status='N', @@ -1424,7 +1429,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2018/07/28 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + del_flow_action1 = FlowAction.query.filter_by(flow_id=del_flow_define.flow_id, action_id=1).one_or_none() if not del_flow_action1: del_flow_action1 = FlowAction(status='N', @@ -1448,7 +1453,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2025/05/01 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + app_flow_action1 = FlowAction.query.filter_by(flow_id=app_flow_define.flow_id, action_id=1).one_or_none() if not app_flow_action1: app_flow_action1 = FlowAction(status='N', @@ -1460,7 +1465,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2025/05/01 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + app_flow_action2 = FlowAction.query.filter_by(flow_id=app_flow_define.flow_id, action_id=4).one_or_none() if not app_flow_action2: app_flow_action2 = FlowAction(status='N', @@ -1472,7 +1477,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_status='A', action_date=datetime.strptime('2025/05/01 0:00:00','%Y/%m/%d %H:%M:%S'), send_mail_setting={}) - + app_flow_action3 = FlowAction.query.filter_by(flow_id=app_flow_define.flow_id, action_id=2).one_or_none() if not app_flow_action3: app_flow_action3 = FlowAction(status='N', @@ -1502,31 +1507,31 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_role_1 = FlowActionRole(flow_action_id=flow_action1.id, action_role=1, action_user=1) - - action_role_2_1 = FlowActionRole.query.filter_by(flow_action_id=flow_action2.id, action_role=1, action_user=2).one_or_none() + + action_role_2_1 = FlowActionRole.query.filter_by(flow_action_id=flow_action2.id, action_role=1, action_user=2).one_or_none() if not action_role_2_1: action_role_2_1 = FlowActionRole(flow_action_id=flow_action2.id, action_role=1, action_user=2) - - action_role_2_2 = FlowActionRole.query.filter_by(flow_action_id=flow_action2.id, action_role=2, action_user=1).one_or_none() + + action_role_2_2 = FlowActionRole.query.filter_by(flow_action_id=flow_action2.id, action_role=2, action_user=1).one_or_none() if not action_role_2_2: action_role_2_2 = FlowActionRole(flow_action_id=flow_action2.id, action_role=2, action_user=1) - + action_role_2_3 = FlowActionRole.query.filter_by(flow_action_id=flow_action2.id, action_role=2, action_user=2).one_or_none() if not action_role_2_3: action_role_2_3 = FlowActionRole(flow_action_id=flow_action2.id, action_role=2, action_user=2) - + action_role_2_4 = FlowActionRole.query.filter_by(flow_action_id=flow_action2.id, action_role=2, action_user=3).one_or_none() if not action_role_2_4: action_role_2_4 = FlowActionRole(flow_action_id=flow_action2.id, action_role=2, action_user=3) - + action_role_3 = FlowActionRole.query.filter_by(flow_action_id=flow_action3.id, action_role=1, action_user=3).one_or_none() if not action_role_3: action_role_3 = FlowActionRole(flow_action_id=flow_action3.id, @@ -1541,8 +1546,8 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_role_exclude=True, action_user_exclude=True ) - - action_role_4_2 = FlowActionRole.query.filter_by(flow_action_id=flow_action4.id, action_role=1, action_user=2, action_role_exclude=True, action_user_exclude=True).one_or_none() + + action_role_4_2 = FlowActionRole.query.filter_by(flow_action_id=flow_action4.id, action_role=1, action_user=2, action_role_exclude=True, action_user_exclude=True).one_or_none() if not action_role_4_2: action_role_4_2 = FlowActionRole(flow_action_id=flow_action4.id, action_role=1, @@ -1550,8 +1555,8 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_role_exclude=True, action_user_exclude=True ) - - action_role_4_3 = FlowActionRole.query.filter_by(flow_action_id=flow_action4.id, action_role=1, action_user=3, action_role_exclude=False, action_user_exclude=False).one_or_none() + + action_role_4_3 = FlowActionRole.query.filter_by(flow_action_id=flow_action4.id, action_role=1, action_user=3, action_role_exclude=False, action_user_exclude=False).one_or_none() if not action_role_4_3: action_role_4_3 = FlowActionRole(flow_action_id=flow_action4.id, action_role=1, @@ -1559,9 +1564,9 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_role_exclude=False, action_user_exclude=False ) - + action_role_4_4 = FlowActionRole.query.filter_by(flow_action_id=flow_action4.id, action_role=2, action_user=1).one_or_none() - if not action_role_4_4: + if not action_role_4_4: action_role_4_4 = FlowActionRole(flow_action_id=flow_action4.id, action_role=2, action_user=1 @@ -1590,7 +1595,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): open_restricted=False, location_id=None, is_gakuninrdm=False) - + del_workflow = WorkFlow.query.filter_by(flows_name='test delete workflow').one_or_none() if not del_workflow: del_workflow = WorkFlow(flows_id=uuid.uuid4(), @@ -1603,7 +1608,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): open_restricted=False, location_id=None, is_gakuninrdm=False) - + app_del_workflow = WorkFlow.query.filter_by(flows_name='test delete approval workflow').one_or_none() if not app_del_workflow: app_del_workflow = WorkFlow(flows_id=uuid.uuid4(), @@ -1616,7 +1621,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): open_restricted=False, location_id=None, is_gakuninrdm=False) - + activity = Activity.query.filter_by(activity_id='1').one_or_none() if not activity: activity = Activity(activity_id='1',workflow_id=1, flow_id=flow_define.id, @@ -1628,7 +1633,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test', shared_user_ids='[]', extra_info={}, action_order=1, ) - + activity2 = Activity.query.filter_by(activity_id='A-00000001-10001').one_or_none() if not activity2: activity2 = Activity(activity_id='A-00000001-10001',workflow_id=1, flow_id=flow_define.id, @@ -1664,7 +1669,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test', shared_user_ids=[], extra_info={}, action_order=1, ) - + app_del_activity = Activity.query.filter_by(activity_id='A-00000001-10011').one_or_none() if not app_del_activity: app_del_activity = Activity(activity_id='A-00000001-10011',workflow_id=3, flow_id=app_flow_define.id, @@ -1676,7 +1681,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item1 = Activity.query.filter_by(activity_id='2').one_or_none() if not activity_item1: activity_item1 = Activity(activity_id='2',item_id=db_records[2][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1688,7 +1693,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item1', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item2 = Activity.query.filter_by(activity_id='3').one_or_none() activity_item2 = Activity(activity_id='3', workflow_id=1, flow_id=flow_define.id, action_id=3, activity_login_user=users[3]["id"], @@ -1710,7 +1715,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item3', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item4 = Activity.query.filter_by(activity_id='5').one_or_none() if not activity_item4: activity_item4 = Activity(activity_id='5', workflow_id=1, flow_id=flow_define.id, @@ -1722,7 +1727,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item4', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item5 = Activity.query.filter_by(activity_id='6').one_or_none() if not activity_item5: activity_item5 = Activity(activity_id='6', workflow_id=1, flow_id=flow_define.id, @@ -1734,7 +1739,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item5', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item6 = Activity.query.filter_by(activity_id='7').one_or_none() if not activity_item6: activity_item6 = Activity(activity_id='7', workflow_id=1, flow_id=flow_define.id, @@ -1746,7 +1751,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item5', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item7 = Activity.query.filter_by(activity_id='8').one_or_none() if not activity_item7: activity_item7 = Activity(activity_id='8', item_id=db_records[0][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1758,7 +1763,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item8', shared_user_ids=[], extra_info={}, action_order=1, ) - + activity_item8 = Activity.query.filter_by(activity_id='9').one_or_none() if not activity_item8: activity_item8 = Activity(activity_id='9', item_id=db_records[1][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1770,7 +1775,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item8', shared_user_ids='[]', extra_info={}, action_order=1, ) - + activity_item9 = Activity.query.filter_by(activity_id='10').one_or_none() if not activity_item9: activity_item9 = Activity(activity_id='10', item_id=db_records[1][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1782,7 +1787,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='test item9', shared_user_ids=[6], extra_info={}, action_order=1, ) - + activity_item10 = Activity.query.filter_by(activity_id='11').one_or_none() if not activity_item10: activity_item10 = Activity(activity_id='11', item_id=db_records[1][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1794,7 +1799,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): title='制限公開', shared_user_ids=[2,4], extra_info={}, action_order=1, ) - + activity_guest = Activity.query.filter_by(activity_id='guest').one_or_none() if not activity_guest: activity_guest = Activity(activity_id='guest', item_id=db_records[1][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1807,7 +1812,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_order=1, extra_info={"guest_mail":"guest@test.org","record_id": 1,"related_title":"related_guest_activity","usage_record_id":str(db_records[1][2].id),"usage_activity_id":str(uuid.uuid4())} ) - + activity_landing_url = Activity.query.filter_by(activity_id='A-00000001-10003').one_or_none() if not activity_landing_url: activity_landing_url = Activity(activity_id='A-00000001-10003',workflow_id=1, flow_id=flow_define.id, @@ -1818,7 +1823,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): activity_confirm_term_of_use=True, title='test', shared_user_ids=[], extra_info={"record_id": 1}, action_order=6) - + activity_terms_of_use = Activity.query.filter_by(activity_id='A-00000001-10004').one_or_none() if not activity_terms_of_use: activity_terms_of_use = Activity(activity_id='A-00000001-10004',workflow_id=1, flow_id=flow_define.id, @@ -1829,13 +1834,13 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): activity_confirm_term_of_use=True, title='test', shared_user_ids=[], extra_info={"record_id": 1, "file_name":"aaa.txt"}, action_order=6) - + activity_no_contents = Activity.query.filter_by(activity_id='A-00000001-10005').one_or_none() if not activity_no_contents: activity_no_contents = Activity(activity_id='A-00000001-10005',workflow_id=1, flow_id=flow_define.id, action_id=1, activity_login_user=1,title='test', shared_user_ids=[], extra_info={"record_id": 1, "file_name":"recid/1.0"}, action_order=6) - + activity_guest_2 = Activity.query.filter_by(activity_id='guest_2').one_or_none() if not activity_guest_2: activity_guest_2 = Activity(activity_id='guest_2', item_id=db_records[1][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1861,7 +1866,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_order=1, extra_info={"guest_mail":"guest@test.org","record_id": 3,"related_title":"related_guest_activity","usage_record_id":str(db_records[1][2].id),"usage_activity_id":str(uuid.uuid4())} ) - + activity_guest_4 = Activity.query.filter_by(activity_id='guest_4').one_or_none() if not activity_guest_4: activity_guest_4 = Activity(activity_id='guest_4', item_id=db_records[1][2].id,workflow_id=1, flow_id=flow_define.id, @@ -1909,19 +1914,19 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): activity_action1_item1 = ActivityAction(activity_id=activity_item1.activity_id, action_id=1,action_status="M", action_handler=1, action_order=1) - + activity_action2_item1 = ActivityAction.query.filter_by(activity_id=activity_item1.activity_id, action_id=3).one_or_none() if not activity_action2_item1: activity_action2_item1 = ActivityAction(activity_id=activity_item1.activity_id, action_id=3,action_status="M", action_handler=1, action_order=2) - + activity_action3_item1 = ActivityAction.query.filter_by(activity_id=activity_item1.activity_id, action_id=5).one_or_none() if not activity_action3_item1: activity_action3_item1 = ActivityAction(activity_id=activity_item1.activity_id, action_id=5,action_status="M", action_handler=1, action_order=3) - + activity_action1_item2 = ActivityAction.query.filter_by(activity_id=activity_item2.activity_id, action_id=1).one_or_none() if not activity_action1_item2: activity_action1_item2 = ActivityAction(activity_id=activity_item2.activity_id, @@ -1932,13 +1937,13 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): activity_action2_item2 = ActivityAction(activity_id=activity_item2.activity_id, action_id=3,action_status="M", action_handler=1, action_order=2) - + activity_action3_item2 = ActivityAction.query.filter_by(activity_id=activity_item2.activity_id, action_id=5).one_or_none() if not activity_action3_item2: activity_action3_item2 = ActivityAction(activity_id=activity_item2.activity_id, action_id=5,action_status="M", action_handler=1, action_order=3) - + activity_item2_feedbackmail = ActionFeedbackMail.query.filter_by(activity_id=activity_item2.activity_id, action_id=3).one_or_none() if not activity_item2_feedbackmail: activity_item2_feedbackmail = ActionFeedbackMail(activity_id='3', @@ -1951,7 +1956,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_id=3, feedback_maillist=[{"email": "test@org", "author_id": ""}] ) - + activity_item4_feedbackmail = ActionFeedbackMail.query.filter_by(activity_id=activity_item4.activity_id, action_id=3).one_or_none() if not activity_item4_feedbackmail: activity_item4_feedbackmail = ActionFeedbackMail(activity_id='5', @@ -1965,11 +1970,11 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): action_id=3, feedback_maillist=[{"email": "test1@org", "author_id": "2"}] ) - + activity_item5_Authors = Authors.query.filter_by(id=1).one_or_none() if not activity_item5_Authors: activity_item5_Authors = Authors(id=1,json={'affiliationInfo': [{'affiliationNameInfo': [{'affiliationName': '', 'affiliationNameLang': 'ja', 'affiliationNameShowFlg': 'true'}], 'identifierInfo': [{'affiliationId': 'aaaa', 'affiliationIdType': '1', 'identifierShowFlg': 'true'}]}], 'authorIdInfo': [{'authorId': '1', 'authorIdShowFlg': 'true', 'idType': '1'}, {'authorId': '1', 'authorIdShowFlg': 'true', 'idType': '2'}], 'authorNameInfo': [{'familyName': '一', 'firstName': '二', 'fullName': '一\u3000二 ', 'language': 'ja-Kana', 'nameFormat': 'familyNmAndNm', 'nameShowFlg': 'true'}], 'emailInfo': [{'email': 'test@org'}], 'gather_flg': 0, 'id': {'_id': 'HZ9iXYMBnq6bEezA2CK3', '_index': 'tenant1-authors-author-v1.0.0', '_primary_term': 29, '_seq_no': 0, '_shards': {'failed': 0, 'successful': 1, 'total': 2}, '_type': 'author-v1.0.0', '_version': 1, 'result': 'created'}, 'is_deleted': 'false', 'pk_id': '1'}) - + activity_item6_feedbackmail = ActionFeedbackMail.query.filter_by(activity_id=activity_item6.activity_id, action_id=3).one_or_none() if not activity_item6_feedbackmail: activity_item6_feedbackmail = ActionFeedbackMail(activity_id='7', @@ -2008,7 +2013,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): activity_action03_1 = ActivityAction(activity_id=activity_03.activity_id, action_id=1,action_status="M",action_comment="", action_handler=1, action_order=1) - + activity_action03_2 = ActivityAction.query.filter_by(activity_id=activity_03.activity_id, action_id=3).one_or_none() if not activity_action03_2: activity_action03_2 = ActivityAction(activity_id=activity_03.activity_id, @@ -2030,7 +2035,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): with db.session.begin_nested(): db.session.add(history) db.session.commit() - + doi_identifier = Identifier.query.filter_by(id=1).one_or_none() if not doi_identifier: doi_identifier = Identifier(id=1, repository='Root Index',jalc_flag= True,jalc_crossref_flag= True,jalc_datacite_flag=True,ndl_jalc_flag=True, @@ -2038,7 +2043,7 @@ def db_register_full_action(app, db, db_records, users, action_data, item_type): created_userId='1',created_date=datetime.strptime('2022-09-28 04:33:42','%Y-%m-%d %H:%M:%S'), updated_userId='1',updated_date=datetime.strptime('2022-09-28 04:33:42','%Y-%m-%d %H:%M:%S') ) - + doi_identifier2 = Identifier.query.filter_by(id=2).one_or_none() if not doi_identifier2: doi_identifier2 = Identifier(id=2, repository='test',jalc_flag= True,jalc_crossref_flag= True,jalc_datacite_flag=True,ndl_jalc_flag=True, @@ -2158,7 +2163,7 @@ def db_register_1(app, db, db_records, users_1, action_data, item_type): db.session.commit() return {'flow_define':flow_define, 'item_type':item_type, - 'workflow':workflow, + 'workflow':workflow, "activities":[activity]} @@ -2250,7 +2255,7 @@ def workflow(app, db, item_type, action_data, users): flows_name='test workflow01', itemtype_id=1, index_tree_id=None, - flow_id=1, + flow_id=flow_define.flow_id, is_deleted=False, open_restricted=False, location_id=None, @@ -2259,7 +2264,7 @@ def workflow(app, db, item_type, action_data, users): flows_name='test workflow02', itemtype_id=1, index_tree_id=None, - flow_id=1, + flow_id=flow_define.flow_id, is_deleted=True, open_restricted=False, location_id=None, @@ -2329,10 +2334,10 @@ def no_begin_action(app, db): @pytest.fixture() def workflow_open_restricted(app, db, item_type, action_data, users): - flow_define1 = FlowDefine(id=2,flow_id=uuid.uuid4(), + flow_define1 = FlowDefine(flow_id=uuid.uuid4(), flow_name='terms_of_use_only', flow_user=1) - flow_define2 = FlowDefine(id=3,flow_id=uuid.uuid4(), + flow_define2 = FlowDefine(flow_id=uuid.uuid4(), flow_name='usage application', flow_user=1) with db.session.begin_nested(): @@ -2413,7 +2418,7 @@ def workflow_open_restricted(app, db, item_type, action_data, users): flows_name='terms_of_use_only', itemtype_id=1, index_tree_id=None, - flow_id=2, + flow_id=flow_define1.id, is_deleted=False, open_restricted=True, location_id=None, @@ -2422,7 +2427,7 @@ def workflow_open_restricted(app, db, item_type, action_data, users): flows_name='usage application', itemtype_id=1, index_tree_id=None, - flow_id=3, + flow_id=flow_define2.id, is_deleted=False, open_restricted=True, location_id=None, @@ -2431,7 +2436,7 @@ def workflow_open_restricted(app, db, item_type, action_data, users): flows_name='nomal workflow', itemtype_id=1, index_tree_id=None, - flow_id=3, + flow_id=flow_define2.id, is_deleted=False, open_restricted=False, location_id=None, @@ -3241,24 +3246,24 @@ def db_register_usage_application_workflows(app, db, action_data, item_type ): db.session.commit() workflows.update({ - "flow_define1" : flow_define1 - # ,"flow_define2" : flow_define2 - ,"flow_define3" : flow_define3 - ,"flow_define4" : flow_define4 - ,"flow_action1_1" : flow_action1_1 - ,"flow_action1_2" : flow_action1_2 - ,"flow_action1_3" : flow_action1_3 - # ,"flow_action2_1" : flow_action2_1 - # ,"flow_action2_2" : flow_action2_2 - ,"flow_action3_1" : flow_action3_1 - ,"flow_action3_2" : flow_action3_2 - ,"flow_action3_3" : flow_action3_3 - ,"flow_action3_4" : flow_action3_4 - ,"flow_action4_1" : flow_action4_1 - ,"flow_action4_2" : flow_action4_2 - ,"flow_action4_3" : flow_action4_3 - ,"flow_action4_4" : flow_action4_4 - ,"flow_action4_5" : flow_action4_5 + "flow_define1" : flow_define1 + # ,"flow_define2" : flow_define2 + ,"flow_define3" : flow_define3 + ,"flow_define4" : flow_define4 + ,"flow_action1_1" : flow_action1_1 + ,"flow_action1_2" : flow_action1_2 + ,"flow_action1_3" : flow_action1_3 + # ,"flow_action2_1" : flow_action2_1 + # ,"flow_action2_2" : flow_action2_2 + ,"flow_action3_1" : flow_action3_1 + ,"flow_action3_2" : flow_action3_2 + ,"flow_action3_3" : flow_action3_3 + ,"flow_action3_4" : flow_action3_4 + ,"flow_action4_1" : flow_action4_1 + ,"flow_action4_2" : flow_action4_2 + ,"flow_action4_3" : flow_action4_3 + ,"flow_action4_4" : flow_action4_4 + ,"flow_action4_5" : flow_action4_5 ,"workflow_workflow1" : workflow_workflow1 # ,"workflow_workflow2" : workflow_workflow2 ,"workflow_workflow3" : workflow_workflow3 @@ -3270,12 +3275,12 @@ def db_register_usage_application_workflows(app, db, action_data, item_type ): @pytest.fixture() def db_register_usage_application(app, db, db_records, users, action_data, item_type, db_register_usage_application_workflows ): workflows = db_register_usage_application_workflows - + # 利用登録(now -> item_registration, next ->end) activity1 = Activity(activity_id='A-00000001-20001' ,workflow_id=workflows["workflow_workflow1"].id , flow_id=workflows["flow_define1"].id, - action_id=3, + action_id=3, item_id=db_records[2][2].id, activity_login_user=1, action_status = 'M', @@ -4487,7 +4492,7 @@ def activity_with_roles_for_request_mail(app, workflow, db, item_type, users): flow_action_id = flow_actions[1].id, action_user = 2, action_user_exclude = False, - ) + ) ] with db.session.begin_nested(): db.session.add_all(flow_action_roles) @@ -4545,7 +4550,7 @@ def activity_with_roles_for_request_mail(app, workflow, db, item_type, users): action_order=6, item_id=item_metdata.model.id, action_status=ActionStatusPolicy.ACTION_BEGIN ) - + with db.session.begin_nested(): db.session.add(activity) db.session.commit() @@ -4561,7 +4566,7 @@ def activity_with_roles_for_request_mail(app, workflow, db, item_type, users): request_mail = ActivityRequestMail(activity_id = activity.activity_id, display_request_button = True, request_maillist = [{"email": "contributor@test.org", "author_id": "1"}]) - + with db.session.begin_nested(): db.session.add(request_mail) db.session.commit() @@ -4741,7 +4746,7 @@ def db_register_for_application_api(app, db, users, db_register_for_application_ flow_action2_4 = workflows["flow_action2_4"] workflow_workflow1 = workflows["workflow_workflow1"] workflow_workflow2 = workflows["workflow_workflow2"] - + # 1.利用申請(now -> item_registration) activity1 = Activity(activity_id='A-00000001-20001' ,workflow_id=workflow_workflow1.id @@ -4820,7 +4825,7 @@ def db_register_for_application_api(app, db, users, db_register_for_application_ ,expiration_date=10 ,is_usage_report=False ) - + # 3.利用申請(end) activity3 = Activity(activity_id='A-00000001-20003' ,workflow_id=workflow_workflow1.id @@ -4959,7 +4964,7 @@ def db_register_for_application_api(app, db, users, db_register_for_application_ ,expiration_date=10 ,is_usage_report=False ) - + # 6.利用申請 edit item (now -> item_registration) activity6 = Activity(activity_id='A-00000001-20006' ,workflow_id=workflow_workflow1.id @@ -5181,7 +5186,7 @@ def db_register_for_application_api(app, db, users, db_register_for_application_ }) return workflows - + @pytest.fixture() def application_api_request_body(app, item_type): diff --git a/modules/weko-workflow/tests/test_admin.py b/modules/weko-workflow/tests/test_admin.py index a6f4b216e6..eea66717d0 100644 --- a/modules/weko-workflow/tests/test_admin.py +++ b/modules/weko-workflow/tests/test_admin.py @@ -151,6 +151,39 @@ def test_flow_detail_update_delete(self,app,client,users,workflow ,workflow_open res = client.get(url) assert res.status_code == 404 + # .tox/c1/bin/pytest --cov=weko_workflow tests/test_admin.py::TestFlowSettingView::test_flow_detail_roles_filter -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp + def test_flow_detail_roles_filter(self, client, db, users): + from invenio_accounts.models import Role + from invenio_accounts.testutils import login_user_via_session as login + + client.application.config.update(dict( + WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT={ + "prefix":"jc", + "role_keyword": "roles", + "role_mapping": { + "repoadm": "Repository Administrator", + "comadm": "Community Administrator", + "contributor": "Contributor", + } + } + )) + user = users[1]['obj'] + db.session.add(user) + role1 = Role(name="Contributor_test", description=None) + role2 = Role(name="jc_xxx_roles_contributor", description=None) + role3 = Role(name="jc_xxx_groups_yyy", description=None) + db.session.add_all([role1, role2, role3]) + db.session.commit() + + login(client=client, email=users[1]['email']) + with patch("flask.templating._render", return_value=b"") as mock_render: + response = client.get('/admin/flowsetting/0') + args, kwargs = mock_render.call_args + context = args[1] + filtered_role_names = [role.name for role in context['roles']] + assert "jc_xxx_roles_contributor" not in filtered_role_names + assert "jc_xxx_groups_yyy" in filtered_role_names + assert "Contributor_test" in filtered_role_names # def get_specified_properties(): # .tox/c1/bin/pytest --cov=weko_workflow tests/test_admin.py::TestFlowSettingView::test_get_specified_properties -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp @@ -437,6 +470,39 @@ def test_index_acl(self,client,db_register2,users,users_index,status_code): res = client.get(url) assert res.status_code == status_code + # .tox/c1/bin/pytest --cov=weko_workflow tests/test_admin.py::TestWorkFlowSettingView::test_index_role_filtering -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp + def test_index_role_filtering(self, client, db, app, mocker, users): + from invenio_accounts.models import Role + role1 = Role(name="test_role", description=None) + role2 = Role(name="jc_xxx_roles_contributor", description=None) + role3 = Role(name="jc_xxx_groups_yyy", description=None) + db.session.add_all([role1, role2, role3]) + db.session.commit() + + app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"] = { + "role_keyword": "roles", + "prefix": "jc" + } + wf_mock = mocker.MagicMock() + wf_mock.id = 1 + wf_mock.index_tree_id = None + mocker.patch("weko_workflow.admin.WorkFlow.get_workflow_list", return_value=[wf_mock]) + mocker.patch("weko_workflow.admin.Index.get_index_by_id", return_value=None) + + from invenio_accounts.testutils import login_user_via_session + login_user_via_session(client, email=users[2]['email']) + + mock_render = mocker.patch("flask.templating._render", return_value=b"") + url = "/admin/workflowsetting/" + res = client.get(url) + assert res.status_code == 200 + args, kwargs = mock_render.call_args + context = args[1] + display_names = context['workflows'][0].display.replace(',
', ',').split(',') + assert "jc_xxx_roles_contributor" not in display_names + assert "test_role" in display_names + assert "jc_xxx_groups_yyy" in display_names + # def workflow_detail(self, workflow_id='0'): # .tox/c1/bin/pytest --cov=weko_workflow tests/test_admin.py::TestWorkFlowSettingView::test_workflow_detail_acl_guest -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp def test_workflow_detail_acl_guest(self,client,db_register2): @@ -618,7 +684,7 @@ def test_update_workflow_acl(self,client,db_register2,workflow,users,users_index url = '/admin/workflowsetting/{}'.format(0) with patch("flask.templating._render", return_value=""): res = client.put(url) - assert res.status_code == status_code + assert res.status_code == status_code res = client.post(url, data=json.dumps(data), headers=[('Content-Type', 'application/json')]) assert res.status_code == 200 @@ -633,7 +699,7 @@ def test_update_workflow(self,client,db,db_register2,users,workflow): wflow : WorkFlow = workflow["workflow"] url = url_for('workflowsetting.update_workflow',workflow_id=wflow.flows_id,_external=True) with patch("flask.templating._render", return_value=""): - res = client.post(url + res = client.post(url , headers=[('Content-Type', 'application/json') ,('Accept', 'application/json')] , data=json.dumps({'id': wflow.id,'flow_id': define.id @@ -642,9 +708,9 @@ def test_update_workflow(self,client,db,db_register2,users,workflow): assert res.status_code == 200 wf : WorkFlow = db.session.query(WorkFlow).filter_by(id = wflow.id).one_or_none() assert wf.open_restricted == False - + url = url_for('workflowsetting.update_workflow',workflow_id='0',_external=True) - res = client.post(url + res = client.post(url , headers=[('Content-Type', 'application/json') ,('Accept', 'application/json')] , data=json.dumps({'id': wflow.id,'flow_id': define.id @@ -653,6 +719,40 @@ def test_update_workflow(self,client,db,db_register2,users,workflow): ,'is_gakuninrdm' : False}) ) + # .tox/c1/bin/pytest --cov=weko_workflow tests/test_admin.py::TestWorkFlowSettingView::test_workflow_detail_roles_filter -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp + def test_workflow_detail_roles_filter(self, client, db, users): + from invenio_accounts.models import Role + from invenio_accounts.testutils import login_user_via_session as login + + client.application.config.update(dict( + WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT={ + "prefix": "jc", + "role_keyword": "roles", + "role_mapping": { + "repoadm": "Repository Administrator", + "comadm": "Community Administrator", + "contributor": "Contributor", + } + } + )) + + user = users[1]['obj'] + db.session.add(user) + role1 = Role(name="Contributor_test", description=None) + role2 = Role(name="jc_xxx_roles_contributor", description=None) + role3 = Role(name="jc_xxx_groups_yyy", description=None) + db.session.add_all([role1, role2, role3]) + db.session.commit() + + login(client=client, email=users[1]['email']) + with patch("flask.templating._render", return_value=b"") as mock_render: + response = client.get('/admin/workflowsetting/0') + args, kwargs = mock_render.call_args + context = args[1] + filtered_role_names = [role.name for role in context['display_list']] + assert "jc_xxx_roles_contributor" not in filtered_role_names + assert "jc_xxx_groups_yyy" in filtered_role_names + assert "Contributor_test" in filtered_role_names # def delete_workflow(self, workflow_id='0'): # .tox/c1/bin/pytest --cov=weko_workflow tests/test_admin.py::TestWorkFlowSettingView::test_delete_workflow_acl_guest -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp @@ -846,7 +946,7 @@ def test_index(self,client,db,admin_settings,app,users,db_register2,mocker): data = { "workFlow_select_flg":"1", "submit":"set_workspace_workflow_setting_form" - + } mock_render = mocker.patch("weko_workflow.admin.WorkSpaceWorkFlowSettingView.render",return_value=make_response()) from flask import abort, current_app, jsonify, flash, request @@ -856,7 +956,7 @@ def test_index(self,client,db,admin_settings,app,users,db_register2,mocker): data = { "registrationRadio":"1", "submit":"set_workspace_workflow_setting_form" - + } mock_render = mocker.patch("weko_workflow.admin.WorkSpaceWorkFlowSettingView.render",return_value=make_response()) from flask import abort, current_app, jsonify, flash, request @@ -866,7 +966,7 @@ def test_index(self,client,db,admin_settings,app,users,db_register2,mocker): data = { "registrationRadio":"1", "submit":"set_workspace_workflow_setting_form" - + } mock_render = mocker.patch("weko_workflow.admin.WorkSpaceWorkFlowSettingView.render",return_value=make_response()) from flask import abort, current_app, jsonify, flash, request From aac37d1bd1afdddfa04743260a63df5d3977a4d5 Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Sun, 19 Oct 2025 16:25:13 +0900 Subject: [PATCH 06/19] Unit Test for Index Membership Conditions --- modules/weko-index-tree/tests/test_utils.py | 136 ++++++++++++++++---- 1 file changed, 110 insertions(+), 26 deletions(-) diff --git a/modules/weko-index-tree/tests/test_utils.py b/modules/weko-index-tree/tests/test_utils.py index b1f167e077..968f6bb0a9 100644 --- a/modules/weko-index-tree/tests/test_utils.py +++ b/modules/weko-index-tree/tests/test_utils.py @@ -218,31 +218,91 @@ def test_get_user_groups(i18n_app, client_rest, users, db): # .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_check_roles -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp #+++ def check_roles(user_role, roles): def test_check_roles(i18n_app, users): - # admin user - user_role = (True, []) - roles = ["1","2"] - check_roles(user_role, roles) + # Match both role and role group + params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} + user_role = (False, ["10"]) + roles = ["10"] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is True - # not admin user - ## not login - ### not allow -99 - user_role = (False,[]) - roles = "1,2" - assert check_roles(user_role, roles) == False - ### allow -99 - user_role = (False,[]) - roles = "1,2,-99" - assert check_roles(user_role, roles) == True - ## login + # Only role group matches + params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} + user_role = (False, ["30"]) + roles = ["20"] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is False + + # Only role matches + params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} + user_role = (False, ["30"]) + roles = ["30"] with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - ### all allow - user_role = (False,["1", "2"]) - roles = "1,2" - assert check_roles(user_role, roles) == True - ### exist deny - user_role = (False,["1", "2", "3"]) - roles = "1,2" - assert check_roles(user_role, roles) == False + assert check_roles(user_role, roles, params) is True + + # Neither role nor role group matches + params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} + user_role = (False, ["40"]) + roles = ["50"] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is False + + # Both not set (group check) + params = {"role_groups": [], "groups": [100], "access_group_ids": [200]} + user_role = (False, ["40"]) + roles = [] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is False + + # Admin user + user_role = (True, ["1"]) + roles = ["1", "2"] + params = {"role_groups": [10], "groups": [100], "access_group_ids": [200]} + assert check_roles(user_role, roles, params) is True + + # Guest user (role -99) + params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} + user_role = (False, []) + roles = ["-99"] + with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=False)): + assert check_roles(user_role, roles, params) is True + + # Role group is [10], roles are [10, 30], user_role is (not admin, [10, 30]) + params = {"role_groups": [10], "groups": [100], "access_group_ids": [200]} + user_role = (False, ["10", "30"]) + roles = ["10", "30"] + with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=True)): + assert check_roles(user_role, roles, params) is True + + # Role group is [10, 20], roles are [30], user_role is (not admin, [30]) + params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} + user_role = (False, []) + roles = ["-99", "10"] + with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=False)): + assert check_roles(user_role, roles, params) is False + +# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_set_params -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp +def test_set_params(app): + with app.app_context(): + from weko_index_tree.utils import set_params + class MockRole: + def __init__(self, id, name): + self.id = id + self.name = name + mock_roles = [ + MockRole(1, "group_admin"), + MockRole(2, "user"), + MockRole(3, "group_editor"), + MockRole(4, "guest") + ] + with patch("invenio_accounts.models.Role.query") as mock_query: + mock_query.all.return_value = mock_roles + groups = [100, 200] + index_group = [300] + params = set_params(groups, index_group) + + assert params["role_groups"] == [1, 3] + assert params["groups"] == [100, 200] + assert params["access_group_ids"] == [300] #+++ def check_groups(user_group, groups): def test_check_groups(i18n_app, users, db): @@ -264,19 +324,43 @@ def test_check_groups(i18n_app, users, db): #+++ def filter_index_list_by_role(index_list): # def _check(index_data, roles, groups): def test_filter_index_list_by_role(i18n_app, indices, users, db): + # Case 1: check_roles returns False + class DummyIndex1: + def __init__(self): + self.browsing_group = [] + self.browsing_role = [] + self.public_state = True + self.public_date = None + dummy1 = DummyIndex1() + with patch("weko_index_tree.utils.set_params", return_value={"role_groups": [], "groups": [], "access_group_ids": []}): + with patch("weko_index_tree.utils.check_roles", return_value=False): + assert filter_index_list_by_role([dummy1]) == [] + + # Case 2: public_state is False + class DummyIndex2: + def __init__(self): + self.browsing_group = [] + self.browsing_role = [] + self.public_state = False + self.public_date = None + dummy2 = DummyIndex2() + with patch("weko_index_tree.utils.set_params", return_value={"role_groups": [], "groups": [], "access_group_ids": []}): + with patch("weko_index_tree.utils.check_roles", return_value=True): + with patch("weko_records_ui.utils.is_future", return_value=False): + assert filter_index_list_by_role([dummy2]) == [] + + # Case 3: normal access (user logged in and has group membership) with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): from weko_groups.models import Group g1 = Group.create(name="group_test1").add_member(users[-1]['obj']) g2 = Group.create(name="group_test2").add_member(users[-1]['obj']) - db.session.add(g1) db.session.add(g2) - assert len(filter_index_list_by_role([indices['index_non_dict']])) > 0 + # Case 4: not logged in assert len(filter_index_list_by_role([indices['index_non_dict']])) == 1 - # def reduce_index_by_role # .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_reduce_index_by_role -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp def test_reduce_index_by_role(app, db, users): From 53463e955e60391b57786b2c4758a1511dc9d36f Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Wed, 22 Oct 2025 18:04:38 +0900 Subject: [PATCH 07/19] Correction of Pointed Issues --- .../invenio_communities/admin.py | 2 +- modules/weko-index-tree/weko_index_tree/api.py | 14 +++++++------- modules/weko-index-tree/weko_index_tree/utils.py | 3 ++- modules/weko-workflow/weko_workflow/admin.py | 10 +++++++--- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/modules/invenio-communities/invenio_communities/admin.py b/modules/invenio-communities/invenio_communities/admin.py index 7f0f54f277..25b92e3ac2 100644 --- a/modules/invenio-communities/invenio_communities/admin.py +++ b/modules/invenio-communities/invenio_communities/admin.py @@ -565,7 +565,7 @@ def validate_community_id(self, community_id): 'query_factory': lambda: db.session.query(Role).filter( ~( Role.name.like(f"%{current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('role_keyword','')}%") & - Role.name.like(f"%{current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('prefix','')}%") + Role.name.startswith(current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'].get('prefix','')) ) ).all(), }, diff --git a/modules/weko-index-tree/weko_index_tree/api.py b/modules/weko-index-tree/weko_index_tree/api.py index 46c6209e43..16ac64e771 100644 --- a/modules/weko-index-tree/weko_index_tree/api.py +++ b/modules/weko-index-tree/weko_index_tree/api.py @@ -837,7 +837,7 @@ def get_recursive_tree(cls, pid: int = 0, lang: str = None, with_deleted: bool = @classmethod def filter_roles(cls, roles): """Filter roles to gakunin_map group roles, gakunin_map general roles, other roles. - + Args: roles (list): List of role dictionaries to be filtered. Returns: @@ -888,7 +888,7 @@ def _get_allow_deny(allow, role, browse_flag=False): if tmp["name"] not in current_app.config['WEKO_PERMISSION_SUPER_ROLE_USER']: role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] - if tmp["name"] not in role_key and not (tmp["name"].startswith(prefix)): + if role_key not in tmp["name"] and not (tmp["name"].startswith(prefix)): if str(tmp["id"]) in allow: alw.append(tmp) else: @@ -2202,7 +2202,7 @@ def bind_roles_including_permission(cls, roles, permission): continue bind_roles.append(role) return bind_roles - + @classmethod def update_browsing_roles_groups(cls, current_index, index_setting, update_browsing_role, update_browsing_groups): @@ -2217,7 +2217,7 @@ def update_browsing_roles_groups(cls, current_index, index_setting, # if not update browsing roles and groups if not update_browsing_role and not update_browsing_groups: return - + browsing_roles = set( current_index.browsing_role.split(",") if current_index.browsing_role else [] ) @@ -2273,18 +2273,18 @@ def update_contribute_roles_groups(cls, current_index, index_setting, # if not update contribute roles and groups if not update_contribute_role and not update_contribute_groups: return - + contribute_roles = set( current_index.contribute_role.split(",") if current_index.contribute_role else [] ) contribute_groups = set( current_index.contribute_group.split(",") if current_index.contribute_group else [] ) - + # if update contribute roles if update_contribute_role: contribute_roles = set(index_setting.get('contribute_role_ids', [])) - + # if update contribute groups if update_contribute_groups: contribute_roles = contribute_roles.union( diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index 48870f4f87..35afc6254f 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -346,7 +346,8 @@ def check_roles(user_role, roles, params): group_perm = any(str(rg) in [str(sr) for sr in self_role] for rg in index_role_group) if set_index_role: # If the index has both role groups and roles. - role_perm = any(str(role) in [str(r) for r in roles_list] for role in self_role) + index_role = [int(r) for r in roles_list if int(r) not in params["role_groups"]] + role_perm = any(str(role) in [str(r) for r in index_role] for role in self_role) is_can = group_perm and role_perm else: # If the index has role groups but no roles. diff --git a/modules/weko-workflow/weko_workflow/admin.py b/modules/weko-workflow/weko_workflow/admin.py index 5fd4f006db..122980b33b 100644 --- a/modules/weko-workflow/weko_workflow/admin.py +++ b/modules/weko-workflow/weko_workflow/admin.py @@ -73,7 +73,7 @@ def flow_detail(self, flow_id='0'): role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] roles = Role.query.filter( - ~and_(Role.name.like(f"%{role_key}%"), Role.name.like(f"%{prefix}%")) + ~and_(Role.name.like(f"%{role_key}%"), Role.name.startswith(prefix)) ).all() if set(role.name for role in current_user.roles) & \ set(current_app.config['WEKO_PERMISSION_SUPER_ROLE_USER']): @@ -322,7 +322,11 @@ def index(self): """ workflow = WorkFlow() workflows = workflow.get_workflow_list(user=current_user) - role = Role.query.filter(~Role.name.like('%roles%')).all() + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + role = Role.query.filter( + ~and_(Role.name.like(f"%{role_key}%"), Role.name.startswith(prefix)) + ).all() for wf in workflows: index_tree = Index().get_index_by_id(wf.index_tree_id) wf.index_tree = index_tree @@ -364,7 +368,7 @@ def workflow_detail(self, workflow_id='0'): role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] role = Role.query.filter( - ~and_(Role.name.like(f"%{role_key}%"), Role.name.like(f"%{prefix}%")) + ~and_(Role.name.like(f"%{role_key}%"), Role.name.startswith(prefix)) ).all() display_label = self.get_language_workflows("display") hide_label = self.get_language_workflows("hide") From 33064a200dd602642f1d1fcdd494ac8684f745e7 Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Thu, 23 Oct 2025 14:16:45 +0900 Subject: [PATCH 08/19] delete_info_display && _get_allow_deny unit test --- .../invenio_accounts/admin.py | 9 ++- .../invenio_accounts/models.py | 20 ------- modules/invenio-accounts/tests/test_admin.py | 24 ++++++++ modules/invenio-accounts/tests/test_models.py | 39 ------------- modules/weko-index-tree/tests/test_api.py | 56 +++++++++++++++++++ 5 files changed, 88 insertions(+), 60 deletions(-) diff --git a/modules/invenio-accounts/invenio_accounts/admin.py b/modules/invenio-accounts/invenio_accounts/admin.py index 36f5ea212a..7ac1c09e9e 100644 --- a/modules/invenio-accounts/invenio_accounts/admin.py +++ b/modules/invenio-accounts/invenio_accounts/admin.py @@ -88,7 +88,14 @@ def scaffold_form(self): form_class = super(UserView, self).scaffold_form() form_class.role = QuerySelectMultipleField( 'Roles', - query_factory=lambda: Role.query.filter(~Role.name.like('%_groups_%')).all(), + query_factory=lambda: ( + Role.query.filter( + ~( + Role.name.like(f"%{current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT']['role_keyword']}%") & + Role.name.startswith(current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT']['prefix']) + ) + ).all() + ), get_label='name', widget=Select2Widget(multiple=True) ) diff --git a/modules/invenio-accounts/invenio_accounts/models.py b/modules/invenio-accounts/invenio_accounts/models.py index ab123da84d..b7fb56ea85 100644 --- a/modules/invenio-accounts/invenio_accounts/models.py +++ b/modules/invenio-accounts/invenio_accounts/models.py @@ -47,26 +47,6 @@ def __str__(self): """Return the name and description of the role.""" return '{0.name} - {0.description}'.format(self) - @property - def info_display(self): - """Return organized role info as dict.""" - roles_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] - role_mapping = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_mapping"] - role_name = self.name - # roles_keyやrole_mappingに含まれる場合は渡さない - suffix = None - if roles_key in role_name: - suffix = role_name.split(roles_key + '_')[-1] - if (roles_key in role_name and suffix and suffix not in role_mapping.keys()) or (roles_key not in role_name): - return { - "id": self.id, - "name": role_name, - "description": self.description - } - - # それ以外はNone - return None - class User(db.Model, UserMixin): """User data model.""" diff --git a/modules/invenio-accounts/tests/test_admin.py b/modules/invenio-accounts/tests/test_admin.py index db834cc7da..c74e2d4305 100644 --- a/modules/invenio-accounts/tests/test_admin.py +++ b/modules/invenio-accounts/tests/test_admin.py @@ -283,3 +283,27 @@ def test_userview_edit_form(app, users): view = UserView(User, db.session) form = view.edit_form() assert form.data['active'] is False + +# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_admin.py::test_scaffold_form -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp +def test_scaffold_form(app): + """Test scaffold_form method of UserView.""" + with app.app_context(): + current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT'] = { + 'role_keyword': 'roles', + 'prefix': 'jc' + } + db.session.add(Role(id=1, name='Contributor', description=None)) + db.session.add(Role(id=2, name='jc_xxx_roles_contributor', description=None)) + db.session.add(Role(id=3, name='jc_xxx_groups_yyy', description=None)) + db.session.commit() + + view = UserView(User, db.session) + form_class = view.scaffold_form() + + roles = form_class.role.kwargs['query_factory']() + groups = form_class.group.kwargs['query_factory']() + + role_names = [r.name for r in roles] + group_names = [g.name for g in groups] + assert 'Contributor' in role_names + assert 'jc_xxx_groups_yyy' in group_names diff --git a/modules/invenio-accounts/tests/test_models.py b/modules/invenio-accounts/tests/test_models.py index d71152d4b5..6866b888ec 100644 --- a/modules/invenio-accounts/tests/test_models.py +++ b/modules/invenio-accounts/tests/test_models.py @@ -73,42 +73,3 @@ def test_get_email_by_id(app, users): with app.test_client() as client: lst = User.get_email_by_id(1) assert len(lst) > 0 - -# .tox/c1/bin/pytest --cov=invenio_accounts tests/test_models.py::test_info_display -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/invenio-accounts/.tox/c1/tmp -def test_info_display(app): - """Test Role.info_display property.""" - from invenio_accounts.models import Role - with app.app_context(): - app.config.update(dict( - WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT= - { - "role_keyword": "roles", - "role_mapping": { - "repoadm": "Repository Administrator", - "comadm": "Community Administrator", - "contributor": "Contributor", - } - } - )) - role1 = Role(id=1, name="Contributor", description=None) - role2 = Role(id=2, name="jc_xxx_roles_contributor", description=None) - role3 = Role(id=3, name="jc_xxx_groups_yyy", description=None) - db.session.add_all([role1, role2, role3]) - db.session.commit() - - # role_keywordを含み、role_mappingに含まれないsuffix - assert role1.info_display == { - "id": 1, - "name": "Contributor", - "description": None - } - - # role_keywordを含み、role_mappingに含まれるsuffix - assert role2.info_display is None - - # role_keywordを含まない - assert role3.info_display == { - "id": 3, - "name": "jc_xxx_groups_yyy", - "description": None - } diff --git a/modules/weko-index-tree/tests/test_api.py b/modules/weko-index-tree/tests/test_api.py index 439be311cb..0b597cabe9 100644 --- a/modules/weko-index-tree/tests/test_api.py +++ b/modules/weko-index-tree/tests/test_api.py @@ -1231,3 +1231,59 @@ def test_indexes_get_handle_index_url(app, db, users, test_indices, mocker): handle, index_url = Indexes.get_handle_index_url(1) assert index_url == "http://TEST_SERVER/search?search_type=2&q=1" assert handle == "https://test/handle/1" + +# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_api.py::test_get_allow_deny -v -s -vv --cov-branch --cov-report=html --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp +def test_get_allow_deny(app, db, mocker): + with app.app_context(): + mocker.patch.dict(current_app.config, { + 'WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT': { + 'prefix': 'jc', + 'role_keyword': 'roles' + }, + 'WEKO_PERMISSION_SUPER_ROLE_USER': ['System Administrator', 'Repository Administrator'] + }) + + # Patterns for super role, role_keyword, prefix, allow/deny + roles = [ + {"id": 1, "name": "NormalRole"}, # subject to allow/deny + {"id": 2, "name": "roles_test"}, # contains role_keyword → skip + {"id": 3, "name": "jcAdmin"}, # starts with prefix → skip + {"id": 4, "name": "System Administrator"}, # super role → skip + {"id": 5, "name": "OtherRole"}, # subject to allow/deny + ] + index_data = { + 'id': 1, + 'browsing_role': '1,5', # allow list + 'contribute_role': '1', # allow list + 'browsing_group': '', + 'contribute_group': '', + 'public_date': None, + } + mocker.patch.object(Indexes, 'get_index', return_value=index_data) + mocker.patch.object(Indexes, 'get_account_role', return_value=roles) + mocker.patch('weko_index_tree.api.Group.query.all', return_value=[]) + + result = Indexes.get_index_with_role(1) + + # Only 1 and 5 are allowed for browsing_role, others are deny or skipped + assert [r['id'] for r in result['browsing_role']['allow']] == [1, 5] + assert [r['id'] for r in result['browsing_role']['deny']] == [] + + # Only 1 is allowed for contribute_role, 5 is deny + index_data['contribute_role'] = '1' + result = Indexes.get_index_with_role(1) + assert [r['id'] for r in result['contribute_role']['allow']] == [1] + assert [r['id'] for r in result['contribute_role']['deny']] == [5] + + # When allow list is empty + index_data['browsing_role'] = '' + result = Indexes.get_index_with_role(1) + assert [r['id'] for r in result['browsing_role']['allow']] == [] + assert [r['id'] for r in result['browsing_role']['deny']] == [1, 5] + + # When role list is empty + mocker.patch.object(Indexes, 'get_account_role', return_value=[]) + index_data['browsing_role'] = '1,5' + result = Indexes.get_index_with_role(1) + assert result['browsing_role']['allow'] == [] + assert result['browsing_role']['deny'] == [] From bfd6b222ee04a101865e854e9bdb5dc6660a2320 Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Fri, 24 Oct 2025 00:28:16 +0900 Subject: [PATCH 09/19] fix weko-index-tree unit test --- modules/weko-index-tree/tests/test_api.py | 21 +++++++++--------- modules/weko-index-tree/tests/test_utils.py | 22 +++++++++++++++++++ .../weko-index-tree/weko_index_tree/api.py | 2 +- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/modules/weko-index-tree/tests/test_api.py b/modules/weko-index-tree/tests/test_api.py index 0b597cabe9..13e8f55790 100644 --- a/modules/weko-index-tree/tests/test_api.py +++ b/modules/weko-index-tree/tests/test_api.py @@ -1246,15 +1246,16 @@ def test_get_allow_deny(app, db, mocker): # Patterns for super role, role_keyword, prefix, allow/deny roles = [ {"id": 1, "name": "NormalRole"}, # subject to allow/deny - {"id": 2, "name": "roles_test"}, # contains role_keyword → skip - {"id": 3, "name": "jcAdmin"}, # starts with prefix → skip + {"id": 2, "name": "roles_test"}, # subject to allow/deny + {"id": 3, "name": "jcAdmin"}, # starts with 'jc' and does not contain 'roles' → excluded by filter_roles {"id": 4, "name": "System Administrator"}, # super role → skip {"id": 5, "name": "OtherRole"}, # subject to allow/deny + {"id": 6, "name": "jc_test_roles"}, # contains prefix and role_keyword → skip ] index_data = { 'id': 1, - 'browsing_role': '1,5', # allow list - 'contribute_role': '1', # allow list + 'browsing_role': '1,2,3,4,5,6', # allow list + 'contribute_role': '1,2,3,4,5,6', # allow list 'browsing_group': '', 'contribute_group': '', 'public_date': None, @@ -1265,25 +1266,25 @@ def test_get_allow_deny(app, db, mocker): result = Indexes.get_index_with_role(1) - # Only 1 and 5 are allowed for browsing_role, others are deny or skipped - assert [r['id'] for r in result['browsing_role']['allow']] == [1, 5] + # browsing_role: allow = [1, 2, 5], deny = [] (others are skipped) + assert [r['id'] for r in result['browsing_role']['allow']] == [1, 2, 5] assert [r['id'] for r in result['browsing_role']['deny']] == [] - # Only 1 is allowed for contribute_role, 5 is deny + # contribute_role: allow = [1], deny = [2, 5] (others are skipped) index_data['contribute_role'] = '1' result = Indexes.get_index_with_role(1) assert [r['id'] for r in result['contribute_role']['allow']] == [1] - assert [r['id'] for r in result['contribute_role']['deny']] == [5] + assert [r['id'] for r in result['contribute_role']['deny']] == [2, 5] # When allow list is empty index_data['browsing_role'] = '' result = Indexes.get_index_with_role(1) assert [r['id'] for r in result['browsing_role']['allow']] == [] - assert [r['id'] for r in result['browsing_role']['deny']] == [1, 5] + assert [r['id'] for r in result['browsing_role']['deny']] == [1, 2, 5] # When role list is empty mocker.patch.object(Indexes, 'get_account_role', return_value=[]) - index_data['browsing_role'] = '1,5' + index_data['browsing_role'] = '1,2,3,4,5,6' result = Indexes.get_index_with_role(1) assert result['browsing_role']['allow'] == [] assert result['browsing_role']['deny'] == [] diff --git a/modules/weko-index-tree/tests/test_utils.py b/modules/weko-index-tree/tests/test_utils.py index 968f6bb0a9..40dc39d381 100644 --- a/modules/weko-index-tree/tests/test_utils.py +++ b/modules/weko-index-tree/tests/test_utils.py @@ -280,6 +280,28 @@ def test_check_roles(i18n_app, users): with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=False)): assert check_roles(user_role, roles, params) is False + # User has only the role set for the index + params = {"role_groups": [100, 200], "groups": [300], "access_group_ids": [400]} + user_role = (False, ["1"]) + roles = ["1", "100"] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is False + + # User has only the role group set for the index + params = {"role_groups": [100, 200], "groups": [300], "access_group_ids": [400]} + user_role = (False, ["100"]) + roles = ["1", "100"] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is False + + # User has neither the role nor the role group set for the index + params = {"role_groups": [100, 200], "groups": [300], "access_group_ids": [400]} + user_role = (False, ["999"]) + roles = ["1", "100"] + with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): + assert check_roles(user_role, roles, params) is False + + # .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_set_params -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp def test_set_params(app): with app.app_context(): diff --git a/modules/weko-index-tree/weko_index_tree/api.py b/modules/weko-index-tree/weko_index_tree/api.py index 16ac64e771..f43fc9a977 100644 --- a/modules/weko-index-tree/weko_index_tree/api.py +++ b/modules/weko-index-tree/weko_index_tree/api.py @@ -888,7 +888,7 @@ def _get_allow_deny(allow, role, browse_flag=False): if tmp["name"] not in current_app.config['WEKO_PERMISSION_SUPER_ROLE_USER']: role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] - if role_key not in tmp["name"] and not (tmp["name"].startswith(prefix)): + if role_key not in tmp["name"] or not (tmp["name"].startswith(prefix)): if str(tmp["id"]) in allow: alw.append(tmp) else: From 51f1a5a24cd72b0e65c27413dd954c5d7b514cf5 Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Fri, 24 Oct 2025 14:26:20 +0900 Subject: [PATCH 10/19] fix scaffold_form --- modules/invenio-accounts/invenio_accounts/admin.py | 2 +- modules/invenio-accounts/tests/test_admin.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/invenio-accounts/invenio_accounts/admin.py b/modules/invenio-accounts/invenio_accounts/admin.py index 7ac1c09e9e..f6b25c0067 100644 --- a/modules/invenio-accounts/invenio_accounts/admin.py +++ b/modules/invenio-accounts/invenio_accounts/admin.py @@ -94,7 +94,7 @@ def scaffold_form(self): Role.name.like(f"%{current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT']['role_keyword']}%") & Role.name.startswith(current_app.config['WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT']['prefix']) ) - ).all() + ).filter(~Role.name.like('%_groups_%')).all() ), get_label='name', widget=Select2Widget(multiple=True) diff --git a/modules/invenio-accounts/tests/test_admin.py b/modules/invenio-accounts/tests/test_admin.py index c74e2d4305..b565b87c88 100644 --- a/modules/invenio-accounts/tests/test_admin.py +++ b/modules/invenio-accounts/tests/test_admin.py @@ -305,5 +305,13 @@ def test_scaffold_form(app): role_names = [r.name for r in roles] group_names = [g.name for g in groups] + # Check included roles assert 'Contributor' in role_names + # Check included groups assert 'jc_xxx_groups_yyy' in group_names + # Check excluded roles + assert 'jc_xxx_roles_contributor' not in role_names + assert 'jc_xxx_groups_yyy' not in role_names + # Check excluded groups + assert 'Contributor' not in group_names + assert 'jc_xxx_roles_contributor' not in group_names From 7d043d9b531b6e6f2941b16522e00e7e055b9a01 Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Wed, 29 Oct 2025 12:45:11 +0900 Subject: [PATCH 11/19] fix check_roles --- modules/weko-index-tree/weko_index_tree/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index 35afc6254f..e5b9cf523a 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -339,7 +339,8 @@ def check_roles(user_role, roles, params): set_index_group = any(int(r) in params["role_groups"] for r in roles_list) set_index_role = any(int(r) not in params["role_groups"] for r in roles_list) if current_user.is_authenticated: - self_role = user_role[1] or ['-98'] + self_role = user_role[1] or [] + self_role.append('-98') if set_index_group: index_role_group = [int(r) for r in roles_list if int(r) in params["role_groups"]] # Groups(role) are set for the index. From 1e13f007e737716c8d1edaa009d271fb5047264f Mon Sep 17 00:00:00 2001 From: "kenji.shiokawa" Date: Thu, 30 Oct 2025 04:28:04 +0900 Subject: [PATCH 12/19] fix workflow_detail hide_list --- modules/weko-workflow/weko_workflow/admin.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/modules/weko-workflow/weko_workflow/admin.py b/modules/weko-workflow/weko_workflow/admin.py index 122980b33b..ce6a4e8d0e 100644 --- a/modules/weko-workflow/weko_workflow/admin.py +++ b/modules/weko-workflow/weko_workflow/admin.py @@ -414,6 +414,7 @@ def workflow_detail(self, workflow_id='0'): .filter(WorkflowRole.workflow_id == workflows.id) \ .filter(WorkflowRole.role_id == Role.id) \ .all() + hide = [r for r in hide if r in role] if hide: display = self.get_displays(hide, role) else: @@ -573,8 +574,22 @@ def save_workflow_role(cls, wf_id, list_hide): # ['4'] current_app.logger.error("list_hide:{}".format(list_hide)) with db.session.begin_nested(): - db.session.query(WorkflowRole).filter_by( - workflow_id=wf_id).delete() + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + roles_to_delete = db.session.query(WorkflowRole).filter_by(workflow_id=wf_id).all() + filtered_role_ids = [] + for wfrole in roles_to_delete: + role = Role.query.filter_by(id=wfrole.role_id).first() + if role: + if role_key in role.name and role.name.startswith(prefix): + continue + filtered_role_ids.append(wfrole.role_id) + + if filtered_role_ids: + db.session.query(WorkflowRole).filter( + WorkflowRole.workflow_id == wf_id, + WorkflowRole.role_id.in_(filtered_role_ids) + ).delete(synchronize_session=False) if isinstance(list_hide, list): while list_hide: tmp = list_hide.pop(0) From 1e3cac03f7d0b09c33937edebbba816282dce7cc Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Mon, 10 Nov 2025 18:23:53 +0900 Subject: [PATCH 13/19] fix select workflow --- modules/weko-workflow/tests/test_api.py | 46 +++++++++++++++++++- modules/weko-workflow/weko_workflow/admin.py | 18 +------- modules/weko-workflow/weko_workflow/api.py | 8 +++- 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/modules/weko-workflow/tests/test_api.py b/modules/weko-workflow/tests/test_api.py index 5719dd63e5..999d99facd 100644 --- a/modules/weko-workflow/tests/test_api.py +++ b/modules/weko-workflow/tests/test_api.py @@ -799,6 +799,50 @@ def test_WorkFlow_get_workflow_list(app, db, workflow, users): user = users[3]["obj"] res = _workflow.get_workflow_list(user=user) assert len(res) == 1 + + +# .tox/c1/bin/pytest --cov=weko_workflow tests/test_api.py::test_get_workflows_by_roles -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp +def test_get_workflows_by_roles(app, mocker): + role_1 = MagicMock(id=1, name='') + role_2 = MagicMock(id=2, name='') + role_3 = MagicMock(id=3, name='') + + mock_user = MagicMock() + mock_user.roles = [role_1, role_2] + mocker.patch("flask_login.utils._get_user", return_value=mock_user) # current_user_roles + + mock_query = mocker.patch("invenio_accounts.models.Role.query") + mock_filter = mock_query.filter.return_value + mock_filter.all.return_value = [role_1, role_3] # all roles + + mock_outerjoin = mock_query.outerjoin.return_value + mock_filter1 = mock_outerjoin.filter.return_value + mock_filter2 = mock_filter1.filter.return_value + mock_filter2.all.return_value = [role_1] # list_hide + + wf = WorkFlow() + wf1 = MagicMock(id=11, name='') + + # user_roles:[role_1] - list_hide:[role_1] = [] -> not show wf1 + workflows = wf.get_workflows_by_roles([wf1]) + assert workflows == [] + + mock_filter2.all.return_value = [] # list_hide + + # user_roles:[role_1] - list_hide:[] = [role_1] -> show wf1 + workflows = wf.get_workflows_by_roles([wf1]) + assert workflows == [wf1] + + # argument is None + workflows = wf.get_workflows_by_roles(None) + assert workflows == [] + + # user_roles is None + mock_filter.all.return_value = None # role + workflows = wf.get_workflows_by_roles([wf1]) + assert workflows == [] + + # .tox/c1/bin/pytest --cov=weko_workflow tests/test_api.py::test_WorkActivity_filter_by_action -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp def test_WorkActivity_filter_by_action(app, db): query = db.session.query(Activity) @@ -2156,7 +2200,7 @@ def test_query_activities_by_tab_is_wait(users, db): "OR workflow_activity_action.action_handler NOT IN (%(action_handler_1)s))" ret = WorkActivity.query_activities_by_tab_is_wait(query, False, False, []) assert str(ret).find(expected) != -1 - + # admin user with patch("flask_login.utils._get_user", return_value=users[1]['obj']): admin_expected = expected.replace( diff --git a/modules/weko-workflow/weko_workflow/admin.py b/modules/weko-workflow/weko_workflow/admin.py index ce6a4e8d0e..6d04caa4b2 100644 --- a/modules/weko-workflow/weko_workflow/admin.py +++ b/modules/weko-workflow/weko_workflow/admin.py @@ -574,22 +574,8 @@ def save_workflow_role(cls, wf_id, list_hide): # ['4'] current_app.logger.error("list_hide:{}".format(list_hide)) with db.session.begin_nested(): - role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] - prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] - roles_to_delete = db.session.query(WorkflowRole).filter_by(workflow_id=wf_id).all() - filtered_role_ids = [] - for wfrole in roles_to_delete: - role = Role.query.filter_by(id=wfrole.role_id).first() - if role: - if role_key in role.name and role.name.startswith(prefix): - continue - filtered_role_ids.append(wfrole.role_id) - - if filtered_role_ids: - db.session.query(WorkflowRole).filter( - WorkflowRole.workflow_id == wf_id, - WorkflowRole.role_id.in_(filtered_role_ids) - ).delete(synchronize_session=False) + db.session.query(WorkflowRole).filter_by( + workflow_id=wf_id).delete() if isinstance(list_hide, list): while list_hide: tmp = list_hide.pop(0) diff --git a/modules/weko-workflow/weko_workflow/api.py b/modules/weko-workflow/weko_workflow/api.py index e4ab10cca8..3db1f65e2b 100644 --- a/modules/weko-workflow/weko_workflow/api.py +++ b/modules/weko-workflow/weko_workflow/api.py @@ -728,7 +728,11 @@ def get_display_role(list_hide, role): wfs = [] current_user_roles = [role.id for role in current_user.roles] if isinstance(workflows, list): - role = Role.query.all() + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + role = Role.query.filter( + ~and_(Role.name.like(f"%{role_key}%"), Role.name.startswith(prefix)) + ).all() while workflows: tmp = workflows.pop(0) list_hide = Role.query.outerjoin(WorkflowRole) \ @@ -2283,7 +2287,7 @@ def __get_community_user_ids(): ).all() com_roles = list( set([comm.id_role for comm in comm_list]).union( - set([comm.group_id for comm in comm_list]))) + set([comm.group_id for comm in comm_list]))) if com_roles: com_users = User.query.outerjoin(userrole).outerjoin(Role) \ From 9c0d51675b7351898bde97b8fe6ad8ead6a701cc Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Tue, 11 Nov 2025 21:48:36 +0900 Subject: [PATCH 14/19] fix index permissions --- modules/weko-index-tree/tests/conftest.py | 3 +- modules/weko-index-tree/tests/test_api.py | 62 ++++-- modules/weko-index-tree/tests/test_utils.py | 210 +++++++----------- .../weko-index-tree/weko_index_tree/api.py | 36 ++- .../weko-index-tree/weko_index_tree/utils.py | 162 +++++--------- 5 files changed, 215 insertions(+), 258 deletions(-) diff --git a/modules/weko-index-tree/tests/conftest.py b/modules/weko-index-tree/tests/conftest.py index 1b472d5e87..6d868d0de1 100644 --- a/modules/weko-index-tree/tests/conftest.py +++ b/modules/weko-index-tree/tests/conftest.py @@ -728,6 +728,7 @@ def indices(app, db): testIndexThree = Index( index_name="testIndexThree", browsing_role="1,2,3,4,-98,-99", + browsing_group="-89", public_state=True, harvest_public_state=True, id=33, @@ -796,7 +797,7 @@ def base_index(id, parent, position, harvest_public_state=True, public_state=Tru recursive_contribute_group=False, online_issn='', is_deleted=False): _browsing_role = "3,-99" _contribute_role = "1,2,3,4,-98,-99" - _group = "g1,g2" + _group = "g1,g2,-89" return Index( id=id, parent=parent, diff --git a/modules/weko-index-tree/tests/test_api.py b/modules/weko-index-tree/tests/test_api.py index 13e8f55790..7b0e4409eb 100644 --- a/modules/weko-index-tree/tests/test_api.py +++ b/modules/weko-index-tree/tests/test_api.py @@ -27,6 +27,7 @@ import os from datetime import datetime from mock import patch, Mock, MagicMock +from types import SimpleNamespace from redis.exceptions import RedisError from elasticsearch.exceptions import NotFoundError @@ -34,7 +35,7 @@ from invenio_communities.models import Community from invenio_accounts.testutils import login_user_via_view, login_user_via_session from invenio_i18n.ext import current_i18n -from sqlalchemy.exc import IntegrityError +from sqlalchemy.exc import IntegrityError, SQLAlchemyError from flask import current_app from weko_deposit.api import WekoDeposit from weko_index_tree.api import Indexes @@ -829,25 +830,25 @@ def test_indexes_get_index_tree(i18n_app, db, redis_connect, users, db_records, res = Indexes.get_recursive_tree() assert len(res)==7 res = Indexes.get_recursive_tree(11) - assert res==[(1, 11, 0, 'テストインデックス 11', 'Test index link 11_ja', True, True, None, '3,-99', '1,2,3,4,-98,-99', 'g1,g2', 'g1,g2', False, 0, False, False, False)] + assert res==[(1, 11, 0, 'テストインデックス 11', 'Test index link 11_ja', True, True, None, '3,-99', '1,2,3,4,-98,-99', 'g1,g2,-89', 'g1,g2,-89', False, 0, False, False, False)] res = Indexes.get_recursive_tree(lang="en") assert len(res) == 7 res = Indexes.get_recursive_tree(11, lang="en") - assert res==[(1, 11, 0, 'Test index 11', 'Test index link 11_en', True, True, None, '3,-99', '1,2,3,4,-98,-99', 'g1,g2', 'g1,g2', False, 0, False, False, False)] + assert res==[(1, 11, 0, 'Test index 11', 'Test index link 11_en', True, True, None, '3,-99', '1,2,3,4,-98,-99', 'g1,g2,-89', 'g1,g2,-89', False, 0, False, False, False)] res = Indexes.get_recursive_tree(with_deleted=True) assert len(res)==11 res = Indexes.get_recursive_tree(32) assert res==[] res = Indexes.get_recursive_tree(32, with_deleted=True) - assert res==[(3, 32, 1, 'テストインデックス 32', 'Test index link 32_ja', True, True, None, '3,-99', '1,2,3,4,-98,-99', 'g1,g2', 'g1,g2', False, 1, False, False, True)] + assert res==[(3, 32, 1, 'テストインデックス 32', 'Test index link 32_ja', True, True, None, '3,-99', '1,2,3,4,-98,-99', 'g1,g2,-89', 'g1,g2,-89', False, 1, False, False, True)] # get_index_with_role res = Indexes.get_index_with_role(1) - assert res=={'biblio_flag': False, 'browsing_group': {'allow': [],'deny': []}, 'browsing_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 4, 'name': 'Community Administrator'}, {'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}, {'id': -98, 'name': 'Authenticated User'}]}, 'comment': '', 'contribute_group': {'allow': [], 'deny': []}, 'contribute_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': 4, 'name': 'Community Administrator'}, {'id': -98, 'name': 'Authenticated User'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}]}, 'coverpage_state': True, 'display_format': '1', 'display_no': 0, 'harvest_public_state': True, 'harvest_spec': '', 'id': 1, 'image_name': '', 'index_link_enabled': True, 'index_link_name': 'Test index link 1_ja', 'index_link_name_english': 'Test index link 1_en', 'index_name': 'テストインデックス 1', 'index_name_english': 'Test index 1', 'is_deleted': False, 'more_check': False, 'online_issn': '1234-5678', 'owner_user_id': 0, 'parent': 0, 'position': 0, 'public_date': '20220101', 'public_state': True, 'recursive_browsing_group': True, 'recursive_browsing_role': True, 'recursive_contribute_group': True, 'recursive_contribute_role': True, 'recursive_coverpage_check': True, 'recursive_public_state': False, 'rss_status': False, 'cnri': '', 'index_url': ''} + assert res=={'biblio_flag': False, 'browsing_group': {'allow': [{'id': '-89', 'name': 'No Group'}],'deny': []}, 'browsing_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 4, 'name': 'Community Administrator'}, {'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}, {'id': -98, 'name': 'Authenticated User'}]}, 'comment': '', 'contribute_group': {'allow': [{'id': '-89', 'name': 'No Group'}], 'deny': []}, 'contribute_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': 4, 'name': 'Community Administrator'}, {'id': -98, 'name': 'Authenticated User'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}]}, 'coverpage_state': True, 'display_format': '1', 'display_no': 0, 'harvest_public_state': True, 'harvest_spec': '', 'id': 1, 'image_name': '', 'index_link_enabled': True, 'index_link_name': 'Test index link 1_ja', 'index_link_name_english': 'Test index link 1_en', 'index_name': 'テストインデックス 1', 'index_name_english': 'Test index 1', 'is_deleted': False, 'more_check': False, 'online_issn': '1234-5678', 'owner_user_id': 0, 'parent': 0, 'position': 0, 'public_date': '20220101', 'public_state': True, 'recursive_browsing_group': True, 'recursive_browsing_role': True, 'recursive_contribute_group': True, 'recursive_contribute_role': True, 'recursive_coverpage_check': True, 'recursive_public_state': False, 'rss_status': False, 'cnri': '', 'index_url': ''} res = Indexes.get_index_with_role(22) - assert res=={'biblio_flag': True, 'browsing_group': {'allow': [], 'deny': []}, 'browsing_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 4, 'name': 'Community Administrator'}, {'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}, {'id': -98, 'name': 'Authenticated User'}]}, 'comment': '', 'contribute_group': {'allow': [], 'deny': []}, 'contribute_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': 4, 'name': 'Community Administrator'}, {'id': -98, 'name': 'Authenticated User'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}]}, 'coverpage_state': False, 'display_format': '1', 'display_no': 1, 'harvest_public_state': True, 'harvest_spec': '', 'id': 22, 'image_name': '', 'index_link_enabled': True, 'index_link_name': 'Test index link 22_ja', 'index_link_name_english': 'Test index link 22_en', 'index_name': 'テストインデックス 22', 'index_name_english': 'Test index 22', 'is_deleted': False, 'more_check': False, 'online_issn': '', 'owner_user_id': 0, 'parent': 2, 'position': 1, 'public_date': '', 'public_state': True, 'recursive_browsing_group': False, 'recursive_browsing_role': False, 'recursive_contribute_group': False, 'recursive_contribute_role': False, 'recursive_coverpage_check': False, 'recursive_public_state': True, 'rss_status': False, 'cnri': '', 'index_url': ''} + assert res=={'biblio_flag': True, 'browsing_group': {'allow': [{'id': '-89', 'name': 'No Group'}], 'deny': []}, 'browsing_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 4, 'name': 'Community Administrator'}, {'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}, {'id': -98, 'name': 'Authenticated User'}]}, 'comment': '', 'contribute_group': {'allow': [{'id': '-89', 'name': 'No Group'}], 'deny': []}, 'contribute_role': {'allow': [{'id': 3, 'name': 'Contributor'}, {'id': 4, 'name': 'Community Administrator'}, {'id': -98, 'name': 'Authenticated User'}, {'id': -99, 'name': 'Guest'}], 'deny': [{'id': 5, 'name': 'General'}, {'id': 6, 'name': 'Original Role'}]}, 'coverpage_state': False, 'display_format': '1', 'display_no': 1, 'harvest_public_state': True, 'harvest_spec': '', 'id': 22, 'image_name': '', 'index_link_enabled': True, 'index_link_name': 'Test index link 22_ja', 'index_link_name_english': 'Test index link 22_en', 'index_name': 'テストインデックス 22', 'index_name_english': 'Test index 22', 'is_deleted': False, 'more_check': False, 'online_issn': '', 'owner_user_id': 0, 'parent': 2, 'position': 1, 'public_date': '', 'public_state': True, 'recursive_browsing_group': False, 'recursive_browsing_role': False, 'recursive_contribute_group': False, 'recursive_contribute_role': False, 'recursive_coverpage_check': False, 'recursive_public_state': True, 'rss_status': False, 'cnri': '', 'index_url': ''} with patch("weko_index_tree.api.Indexes.get_account_role", return_value=[]): res = Indexes.get_index_with_role(1) @@ -856,7 +857,7 @@ def test_indexes_get_index_tree(i18n_app, db, redis_connect, users, db_records, with patch("weko_groups.models.Group.query") as mock_query: mock_query.all.return_value = [Group(id="g1", name="g1"), Group(id="g2", name="g2"), Group(id="g3", name="g3")] res = Indexes.get_index_with_role(1) - assert res["browsing_group"]==res["contribute_group"]=={'allow': [{'id': "g1", 'name': 'g1'}, {'id': "g2", 'name': 'g2'}], 'deny': [{'id': "g3", 'name': 'g3'}]} + assert res["browsing_group"]==res["contribute_group"]=={'allow': [{'id': "g1", 'name': 'g1'}, {'id': "g2", 'name': 'g2'}, {'id': "-89", 'name': 'No Group'}], 'deny': [{'id': "g3", 'name': 'g3'}]} # get_index res = Indexes.get_index(2) @@ -904,34 +905,34 @@ def test_indexes_get_index_tree(i18n_app, db, redis_connect, users, db_records, # get_path_list res = Indexes.get_path_list([3]) - assert res==[(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_path_list([32]) assert res==[] res = Indexes.get_path_list([32], with_deleted=True) - assert res==[(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2', True, True)] + assert res==[(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2,-89', True, True)] res = Indexes.get_path_list([""]) assert res==[] # get_path_name res = Indexes.get_path_name([3]) - assert res==[(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_path_name([32]) assert res==[] res = Indexes.get_path_name([32], with_deleted=True) - assert res==[(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2', True, True)] + assert res==[(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2,-89', True, True)] # get_self_list res = Indexes.get_self_list(3) - assert res==[(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_self_list(1, "comm1") - assert res==[(0, 1, '1', 'テストインデックス 1', 'Test index 1', 1, True, datetime(2022, 1, 1, 0, 0), '', '3,-99', 'g1,g2', True, False),(1, 11, '1/11', 'テストインデックス 1-/-テストインデックス 11', 'Test index 1-/-Test index 11', 2, True, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(0, 1, '1', 'テストインデックス 1', 'Test index 1', 1, True, datetime(2022, 1, 1, 0, 0), '', '3,-99', 'g1,g2,-89', True, False),(1, 11, '1/11', 'テストインデックス 1-/-テストインデックス 11', 'Test index 1-/-Test index 11', 2, True, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_self_list(2, "comm1") - assert res==[(2, 21, '2/21', 'テストインデックス 2-/-テストインデックス 21', 'Test index 2-/-Test index 21', 2, True, None, '', '3,-99', 'g1,g2', True, False), (2, 22, '2/22', 'テストインデックス 2-/-テストインデックス 22', 'Test index 2-/-Test index 22', 2, True, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(2, 21, '2/21', 'テストインデックス 2-/-テストインデックス 21', 'Test index 2-/-Test index 21', 2, True, None, '', '3,-99', 'g1,g2,-89', True, False), (2, 22, '2/22', 'テストインデックス 2-/-テストインデックス 22', 'Test index 2-/-Test index 22', 2, True, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_self_list(0, "comm1") assert res==[] @@ -941,21 +942,21 @@ def test_indexes_get_index_tree(i18n_app, db, redis_connect, users, db_records, with patch("flask_login.utils._get_user", return_value=users[3]['obj']): res = Indexes.get_self_list(31) - assert res==[(3, 31, '3/31', 'テストインデックス 3-/-テストインデックス 31', 'Test index 3-/-Test index 31', 2, False, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(3, 31, '3/31', 'テストインデックス 3-/-テストインデックス 31', 'Test index 3-/-Test index 31', 2, False, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_self_list(32) assert res==[] res = Indexes.get_self_list(32, with_deleted=True) - assert res==[(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2', True, True)] + assert res==[(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2,-89', True, True)] # get_self_path res = Indexes.get_self_path(3) - assert res==(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2', True, False) + assert res==(0, 3, '3', 'テストインデックス 3', 'Test index 3', 1, True, None, '', '3,-99', 'g1,g2,-89', True, False) res = Indexes.get_self_path(32) assert res==None res = Indexes.get_self_path(32, with_deleted=True) - assert res==(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2', True, True) + assert res==(3, 32, '3/32', 'テストインデックス 3-/-テストインデックス 32', 'Test index 3-/-Test index 32', 2, True, None, '', '3,-99', 'g1,g2,-89', True, True) # get_child_list_recursive res = Indexes.get_child_list_recursive(3) @@ -1092,7 +1093,7 @@ def test_indexes_get_index_tree(i18n_app, db, redis_connect, users, db_records, # get_child_list res = Indexes.get_child_list(1) - assert res==[(0, 1, '1', 'テストインデックス 1', 'Test index 1', 1, True, datetime(2022, 1, 1, 0, 0), '', '3,-99', 'g1,g2', True, False),(1, 11, '1/11', 'テストインデックス 1-/-テストインデックス 11', 'Test index 1-/-Test index 11', 2, True, None, '', '3,-99', 'g1,g2', True, False)] + assert res==[(0, 1, '1', 'テストインデックス 1', 'Test index 1', 1, True, datetime(2022, 1, 1, 0, 0), '', '3,-99', 'g1,g2,-89', True, False),(1, 11, '1/11', 'テストインデックス 1-/-テストインデックス 11', 'Test index 1-/-Test index 11', 2, True, None, '', '3,-99', 'g1,g2,-89', True, False)] res = Indexes.get_child_list(3) assert len(res)==1 with patch("flask_login.utils._get_user", return_value=users[3]['obj']): @@ -1208,17 +1209,17 @@ def test_get_index_with_role_group(app, db, mocker): # 結果の検証 assert result['browsing_group']['allow'] == [] - assert result['browsing_group']['deny'] == [{'id': '8gr', 'name': 'group_value_role'}] + assert result['browsing_group']['deny'] == [{'id': '-89', 'name': 'No Group'}, {'id': '8gr', 'name': 'group_value_role'}] assert result['contribute_group']['allow'] == [] - assert result['contribute_group']['deny'] == [{'id': '8gr', 'name': 'group_value_role'}] + assert result['contribute_group']['deny'] == [{'id': '-89', 'name': 'No Group'}, {'id': '8gr', 'name': 'group_value_role'}] # 結果が空の場合のテストケース mocker.patch.object(Indexes, 'get_account_role', return_value=[]) result = Indexes.get_index_with_role(1) assert result['browsing_group']['allow'] == [] - assert result['browsing_group']['deny'] == [] + assert result['browsing_group']['deny'] == [{'id': '-89', 'name': 'No Group'}] assert result['contribute_group']['allow'] == [] - assert result['contribute_group']['deny'] == [] + assert result['contribute_group']['deny'] == [{'id': '-89', 'name': 'No Group'}] # .tox/c1/bin/pytest --cov=weko_index_tree tests/test_api.py::test_indexes_get_handle_index_url -v -s -vv --cov-branch --cov-report=html --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp @@ -1288,3 +1289,18 @@ def test_get_allow_deny(app, db, mocker): result = Indexes.get_index_with_role(1) assert result['browsing_role']['allow'] == [] assert result['browsing_role']['deny'] == [] + + +# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_api.py::test_get_account_group -v -s -vv --cov-branch --cov-report=html --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp +def test_get_account_group(app, mocker): + mock_group_obj = SimpleNamespace( + id=1, name='TestGroup', _underbar='under', no_text=None, not_str=[1]) + mock_query = mocker.patch('weko_groups.api.Group.query') + mock_query.all.return_value = [mock_group_obj] + groups = Indexes.get_account_group() + assert [{'id': 1, 'name': 'TestGroup', 'no_text': ''}, + {'id': -89, 'name': 'No Group'}] == groups + + mock_query.all.side_effect = SQLAlchemyError("DB error") + result = Indexes.get_account_group() + assert result is None diff --git a/modules/weko-index-tree/tests/test_utils.py b/modules/weko-index-tree/tests/test_utils.py index 40dc39d381..042e3c6eda 100644 --- a/modules/weko-index-tree/tests/test_utils.py +++ b/modules/weko-index-tree/tests/test_utils.py @@ -8,6 +8,7 @@ get_user_groups, check_roles, check_groups, + check_index_permission_by_role_and_group, filter_index_list_by_role, get_index_id_list, get_publish_index_id_list, @@ -200,7 +201,6 @@ def test_get_user_roles(i18n_app, client_rest, users): assert result[1] == None - #+++ def get_user_groups(): def test_get_user_groups(i18n_app, client_rest, users, db): with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): @@ -215,132 +215,82 @@ def test_get_user_groups(i18n_app, client_rest, users, db): # User not authenticated assert len(get_user_groups()) == 0 -# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_check_roles -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp -#+++ def check_roles(user_role, roles): -def test_check_roles(i18n_app, users): - # Match both role and role group - params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} - user_role = (False, ["10"]) - roles = ["10"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is True - # Only role group matches - params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} - user_role = (False, ["30"]) - roles = ["20"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is False +# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_check_index_permission_by_role_and_group -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp +# def check_index_permission_by_role_and_group(user_role, roles, user_group, groups): +def test_check_index_permission_by_role_and_group(app, mocker): + mock_role1 = mocker.Mock() + mock_role1.id = 1 + mock_role1.name = "role1" + mock_role2 = mocker.Mock() + mock_role2.id = 2 + mock_role2.name = "jc_groups_xxx" # role_group + mock_roles = [mock_role1, mock_role2] - # Only role matches - params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} - user_role = (False, ["30"]) - roles = ["30"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is True + mock_filter = mocker.Mock() + mock_filter.all.return_value = mock_roles + mock_query = mocker.Mock() + mock_query.filter.return_value = mock_filter + mocker.patch("weko_index_tree.utils.Role.query", mock_query) - # Neither role nor role group matches - params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} - user_role = (False, ["40"]) - roles = ["50"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is False + # Admin User + check_index_permission_by_role_and_group((True, []), '', [], '') is True - # Both not set (group check) - params = {"role_groups": [], "groups": [100], "access_group_ids": [200]} - user_role = (False, ["40"]) - roles = [] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is False - - # Admin user - user_role = (True, ["1"]) - roles = ["1", "2"] - params = {"role_groups": [10], "groups": [100], "access_group_ids": [200]} - assert check_roles(user_role, roles, params) is True - - # Guest user (role -99) - params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} - user_role = (False, []) - roles = ["-99"] - with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=False)): - assert check_roles(user_role, roles, params) is True - - # Role group is [10], roles are [10, 30], user_role is (not admin, [10, 30]) - params = {"role_groups": [10], "groups": [100], "access_group_ids": [200]} - user_role = (False, ["10", "30"]) - roles = ["10", "30"] - with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=True)): - assert check_roles(user_role, roles, params) is True - - # Role group is [10, 20], roles are [30], user_role is (not admin, [30]) - params = {"role_groups": [10, 20], "groups": [100], "access_group_ids": [200]} - user_role = (False, []) - roles = ["-99", "10"] - with patch("flask_login.utils._get_user", return_value=MagicMock(is_authenticated=False)): - assert check_roles(user_role, roles, params) is False - - # User has only the role set for the index - params = {"role_groups": [100, 200], "groups": [300], "access_group_ids": [400]} - user_role = (False, ["1"]) - roles = ["1", "100"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is False + mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles", return_value=True) + mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups", return_value=True) + # mock query returns [1,2] → role id:3 is removed + # role id:2 is role_group → called in check_groups + check_index_permission_by_role_and_group((False, [1,2,3]), '1,2,3', [5,6], '5,6') is False + mock_check_roles.assert_called_with(['1'], ['1']) + mock_check_groups.assert_called_with(['5', '6'], ['5', '6'], ['2'], ['2']) - # User has only the role group set for the index - params = {"role_groups": [100, 200], "groups": [300], "access_group_ids": [400]} - user_role = (False, ["100"]) - roles = ["1", "100"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is False - # User has neither the role nor the role group set for the index - params = {"role_groups": [100, 200], "groups": [300], "access_group_ids": [400]} - user_role = (False, ["999"]) - roles = ["1", "100"] - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_roles(user_role, roles, params) is False - - -# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_set_params -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp -def test_set_params(app): - with app.app_context(): - from weko_index_tree.utils import set_params - class MockRole: - def __init__(self, id, name): - self.id = id - self.name = name - mock_roles = [ - MockRole(1, "group_admin"), - MockRole(2, "user"), - MockRole(3, "group_editor"), - MockRole(4, "guest") - ] - with patch("invenio_accounts.models.Role.query") as mock_query: - mock_query.all.return_value = mock_roles - groups = [100, 200] - index_group = [300] - params = set_params(groups, index_group) +# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_check_roles -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp +# def check_roles(user_role_list, index_role_list): +def test_check_roles(app, mocker): + mock_user = mocker.Mock() + mock_user.is_authenticated = True + # If logged in, the user has '-98'. + mocker.patch('flask_login.utils._get_user', return_value=mock_user) - assert params["role_groups"] == [1, 3] - assert params["groups"] == [100, 200] - assert params["access_group_ids"] == [300] + assert check_roles([], ['r1']) is False + assert check_roles([], ['-98']) is True -#+++ def check_groups(user_group, groups): -def test_check_groups(i18n_app, users, db): - g1 = Group.create(name="group_test1").add_member(users[-1]['obj']) - g2 = Group.create(name="group_test2").add_member(users[-1]['obj']) + # If not logged in, the user has '-99'. + mock_user.is_authenticated = False + mocker.patch('flask_login.utils._get_user', return_value=mock_user) + assert check_roles([], ['r1']) is False + assert check_roles([], ['-99']) is True - db.session.add(g1) - db.session.add(g2) - user_group = ["group_test1", "group_test2"] - groups = [v for k,v in Group.get_group_list().items()] +# .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_check_groups -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp +# def check_groups(user_group, index_group_list, user_role_group, index_role_group): +def test_check_groups(app, mocker): + mock_user = mocker.Mock() - with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): - assert check_groups(user_group, groups) + # If not logged in, the user has '-89'. + mock_user.is_authenticated = False + mocker.patch('flask_login.utils._get_user', return_value=mock_user) + assert check_groups([], ['g1'], [], ['rg1']) is False + assert check_groups([], ['-89', 'g1'], [], ['rg1']) is True - assert check_groups(user_group, groups) == False + # If logged in and has no groups, the user has '-89'. + mock_user.is_authenticated = True + mocker.patch('flask_login.utils._get_user', return_value=mock_user) + assert check_groups([], ['g1'], [], ['rg1']) is False + assert check_groups([], ['-89', 'g1'], [], ['rg1']) is True + + # If logged in and has groups, the user does not have '-89'. + + # group: exists + assert check_groups(['g1'], ['-89'], [], ['rg2']) is False + # role_group: exists + assert check_groups([], ['-89'], ['rg1'], ['rg2']) is False + + # group: OK, role_group: NG → OK + assert check_groups(['g1'], ['-89', 'g1'], ['rg1'], ['rg2']) is True + # group: NG, role_group: OK → OK + assert check_groups(['g1'], ['-89'], ['rg1'], ['rg1']) is True #+++ def filter_index_list_by_role(index_list): @@ -409,8 +359,8 @@ def test_reduce_index_by_role(app, db, users): "public_state": True, "public_date": datetime(2022, 1, 1, 0, 0), "browsing_role": "3,-98,-99", - "contribute_group": "", - "browsing_group": "", + "contribute_group": "group_test1,-89", + "browsing_group": "group_test1,-89", "children": [ { "id": "11", @@ -444,7 +394,18 @@ def test_reduce_index_by_role(app, db, users): "browsing_group": "group_test1", "children": [], "settings": [] - } + }, + { + "id": "14", + "contribute_role": "1,2,3,4,-98,-99", + "public_state": True, + "public_date": datetime(2022, 1, 1, 0, 0), + "browsing_role": "3,-98,-99", + "contribute_group": "", + "browsing_group": "", + "children": [], + "settings": [] + }, ], "settings": { "checked": False @@ -452,27 +413,28 @@ def test_reduce_index_by_role(app, db, users): }] new_tree = copy.deepcopy(tree) reduce_index_by_role(new_tree, admin_roles, groups, True) - assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': []}, {'id': '12', 'children': [], 'settings': []}], 'settings': {'checked': False}}] + assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': []}, {'id': '12', 'children': [], 'settings': []}, + {'id': '14', 'children': [], 'settings': []}], 'settings': {'checked': False}}] new_tree = copy.deepcopy(tree) reduce_index_by_role(new_tree, user_roles, groups, True) - assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': []}, {'id': '12', 'children': [], 'settings': []}], 'settings': {'checked': False}}] + assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': []}], 'settings': {'checked': False}}] new_tree = copy.deepcopy(tree) reduce_index_by_role(new_tree, user_roles, [], True) - assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': []}], 'settings': {'checked': False}}] + assert new_tree==[{'id': '10', 'children': [], 'settings': {'checked': False}}] new_tree = copy.deepcopy(tree) reduce_index_by_role(new_tree, admin_roles, groups, False) - assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': [], 'disabled': False}, {'id': '12', 'children': [], 'settings': [], 'disabled': False}, {'id': '13', 'children': [], 'settings': [], 'disabled': False}], 'settings': {'checked': False}, 'disabled': False}] + assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': [], 'disabled': False}, {'id': '12', 'children': [], 'settings': [], 'disabled': False}, {'id': '13', 'children': [], 'settings': [], 'disabled': False}, {'id': '14', 'children': [], 'settings': [], 'disabled': False}], 'settings': {'checked': False}, 'disabled': False}] new_tree = copy.deepcopy(tree) reduce_index_by_role(new_tree, user_roles, groups, False) - assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': [], 'disabled': False}, {'id': '12', 'children': [], 'settings': [], 'disabled': False}, {'id': '13', 'children': [], 'settings': [], 'disabled': False}], 'settings': {'checked': False}, 'disabled': False}] + assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': [], 'disabled': False}, {'id': '13', 'children': [], 'settings': [], 'disabled': False}], 'settings': {'checked': False}, 'disabled': False}] new_tree = copy.deepcopy(tree) reduce_index_by_role(new_tree, user_roles, [], False, ["10", "12"]) - assert new_tree==[{'id': '10', 'children': [{'id': '11', 'children': [], 'settings': [], 'disabled': False}, {'id': '13', 'children': [], 'settings': [], 'disabled': False}], 'settings': {'checked': True}, 'disabled': False}] + assert new_tree==[{'id': '10', 'children': [], 'settings': {'checked': True}, 'disabled': False}] #+++ def get_index_id_list(indexes, id_list=None): diff --git a/modules/weko-index-tree/weko_index_tree/api.py b/modules/weko-index-tree/weko_index_tree/api.py index f43fc9a977..e34823d679 100644 --- a/modules/weko-index-tree/weko_index_tree/api.py +++ b/modules/weko-index-tree/weko_index_tree/api.py @@ -109,12 +109,12 @@ def _add_index(data): data["recursive_coverpage_check"] = False group_list = "" - groups = Group.query.all() + groups = cls.get_account_group() for group in groups: if not group_list: - group_list = str(group.id) + group_list = str(group["id"]) else: - group_list = group_list + "," + str(group.id) + group_list = group_list + "," + str(group["id"]) data["browsing_group"] = group_list data["contribute_group"] = group_list @@ -901,11 +901,10 @@ def _get_group_allow_deny(allow_group_id=[], groups=[]): if not groups: return allow, deny for group in groups: - if str(group.id) in allow_group_id: - allow.append({'id': str(group.id), 'name': group.name}) + if str(group["id"]) in allow_group_id: + allow.append({'id': str(group["id"]), 'name': group["name"]}) else: - deny.append({'id': str(group.id), 'name': group.name}) - + deny.append({'id': str(group["id"]), 'name': group["name"]}) return allow, deny def _get_filter_gakunin_map_groups_allow_deny(filtered_role_ids=[], filtered_roles=[]): @@ -949,7 +948,7 @@ def _get_filter_gakunin_map_groups_allow_deny(filtered_role_ids=[], filtered_rol if index["public_date"]: index["public_date"] = index["public_date"].strftime('%Y%m%d') - group_list = Group.query.all() + group_list = cls.get_account_group() #browsing_groupとcontribute_groupの値を取得して、browsing_groupとcontribute_groupの辞書の値を保持します。 allow_browsing_group_ids = index["browsing_group"].split(',') \ if len(index["browsing_group"]) else [] @@ -1075,6 +1074,27 @@ def _get_dict(x): except SQLAlchemyError: return + @classmethod + def get_account_group(cls): + """Get account group.""" + def _get_dict(x): + dt = dict() + for k, v in x.__dict__.items(): + if not k.startswith('_') and "description" not in k: + if not v: + v = "" + if isinstance(v, int) or isinstance(v, str): + dt[k] = v + return dt + + try: + with db.session.no_autoflush: + group = Group.query.all() + return list(map(_get_dict, group)) \ + + [{"id": -89, "name": "No Group"}] + except SQLAlchemyError: + return + @classmethod def get_path_list(cls, node_lst, with_deleted=False): """ diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index e5b9cf523a..eb581a0c37 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -33,6 +33,8 @@ from flask_babelex import gettext as _ from flask_babelex import to_user_timezone, to_utc from flask_login import current_user +from sqlalchemy import and_ +from invenio_accounts.models import Role from invenio_cache import current_cache from invenio_communities.models import Community from invenio_db import db @@ -308,78 +310,62 @@ def get_user_groups(): grps.append(group.id) return grps -def check_roles(user_role, roles, params): - """ - Check whether the user has access to an index based on roles and role groups. +def check_index_permission_by_role_and_group(user_role, roles, user_group, groups): - This function determines if a user can access an index by evaluating: - - Whether the user is an administrator (always True) - - Whether the user's roles or role groups match those set for the index - - Whether guest access is allowed (when '-99' is set and no role groups are configured) - - If neither roles nor role groups are set for the index, internal group check is performed + if user_role[0]: + return True - Args: - user_role (tuple): (is_admin, [role_id, ...]) where is_admin is a bool. - roles (str or list): List or comma-separated string of role IDs set for the index. - params (dict): Dictionary containing: - - "role_groups": list of role group IDs set for the index - - "groups": list of group IDs the user belongs to - - "access_group_ids": group IDs set for the index + user_roles = [str(r) for r in user_role[1]] if user_role[1] else [] + user_group_list = [str(g) for g in user_group] - Returns: - bool: True if the user has access to the index, False otherwise. - """ + role_key = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["role_keyword"] + prefix = current_app.config["WEKO_ACCOUNTS_GAKUNIN_GROUP_PATTERN_DICT"]["prefix"] + without_map_role = Role.query.filter( + ~and_(Role.name.like(f"%{role_key}%"), Role.name.startswith(prefix)) + ).all() - is_can = False - group_perm = False - role_perm = False - - if not user_role[0]: - roles_list = sorted(roles if isinstance(roles, list) else (roles.split(',') if roles else [])) - set_index_group = any(int(r) in params["role_groups"] for r in roles_list) - set_index_role = any(int(r) not in params["role_groups"] for r in roles_list) - if current_user.is_authenticated: - self_role = user_role[1] or [] - self_role.append('-98') - if set_index_group: - index_role_group = [int(r) for r in roles_list if int(r) in params["role_groups"]] - # Groups(role) are set for the index. - group_perm = any(str(rg) in [str(sr) for sr in self_role] for rg in index_role_group) - if set_index_role: - # If the index has both role groups and roles. - index_role = [int(r) for r in roles_list if int(r) not in params["role_groups"]] - role_perm = any(str(role) in [str(r) for r in index_role] for role in self_role) - is_can = group_perm and role_perm - else: - # If the index has role groups but no roles. - is_can = group_perm - elif set_index_role: - # Roles are set for the index, but no role groups are configured. - for role in self_role: - if str(role) in roles_list: - is_can = True - break - else: - # No roles or role groups are set for the index, so proceed to internal group check. - is_can = check_groups(params["groups"], params["access_group_ids"]) + role_groups = [str(lst.id) for lst in without_map_role if "_groups_" in lst.name] + without_map_role = [str(r.id) for r in without_map_role] + # Guest, Authenticated User + without_map_role.extend(['-98', '-99']) - # Guest users can view when '-99' is set and no role groups are configured. - elif roles and "-99" in roles and not set_index_group: - is_can = True - # Administrators always have access. - else: - is_can = True - return is_can + user_role_list = [r for r in user_roles + if r in without_map_role and r not in role_groups] + user_role_group = [r for r in user_roles + if r in without_map_role and r in role_groups] + + role_list = sorted(roles if isinstance(roles, list) + else (roles.split(',') if roles else [])) + index_group_list = sorted(groups if isinstance(groups, list) + else (groups.split(',') if groups else [])) + index_role_list = [r for r in role_list + if r in without_map_role and r not in role_groups] + index_role_group = [r for r in role_list + if r in without_map_role and r in role_groups] -def check_groups(user_group, groups): - """Check groups.""" - is_can = False + return check_roles(user_role_list, index_role_list) and \ + check_groups(user_group_list, index_group_list, user_role_group, index_role_group) + + +def check_roles(user_role_list, index_role_list): if current_user.is_authenticated: - group = [x for x in user_group if str(x) in (groups or [])] - if group: - is_can = True - return is_can + user_role_list.append('-98') + else: + user_role_list.append('-99') + return any(r in index_role_list for r in user_role_list) + + +def check_groups(user_group, index_group_list, user_role_group, index_role_group): + if current_user.is_authenticated and not user_group and not user_role_group: + user_group.append('-89') + elif not current_user.is_authenticated: + user_group.append('-89') + + group_perm = any(r in user_group for r in index_group_list) + role_group_perm = any(r in user_role_group for r in index_role_group) + return group_perm or role_group_perm + def filter_index_list_by_role(index_list): """Filter index list by role.""" @@ -390,8 +376,9 @@ def _check(index_data, roles, groups): if roles[0]: can_view = True else: - params = set_params(groups, index_data.browsing_group) - if check_roles(roles, index_data.browsing_role, params): + if check_index_permission_by_role_and_group( + roles, index_data.browsing_role, + groups, index_data.browsing_group): if index_data.public_state \ and (index_data.public_date is None or not is_future(index_data.public_date)): @@ -406,36 +393,6 @@ def _check(index_data, roles, groups): result_list.append(i) return result_list -def set_params(groups, index_group): - """ - Generate a parameter dictionary for index permission checks. - - This function collects all role group IDs from the Role table whose name contains 'group', - and combines them with the specified user groups and index groups to create a parameter - dictionary for permission checking functions. - - Args: - groups (list): List of group IDs the user belongs to. - index_group (list): List of group IDs set for the index. - - Returns: - dict: Dictionary containing: - - "role_groups": list of all role group IDs in the system - - "groups": list of group IDs the user belongs to - - "access_group_ids": list of group IDs set for the index - """ - from invenio_accounts.models import Role - - role_groups = [] - for lst in Role.query.all(): - if "group" in lst.name.lower(): - role_groups.append(lst.id) - params = { - "role_groups": role_groups, - "groups": groups, - "access_group_ids": index_group - } - return params def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): """Reduce index by.""" @@ -464,8 +421,8 @@ def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): # browsing role and group check if browsing_role: - params = set_params(groups, brw_group) - if check_roles(roles, brw_role, params): + if check_index_permission_by_role_and_group( + roles, brw_role, groups, brw_group): if public_state and \ (public_date is None or not is_future(public_date)): @@ -479,8 +436,8 @@ def reduce_index_by_role(tree, roles, groups, browsing_role=True, plst=None): tree.pop(i) # contribute role and group check else: - params = set_params(groups, contribute_group) - if check_roles(roles, contribute_role, params): + if check_index_permission_by_role_and_group( + roles, contribute_role, groups, contribute_group): lst['disabled'] = False plst = plst or [] @@ -872,9 +829,10 @@ def _check_index_permission(index_data) -> bool: in_admin_view_scope = _check_community_admin_permission(index_data) is_content_public = False - params = set_params(groups, index_data.browsing_group) if index_data.public_state: - check_user_role = check_roles(roles, index_data.browsing_role, params) + check_user_role = check_index_permission_by_role_and_group( + roles, index_data.browsing_role, + groups, index_data.browsing_group) check_public_date = \ not is_future(index_data.public_date) \ if index_data.public_date else True From 31c46995f5fcb5d2230396b14861d655b74f9f4a Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Wed, 12 Nov 2025 10:33:18 +0900 Subject: [PATCH 15/19] fix index unit test --- modules/weko-index-tree/tests/conftest.py | 27 ++++++++++++--------- modules/weko-index-tree/tests/test_rest.py | 4 +-- modules/weko-index-tree/tests/test_utils.py | 12 ++++----- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/modules/weko-index-tree/tests/conftest.py b/modules/weko-index-tree/tests/conftest.py index 6d868d0de1..d134ce4f5b 100644 --- a/modules/weko-index-tree/tests/conftest.py +++ b/modules/weko-index-tree/tests/conftest.py @@ -714,6 +714,7 @@ def indices(app, db): testIndexOne = Index( index_name="testIndexOne", browsing_role="1,2,3,4,-98,-99", + browsing_group="-89", public_state=True, id=11, position=0 @@ -744,6 +745,7 @@ def indices(app, db): testIndexThreeChild = Index( index_name="testIndexThreeChild", browsing_role="1,2,3,4,-98,-99", + browsing_group="-89", parent=33, index_link_enabled=True, index_link_name="test_link", @@ -763,6 +765,7 @@ def indices(app, db): testIndexSix = Index( index_name="testIndexSix", browsing_role="1,2,3,4,-98,-99", + browsing_group="-89", public_state=True, id=66, position=4 @@ -857,8 +860,8 @@ def indices_for_api(app, db): index_name_english="Sample Index", browsing_role="3,-98,-99", contribute_role="1,2,3,4,-98", - browsing_group="", - contribute_group="", + browsing_group="-89", + contribute_group="-89", public_state=False, harvest_public_state=False, owner_user_id=1, @@ -874,8 +877,8 @@ def indices_for_api(app, db): index_name_english="parent index", browsing_role="3,4,-98,-99", contribute_role="3,4,-98,-99", - browsing_group="", - contribute_group="", + browsing_group="-89", + contribute_group="-89", public_state=True, harvest_public_state=True, owner_user_id=1, @@ -891,8 +894,8 @@ def indices_for_api(app, db): index_name_english="child index 1", browsing_role="3,4,-98,-99", contribute_role="3,4,-98,-99", - browsing_group="", - contribute_group="", + browsing_group="-89", + contribute_group="-89", public_state=False, harvest_public_state=True, owner_user_id=1, @@ -908,8 +911,8 @@ def indices_for_api(app, db): index_name_english="child index 2", browsing_role="3,4,-98,-99", contribute_role="3,4,-98,-99", - browsing_group="", - contribute_group="", + browsing_group="-89", + contribute_group="-89", public_state=True, harvest_public_state=True, owner_user_id=1, @@ -925,8 +928,8 @@ def indices_for_api(app, db): index_name_english="child index 3", browsing_role="3,4,-98,-99", contribute_role="3,4,-98,-99", - browsing_group="", - contribute_group="", + browsing_group="-89", + contribute_group="-89", public_state=False, harvest_public_state=True, owner_user_id=1, @@ -959,8 +962,8 @@ def indices_for_api(app, db): index_name_english="Community Child Index", browsing_role="3,4,-98,-99", contribute_role="3,4,-98,-99", - browsing_group="", - contribute_group="", + browsing_group="-89", + contribute_group="-89", public_state=True, harvest_public_state=True, owner_user_id=1, diff --git a/modules/weko-index-tree/tests/test_rest.py b/modules/weko-index-tree/tests/test_rest.py index 25a19010d1..a452e2dea4 100644 --- a/modules/weko-index-tree/tests/test_rest.py +++ b/modules/weko-index-tree/tests/test_rest.py @@ -1004,8 +1004,8 @@ def create_index_with_default_values(self, client_rest, auth_headers): assert created_index_db.rss_status is False assert set(created_index_db.browsing_role.split(",")) == set(map(lambda x: str(x["id"]), Indexes.get_account_role())) assert set(created_index_db.browsing_role.split(",")) == set(map(lambda x: str(x["id"]), Indexes.get_account_role())) - assert created_index_db.browsing_group == "" - assert created_index_db.contribute_group == "" + assert created_index_db.browsing_group == "-89" + assert created_index_db.contribute_group == "-89" assert created_index_db.online_issn == "" assert self.count_indices == count_before + 1, "Index has not been created successfully" diff --git a/modules/weko-index-tree/tests/test_utils.py b/modules/weko-index-tree/tests/test_utils.py index 042e3c6eda..28646e8bbf 100644 --- a/modules/weko-index-tree/tests/test_utils.py +++ b/modules/weko-index-tree/tests/test_utils.py @@ -304,9 +304,8 @@ def __init__(self): self.public_state = True self.public_date = None dummy1 = DummyIndex1() - with patch("weko_index_tree.utils.set_params", return_value={"role_groups": [], "groups": [], "access_group_ids": []}): - with patch("weko_index_tree.utils.check_roles", return_value=False): - assert filter_index_list_by_role([dummy1]) == [] + with patch("weko_index_tree.utils.check_roles", return_value=False): + assert filter_index_list_by_role([dummy1]) == [] # Case 2: public_state is False class DummyIndex2: @@ -316,10 +315,9 @@ def __init__(self): self.public_state = False self.public_date = None dummy2 = DummyIndex2() - with patch("weko_index_tree.utils.set_params", return_value={"role_groups": [], "groups": [], "access_group_ids": []}): - with patch("weko_index_tree.utils.check_roles", return_value=True): - with patch("weko_records_ui.utils.is_future", return_value=False): - assert filter_index_list_by_role([dummy2]) == [] + with patch("weko_index_tree.utils.check_roles", return_value=True): + with patch("weko_records_ui.utils.is_future", return_value=False): + assert filter_index_list_by_role([dummy2]) == [] # Case 3: normal access (user logged in and has group membership) with patch("flask_login.utils._get_user", return_value=users[-1]['obj']): From 7105390e92e8be5c3ab0059d0c4dcfd30e8d3d66 Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Wed, 12 Nov 2025 15:16:34 +0900 Subject: [PATCH 16/19] fix index unit test --- modules/weko-index-tree/tests/test_utils.py | 23 +++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/modules/weko-index-tree/tests/test_utils.py b/modules/weko-index-tree/tests/test_utils.py index 28646e8bbf..f70278eff3 100644 --- a/modules/weko-index-tree/tests/test_utils.py +++ b/modules/weko-index-tree/tests/test_utils.py @@ -234,16 +234,31 @@ def test_check_index_permission_by_role_and_group(app, mocker): mocker.patch("weko_index_tree.utils.Role.query", mock_query) # Admin User - check_index_permission_by_role_and_group((True, []), '', [], '') is True + assert check_index_permission_by_role_and_group((True, []), '', [], '') is True - mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles", return_value=True) - mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups", return_value=True) + mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles") + mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups") # mock query returns [1,2] → role id:3 is removed # role id:2 is role_group → called in check_groups - check_index_permission_by_role_and_group((False, [1,2,3]), '1,2,3', [5,6], '5,6') is False + check_index_permission_by_role_and_group((False, [1,2,3]), '1,2,3', [5,6], '5,6') mock_check_roles.assert_called_with(['1'], ['1']) mock_check_groups.assert_called_with(['5', '6'], ['5', '6'], ['2'], ['2']) + mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles", return_value=True) + mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups", return_value=True) + assert check_index_permission_by_role_and_group((False, []), '', [], '') is True + + mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles", return_value=True) + mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups", return_value=False) + assert check_index_permission_by_role_and_group((False, []), '', [], '') is False + + mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles", return_value=False) + mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups", return_value=True) + assert check_index_permission_by_role_and_group((False, []), '', [], '') is False + + mock_check_roles = mocker.patch("weko_index_tree.utils.check_roles", return_value=False) + mock_check_groups = mocker.patch("weko_index_tree.utils.check_groups", return_value=False) + assert check_index_permission_by_role_and_group((False, []), '', [], '') is False # .tox/c1/bin/pytest --cov=weko_index_tree tests/test_utils.py::test_check_roles -v -s -vv --cov-branch --cov-report=term --cov-config=tox.ini --basetemp=/code/modules/weko-index-tree/.tox/c1/tmp # def check_roles(user_role_list, index_role_list): From cc84777efe37cb29185d61a48eb18f67a037678c Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Thu, 13 Nov 2025 19:16:44 +0900 Subject: [PATCH 17/19] add comment --- .../weko-index-tree/weko_index_tree/api.py | 8 ++- .../weko-index-tree/weko_index_tree/utils.py | 66 +++++++++++++++++-- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/modules/weko-index-tree/weko_index_tree/api.py b/modules/weko-index-tree/weko_index_tree/api.py index e34823d679..634b7c07c9 100644 --- a/modules/weko-index-tree/weko_index_tree/api.py +++ b/modules/weko-index-tree/weko_index_tree/api.py @@ -1076,7 +1076,13 @@ def _get_dict(x): @classmethod def get_account_group(cls): - """Get account group.""" + """ + Retrieve the list of groups to which the current user belongs. + + Returns: + list: A list of dictionaries, each representing a group with its attributes. + The list also includes a dictionary for the "No Group" (id: -89). + """ def _get_dict(x): dt = dict() for k, v in x.__dict__.items(): diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index eb581a0c37..76c75a1229 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -311,7 +311,23 @@ def get_user_groups(): return grps def check_index_permission_by_role_and_group(user_role, roles, user_group, groups): + """ + Check if a user has permission to access an index based on roles and groups. + + Args: + user_role (tuple): A tuple where the first element is a boolean indicating + if the user has administrator privileges, and the second element is a list + of role IDs assigned to the user. + roles (list or str): A list of role IDs (or a comma-separated string of IDs) + that are permitted to access the index. + user_group (list): A list of internal group IDs to which the user belongs. + groups (list or str): A list of internal group IDs (or a comma-separated string of IDs) + that are permitted to access the index. + Returns: + bool: True if the user has permission to access the index, False otherwise. + """ + # Administrator users are always granted access. if user_role[0]: return True @@ -324,7 +340,8 @@ def check_index_permission_by_role_and_group(user_role, roles, user_group, group ~and_(Role.name.like(f"%{role_key}%"), Role.name.startswith(prefix)) ).all() - role_groups = [str(lst.id) for lst in without_map_role if "_groups_" in lst.name] + role_groups = [str(lst.id) for lst in without_map_role + if "_groups_" in lst.name] without_map_role = [str(r.id) for r in without_map_role] # Guest, Authenticated User without_map_role.extend(['-98', '-99']) @@ -345,24 +362,59 @@ def check_index_permission_by_role_and_group(user_role, roles, user_group, group if r in without_map_role and r in role_groups] return check_roles(user_role_list, index_role_list) and \ - check_groups(user_group_list, index_group_list, user_role_group, index_role_group) + check_groups(user_group_list, index_group_list, + user_role_group, index_role_group) def check_roles(user_role_list, index_role_list): + """ + Determine whether the user has access permission based on role IDs. + + Args: + user_role_list (list): List of role IDs (as strings) assigned to the user. + index_role_list (list): List of role IDs (as strings) configured for the index. + + Returns: + bool: True if at least one role ID in user_role_list matches an ID in index_role_list, otherwise False. + """ if current_user.is_authenticated: + # Authenticated User user_role_list.append('-98') else: + # Guest user_role_list.append('-99') return any(r in index_role_list for r in user_role_list) -def check_groups(user_group, index_group_list, user_role_group, index_role_group): - if current_user.is_authenticated and not user_group and not user_role_group: - user_group.append('-89') +def check_groups(user_group_list, index_group_list, + user_role_group, index_role_group): + """ + Determine whether the user has access permission based on group and role group IDs. + + Args: + user_group (list): List of internal group IDs (as strings) to which the user belongs. + index_group_list (list): List of internal group IDs (as strings) configured for the index. + user_role_group (list): List of role group IDs (as strings) assigned to the user. + index_role_group (list): List of role group IDs (as strings) configured for the index. + + Returns: + bool: True if at least one group ID in user_group matches index_group_list, + or at least one role group ID in user_role_group matches index_role_group. + Otherwise, returns False. + + Notes: + - If the user is authenticated and both user_group and user_role_group are empty, + '-89' (Authenticated User Group) is appended to user_group. + - If the user is not authenticated, '-89' (Guest Group) is appended to user_group. + - Access is granted if any group or role group ID matches between the user and the index. + """ + # append "No Group" + if current_user.is_authenticated and not user_group_list and not user_role_group: + user_group_list.append('-89') elif not current_user.is_authenticated: - user_group.append('-89') + user_group_list.append('-89') - group_perm = any(r in user_group for r in index_group_list) + group_perm = any(r in user_group_list for r in index_group_list) role_group_perm = any(r in user_role_group for r in index_role_group) return group_perm or role_group_perm From 213e1edb08782bee732b86d55c34240bc9758867 Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Thu, 13 Nov 2025 19:22:46 +0900 Subject: [PATCH 18/19] fix comment --- modules/weko-index-tree/weko_index_tree/utils.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/modules/weko-index-tree/weko_index_tree/utils.py b/modules/weko-index-tree/weko_index_tree/utils.py index 76c75a1229..d941ce7505 100644 --- a/modules/weko-index-tree/weko_index_tree/utils.py +++ b/modules/weko-index-tree/weko_index_tree/utils.py @@ -320,8 +320,8 @@ def check_index_permission_by_role_and_group(user_role, roles, user_group, group of role IDs assigned to the user. roles (list or str): A list of role IDs (or a comma-separated string of IDs) that are permitted to access the index. - user_group (list): A list of internal group IDs to which the user belongs. - groups (list or str): A list of internal group IDs (or a comma-separated string of IDs) + user_group (list): A list of group IDs to which the user belongs. + groups (list or str): A list of group IDs (or a comma-separated string of IDs) that are permitted to access the index. Returns: @@ -392,8 +392,8 @@ def check_groups(user_group_list, index_group_list, Determine whether the user has access permission based on group and role group IDs. Args: - user_group (list): List of internal group IDs (as strings) to which the user belongs. - index_group_list (list): List of internal group IDs (as strings) configured for the index. + user_group (list): List of group IDs (as strings) to which the user belongs. + index_group_list (list): List of group IDs (as strings) configured for the index. user_role_group (list): List of role group IDs (as strings) assigned to the user. index_role_group (list): List of role group IDs (as strings) configured for the index. @@ -401,12 +401,6 @@ def check_groups(user_group_list, index_group_list, bool: True if at least one group ID in user_group matches index_group_list, or at least one role group ID in user_role_group matches index_role_group. Otherwise, returns False. - - Notes: - - If the user is authenticated and both user_group and user_role_group are empty, - '-89' (Authenticated User Group) is appended to user_group. - - If the user is not authenticated, '-89' (Guest Group) is appended to user_group. - - Access is granted if any group or role group ID matches between the user and the index. """ # append "No Group" if current_user.is_authenticated and not user_group_list and not user_role_group: From b19deb3a01a11265aa98b4d4de5eb701ac2f77cb Mon Sep 17 00:00:00 2001 From: ivis-futagami Date: Fri, 14 Nov 2025 14:24:55 +0900 Subject: [PATCH 19/19] add migration tool --- postgresql/ddl/W2025-61a.sql | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 postgresql/ddl/W2025-61a.sql diff --git a/postgresql/ddl/W2025-61a.sql b/postgresql/ddl/W2025-61a.sql new file mode 100644 index 0000000000..8f8bb4ce5a --- /dev/null +++ b/postgresql/ddl/W2025-61a.sql @@ -0,0 +1,25 @@ +DO $$ +BEGIN + RAISE NOTICE 'Start: W2025-61a.sql'; + UPDATE index + SET + browsing_group = + CASE + WHEN (browsing_group IS NULL OR browsing_group = '') + THEN '-89' + WHEN browsing_group LIKE '%-89%' + THEN browsing_group + ELSE browsing_group || ',-89' + END, + contribute_group = + CASE + WHEN (contribute_group IS NULL OR contribute_group = '') + THEN '-89' + WHEN contribute_group LIKE '%-89%' + THEN contribute_group + ELSE contribute_group || ',-89' + END + WHERE is_deleted = false; + RAISE NOTICE 'Update completed.'; +END +$$;