From afe1318a5cb8bbc192229ad8f096a240f113f309 Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Thu, 22 Jan 2026 03:37:46 +0900 Subject: [PATCH 01/12] disable storage function --- .../invenio_files_rest/admin.py | 10 +++++++++ .../weko-records-ui/weko_records_ui/config.py | 3 +++ .../file_details_contents.html | 16 +++++++------- .../weko-records-ui/weko_records_ui/views.py | 2 ++ .../weko_user_profiles/forms.py | 21 +++++++++++++++++++ 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/modules/invenio-files-rest/invenio_files_rest/admin.py b/modules/invenio-files-rest/invenio_files_rest/admin.py index 9ca274993c..2fe1711350 100644 --- a/modules/invenio-files-rest/invenio_files_rest/admin.py +++ b/modules/invenio-files-rest/invenio_files_rest/admin.py @@ -148,6 +148,16 @@ class LocationModelView(ModelView): 'System Administrator') _repoadmin_role = os.environ.get('INVENIO_ROLE_REPOSITORY', 'Repository Administrator') + + def get_query(self): + """Override get_query to filter locations based on user roles.""" + query = super(LocationModelView, self).get_query() + user_role_names = {role.name for role in current_user.roles} + if not self._system_role in user_role_names: + """Non-system admins should not see default locations.""" + query = query.filter_by(default=False) + return query + @expose('/') def index_view(self): """Override index view to add custom logic. diff --git a/modules/weko-records-ui/weko_records_ui/config.py b/modules/weko-records-ui/weko_records_ui/config.py index ac792124e2..9637e6e5bd 100644 --- a/modules/weko-records-ui/weko_records_ui/config.py +++ b/modules/weko-records-ui/weko_records_ui/config.py @@ -827,3 +827,6 @@ class FILE_OPEN_STATUS(Enum): WEKO_RECORDS_UI_GAKUNIN_RDM_URL = "https://rdm.nii.ac.jp" """URL of GakuNin RDM.""" + +WEKO_RECORDS_UI_USER_STORAGE_MODIFICATION_ENABLED = False +"""Enable user storage modification feature.""" diff --git a/modules/weko-records-ui/weko_records_ui/templates/weko_records_ui/file_details_contents.html b/modules/weko-records-ui/weko_records_ui/templates/weko_records_ui/file_details_contents.html index fa92b69213..e3c269181d 100644 --- a/modules/weko-records-ui/weko_records_ui/templates/weko_records_ui/file_details_contents.html +++ b/modules/weko-records-ui/weko_records_ui/templates/weko_records_ui/file_details_contents.html @@ -125,13 +125,15 @@

{{ filename. {%- endif -%} - {% if record | check_permission %} - {% if can_edit %} - - - - - + {% if is_storage_editable %} + {% if record | check_permission %} + {% if can_edit %} + + + + + + {% endif %} {% endif %} {% endif %} diff --git a/modules/weko-records-ui/weko_records_ui/views.py b/modules/weko-records-ui/weko_records_ui/views.py index 26545f02a5..aa7376dc06 100644 --- a/modules/weko-records-ui/weko_records_ui/views.py +++ b/modules/weko-records-ui/weko_records_ui/views.py @@ -781,6 +781,7 @@ def _get_rights_title(result, rights_key_str, rights_values, current_lang, meta_ is_no_content_item_application = item_application_settings.get("item_application_enable", False) \ and int(item_type_id) in item_application_settings.get("application_item_types", []) + return render_template( template, pid=pid, @@ -837,6 +838,7 @@ def _get_rights_title(result, rights_key_str, rights_values, current_lang, meta_ restricted_errorMsg = restricted_errorMsg, with_files = with_files, belonging_community=belonging_community, + is_storage_editable=current_app.config.get('WEKO_RECORDS_UI_USER_STORAGE_MODIFICATION_ENABLED'), **ctx, **kwargs ) diff --git a/modules/weko-user-profiles/weko_user_profiles/forms.py b/modules/weko-user-profiles/weko_user_profiles/forms.py index 09cd3b18f1..9adc347851 100644 --- a/modules/weko-user-profiles/weko_user_profiles/forms.py +++ b/modules/weko-user-profiles/weko_user_profiles/forms.py @@ -425,6 +425,27 @@ class ProfileForm(FlaskForm): choices=WEKO_USERPROFILES_INSTITUTE_POSITION_LIST ) + def __init__(self, *args, **kwargs): + super(ProfileForm, self).__init__(*args, **kwargs) + if not current_app.config.get("WEKO_RECORDS_UI_USER_STORAGE_MODIFICATION_ENABLED", False): + if hasattr(self, 'access_key'): + del self.access_key + + if hasattr(self, 'secret_key'): + del self.secret_key + + if hasattr(self, 's3_endpoint_url'): + del self.s3_endpoint_url + + if hasattr(self, 's3_region_name'): + del self.s3_region_name + + + + + + + def validate_username(form, field): """Wrap username validator for WTForms.""" From f203191530809aa1999f1c4160498690889f12ec Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Thu, 22 Jan 2026 16:53:21 +0900 Subject: [PATCH 02/12] Update modules/invenio-files-rest/invenio_files_rest/admin.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- modules/invenio-files-rest/invenio_files_rest/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/invenio-files-rest/invenio_files_rest/admin.py b/modules/invenio-files-rest/invenio_files_rest/admin.py index 2fe1711350..a288c36d1e 100644 --- a/modules/invenio-files-rest/invenio_files_rest/admin.py +++ b/modules/invenio-files-rest/invenio_files_rest/admin.py @@ -156,7 +156,7 @@ def get_query(self): if not self._system_role in user_role_names: """Non-system admins should not see default locations.""" query = query.filter_by(default=False) - return query + return query @expose('/') def index_view(self): From 951d7de0e043d347fcf055dfdcd75f019be8f932 Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Thu, 22 Jan 2026 16:55:10 +0900 Subject: [PATCH 03/12] Update modules/weko-user-profiles/weko_user_profiles/forms.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- modules/weko-user-profiles/weko_user_profiles/forms.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/modules/weko-user-profiles/weko_user_profiles/forms.py b/modules/weko-user-profiles/weko_user_profiles/forms.py index 9adc347851..f56e5579d4 100644 --- a/modules/weko-user-profiles/weko_user_profiles/forms.py +++ b/modules/weko-user-profiles/weko_user_profiles/forms.py @@ -440,13 +440,6 @@ def __init__(self, *args, **kwargs): if hasattr(self, 's3_region_name'): del self.s3_region_name - - - - - - - def validate_username(form, field): """Wrap username validator for WTForms.""" try: From c7b0d4ebe4cf4d1bd4bb95244daafe5aaef2961d Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Thu, 22 Jan 2026 16:55:32 +0900 Subject: [PATCH 04/12] Update modules/invenio-files-rest/invenio_files_rest/admin.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- modules/invenio-files-rest/invenio_files_rest/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/invenio-files-rest/invenio_files_rest/admin.py b/modules/invenio-files-rest/invenio_files_rest/admin.py index a288c36d1e..310d50b93c 100644 --- a/modules/invenio-files-rest/invenio_files_rest/admin.py +++ b/modules/invenio-files-rest/invenio_files_rest/admin.py @@ -154,7 +154,7 @@ def get_query(self): query = super(LocationModelView, self).get_query() user_role_names = {role.name for role in current_user.roles} if not self._system_role in user_role_names: - """Non-system admins should not see default locations.""" + # Non-system admins should not see default locations. query = query.filter_by(default=False) return query From 6c007d77472d602522cf9ce48ae6f8b807483d73 Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Thu, 22 Jan 2026 16:56:45 +0900 Subject: [PATCH 05/12] Update modules/invenio-files-rest/invenio_files_rest/admin.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- modules/invenio-files-rest/invenio_files_rest/admin.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/invenio-files-rest/invenio_files_rest/admin.py b/modules/invenio-files-rest/invenio_files_rest/admin.py index 310d50b93c..b0e6075c7a 100644 --- a/modules/invenio-files-rest/invenio_files_rest/admin.py +++ b/modules/invenio-files-rest/invenio_files_rest/admin.py @@ -158,6 +158,13 @@ def get_query(self): query = query.filter_by(default=False) return query + def get_count_query(self): + """Override get_count_query to apply the same role-based filters.""" + query = super(LocationModelView, self).get_count_query() + user_role_names = {role.name for role in current_user.roles} + if not self._system_role in user_role_names: + query = query.filter_by(default=False) + return query @expose('/') def index_view(self): """Override index view to add custom logic. From 3cf240c09363a70c0af24b48fe3ce62e69fa87bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 07:59:04 +0000 Subject: [PATCH 06/12] Initial plan From b574650c4aa2d9226a090222b97efa62237d1b39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:00:04 +0000 Subject: [PATCH 07/12] Initial plan From 65de914713738fd98a323e19410985d4f6291730 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:03:44 +0000 Subject: [PATCH 08/12] Add tests for ProfileForm __init__ storage field conditional removal Co-authored-by: mhaya <2446046+mhaya@users.noreply.github.com> --- .../weko-user-profiles/tests/test_forms.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/modules/weko-user-profiles/tests/test_forms.py b/modules/weko-user-profiles/tests/test_forms.py index 579a5700c0..e83ab515fa 100644 --- a/modules/weko-user-profiles/tests/test_forms.py +++ b/modules/weko-user-profiles/tests/test_forms.py @@ -386,6 +386,48 @@ def test_validate_username(self,app,client,register_form,users,db): res = client.post("/test_form/profile_form",data=data) assert res.data == bytes("invalid","utf-8") + def test_init_storage_fields_removed_when_disabled(self, app): + """Test that storage fields are removed when feature flag is disabled.""" + # Set the config to disable storage modification + app.config.update( + WEKO_RECORDS_UI_USER_STORAGE_MODIFICATION_ENABLED=False + ) + + with app.app_context(): + # Create the form + form = ProfileForm() + + # Verify storage-related fields are removed + assert not hasattr(form, 'access_key'), "access_key field should be removed when feature is disabled" + assert not hasattr(form, 'secret_key'), "secret_key field should be removed when feature is disabled" + assert not hasattr(form, 's3_endpoint_url'), "s3_endpoint_url field should be removed when feature is disabled" + assert not hasattr(form, 's3_region_name'), "s3_region_name field should be removed when feature is disabled" + + # Verify other fields are still present + assert hasattr(form, 'fullname'), "fullname field should still be present" + assert hasattr(form, 'email'), "email field should still be present" + + def test_init_storage_fields_present_when_enabled(self, app): + """Test that storage fields are present when feature flag is enabled.""" + # Set the config to enable storage modification + app.config.update( + WEKO_RECORDS_UI_USER_STORAGE_MODIFICATION_ENABLED=True + ) + + with app.app_context(): + # Create the form + form = ProfileForm() + + # Verify storage-related fields are present + assert hasattr(form, 'access_key'), "access_key field should be present when feature is enabled" + assert hasattr(form, 'secret_key'), "secret_key field should be present when feature is enabled" + assert hasattr(form, 's3_endpoint_url'), "s3_endpoint_url field should be present when feature is enabled" + assert hasattr(form, 's3_region_name'), "s3_region_name field should be present when feature is enabled" + + # Verify other fields are still present + assert hasattr(form, 'fullname'), "fullname field should still be present" + assert hasattr(form, 'email'), "email field should still be present" + # def custom_profile_form_factory(profile_cls): # .tox/c1/bin/pytest --cov=weko_user_profiles tests/test_forms.py::test_custom_profile_form_factory -vv -s --cov-branch --cov-report=term --cov-report=html --basetemp=/code/modules/weko-user-profiles/.tox/c1/tmp class DummyClass: From 4cb7711f07ca633cc6cb0373ea160fa23ece81e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:05:04 +0000 Subject: [PATCH 09/12] Simplify test assertions and remove verbose error messages Co-authored-by: mhaya <2446046+mhaya@users.noreply.github.com> --- .../weko-user-profiles/tests/test_forms.py | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/modules/weko-user-profiles/tests/test_forms.py b/modules/weko-user-profiles/tests/test_forms.py index e83ab515fa..c4f10b920d 100644 --- a/modules/weko-user-profiles/tests/test_forms.py +++ b/modules/weko-user-profiles/tests/test_forms.py @@ -398,14 +398,10 @@ def test_init_storage_fields_removed_when_disabled(self, app): form = ProfileForm() # Verify storage-related fields are removed - assert not hasattr(form, 'access_key'), "access_key field should be removed when feature is disabled" - assert not hasattr(form, 'secret_key'), "secret_key field should be removed when feature is disabled" - assert not hasattr(form, 's3_endpoint_url'), "s3_endpoint_url field should be removed when feature is disabled" - assert not hasattr(form, 's3_region_name'), "s3_region_name field should be removed when feature is disabled" - - # Verify other fields are still present - assert hasattr(form, 'fullname'), "fullname field should still be present" - assert hasattr(form, 'email'), "email field should still be present" + assert not hasattr(form, 'access_key') + assert not hasattr(form, 'secret_key') + assert not hasattr(form, 's3_endpoint_url') + assert not hasattr(form, 's3_region_name') def test_init_storage_fields_present_when_enabled(self, app): """Test that storage fields are present when feature flag is enabled.""" @@ -419,14 +415,10 @@ def test_init_storage_fields_present_when_enabled(self, app): form = ProfileForm() # Verify storage-related fields are present - assert hasattr(form, 'access_key'), "access_key field should be present when feature is enabled" - assert hasattr(form, 'secret_key'), "secret_key field should be present when feature is enabled" - assert hasattr(form, 's3_endpoint_url'), "s3_endpoint_url field should be present when feature is enabled" - assert hasattr(form, 's3_region_name'), "s3_region_name field should be present when feature is enabled" - - # Verify other fields are still present - assert hasattr(form, 'fullname'), "fullname field should still be present" - assert hasattr(form, 'email'), "email field should still be present" + assert hasattr(form, 'access_key') + assert hasattr(form, 'secret_key') + assert hasattr(form, 's3_endpoint_url') + assert hasattr(form, 's3_region_name') # def custom_profile_form_factory(profile_cls): # .tox/c1/bin/pytest --cov=weko_user_profiles tests/test_forms.py::test_custom_profile_form_factory -vv -s --cov-branch --cov-report=term --cov-report=html --basetemp=/code/modules/weko-user-profiles/.tox/c1/tmp From f15c6af215d60e4db211aeb83ccea161530f320a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:08:41 +0000 Subject: [PATCH 10/12] Add test coverage for get_query and get_count_query methods Co-authored-by: mhaya <2446046+mhaya@users.noreply.github.com> --- .../invenio-files-rest/tests/test_admin.py | 128 ++++++++++++++++++ test_output.txt | 4 + 2 files changed, 132 insertions(+) create mode 100644 test_output.txt diff --git a/modules/invenio-files-rest/tests/test_admin.py b/modules/invenio-files-rest/tests/test_admin.py index 8c546b61f7..9c828b8a90 100644 --- a/modules/invenio-files-rest/tests/test_admin.py +++ b/modules/invenio-files-rest/tests/test_admin.py @@ -343,3 +343,131 @@ def test_can_delete(self, app, db, monkeypatch): with patch('invenio_files_rest.admin.current_user', mock_user): view = LocationModelView(Location, db.session) assert not view.can_delete + + + # def get_query(self) + # .tox/c1/bin/pytest --cov=invenio_files_rest tests/test_admin.py::TestLocationModelView::test_get_query -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-files-rest/.tox/c1/tmp + def test_get_query(self, app, db, monkeypatch): + """Test get_query filters locations based on user roles.""" + monkeypatch.setenv('INVENIO_ROLE_SYSTEM', 'System Administrator') + monkeypatch.setenv('INVENIO_ROLE_REPOSITORY', 'Repository Administrator') + + # Create test locations + default_loc = Location(name='default-loc', uri='/tmp/default', default=True) + non_default_loc = Location(name='non-default-loc', uri='/tmp/non-default', default=False) + db.session.add(default_loc) + db.session.add(non_default_loc) + db.session.commit() + + mock_user = MagicMock() + + # Test Case: System Administrator sees all locations (including default) + mock_role_sysad = MagicMock() + mock_role_sysad.name = 'System Administrator' + mock_user.roles = [mock_role_sysad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' in location_names + assert 'non-default-loc' in location_names + + # Test Case: Repository Administrator does not see default locations + mock_role_repoad = MagicMock() + mock_role_repoad.name = 'Repository Administrator' + mock_user.roles = [mock_role_repoad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' not in location_names + assert 'non-default-loc' in location_names + + # Test Case: Community Administrator does not see default locations + mock_role_commad = MagicMock() + mock_role_commad.name = 'Community Administrator' + mock_user.roles = [mock_role_commad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' not in location_names + assert 'non-default-loc' in location_names + + # Test Case: User without role does not see default locations + mock_user.roles = [] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' not in location_names + assert 'non-default-loc' in location_names + + + # def get_count_query(self) + # .tox/c1/bin/pytest --cov=invenio_files_rest tests/test_admin.py::TestLocationModelView::test_get_count_query -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-files-rest/.tox/c1/tmp + def test_get_count_query(self, app, db, monkeypatch): + """Test get_count_query filters locations based on user roles.""" + monkeypatch.setenv('INVENIO_ROLE_SYSTEM', 'System Administrator') + monkeypatch.setenv('INVENIO_ROLE_REPOSITORY', 'Repository Administrator') + + # Create test locations + default_loc = Location(name='default-loc-count', uri='/tmp/default-count', default=True) + non_default_loc1 = Location(name='non-default-loc-1', uri='/tmp/non-default-1', default=False) + non_default_loc2 = Location(name='non-default-loc-2', uri='/tmp/non-default-2', default=False) + db.session.add(default_loc) + db.session.add(non_default_loc1) + db.session.add(non_default_loc2) + db.session.commit() + + mock_user = MagicMock() + + # Test Case: System Administrator sees count of all locations (including default) + mock_role_sysad = MagicMock() + mock_role_sysad.name = 'System Administrator' + mock_user.roles = [mock_role_sysad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should see all locations in the database + total_locations = db.session.query(Location).count() + assert count == total_locations + + # Test Case: Repository Administrator sees count excluding default locations + mock_role_repoad = MagicMock() + mock_role_repoad.name = 'Repository Administrator' + mock_user.roles = [mock_role_repoad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should only see non-default locations + non_default_count = db.session.query(Location).filter_by(default=False).count() + assert count == non_default_count + + # Test Case: Community Administrator sees count excluding default locations + mock_role_commad = MagicMock() + mock_role_commad.name = 'Community Administrator' + mock_user.roles = [mock_role_commad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should only see non-default locations + non_default_count = db.session.query(Location).filter_by(default=False).count() + assert count == non_default_count + + # Test Case: User without role sees count excluding default locations + mock_user.roles = [] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should only see non-default locations + non_default_count = db.session.query(Location).filter_by(default=False).count() + assert count == non_default_count diff --git a/test_output.txt b/test_output.txt new file mode 100644 index 0000000000..9733e7e3aa --- /dev/null +++ b/test_output.txt @@ -0,0 +1,4 @@ +ImportError while loading conftest '/home/runner/work/weko/weko/modules/invenio-files-rest/tests/conftest.py'. +modules/invenio-files-rest/tests/conftest.py:20: in + from flask_babelex import Babel +E ModuleNotFoundError: No module named 'flask_babelex' From 4b8cfd8e2d1800fd014190ae409c3df9a07ea435 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:09:22 +0000 Subject: [PATCH 11/12] Remove test_output.txt and add to gitignore Co-authored-by: mhaya <2446046+mhaya@users.noreply.github.com> --- .gitignore | 3 +++ test_output.txt | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-) delete mode 100644 test_output.txt diff --git a/.gitignore b/.gitignore index faee6bab08..32927ea4c9 100644 --- a/.gitignore +++ b/.gitignore @@ -79,6 +79,9 @@ scripts/demo/postgresql/ docker-compose.dev.yml *.bak +# Test output files +test_output.txt + #PDF cover page-related files header-icons/* combined-pdfs/* diff --git a/test_output.txt b/test_output.txt deleted file mode 100644 index 9733e7e3aa..0000000000 --- a/test_output.txt +++ /dev/null @@ -1,4 +0,0 @@ -ImportError while loading conftest '/home/runner/work/weko/weko/modules/invenio-files-rest/tests/conftest.py'. -modules/invenio-files-rest/tests/conftest.py:20: in - from flask_babelex import Babel -E ModuleNotFoundError: No module named 'flask_babelex' From 3d039f5257544b97e4c0a90e152b26ad0016ffc1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:11:21 +0000 Subject: [PATCH 12/12] Address code review feedback: remove command comments and add cleanup Co-authored-by: mhaya <2446046+mhaya@users.noreply.github.com> --- .../invenio-files-rest/tests/test_admin.py | 205 +++++++++--------- 1 file changed, 107 insertions(+), 98 deletions(-) diff --git a/modules/invenio-files-rest/tests/test_admin.py b/modules/invenio-files-rest/tests/test_admin.py index 9c828b8a90..6dfefc9b4c 100644 --- a/modules/invenio-files-rest/tests/test_admin.py +++ b/modules/invenio-files-rest/tests/test_admin.py @@ -345,8 +345,6 @@ def test_can_delete(self, app, db, monkeypatch): assert not view.can_delete - # def get_query(self) - # .tox/c1/bin/pytest --cov=invenio_files_rest tests/test_admin.py::TestLocationModelView::test_get_query -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-files-rest/.tox/c1/tmp def test_get_query(self, app, db, monkeypatch): """Test get_query filters locations based on user roles.""" monkeypatch.setenv('INVENIO_ROLE_SYSTEM', 'System Administrator') @@ -359,57 +357,61 @@ def test_get_query(self, app, db, monkeypatch): db.session.add(non_default_loc) db.session.commit() - mock_user = MagicMock() - - # Test Case: System Administrator sees all locations (including default) - mock_role_sysad = MagicMock() - mock_role_sysad.name = 'System Administrator' - mock_user.roles = [mock_role_sysad] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_query() - locations = query.all() - location_names = {loc.name for loc in locations} - assert 'default-loc' in location_names - assert 'non-default-loc' in location_names - - # Test Case: Repository Administrator does not see default locations - mock_role_repoad = MagicMock() - mock_role_repoad.name = 'Repository Administrator' - mock_user.roles = [mock_role_repoad] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_query() - locations = query.all() - location_names = {loc.name for loc in locations} - assert 'default-loc' not in location_names - assert 'non-default-loc' in location_names - - # Test Case: Community Administrator does not see default locations - mock_role_commad = MagicMock() - mock_role_commad.name = 'Community Administrator' - mock_user.roles = [mock_role_commad] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_query() - locations = query.all() - location_names = {loc.name for loc in locations} - assert 'default-loc' not in location_names - assert 'non-default-loc' in location_names - - # Test Case: User without role does not see default locations - mock_user.roles = [] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_query() - locations = query.all() - location_names = {loc.name for loc in locations} - assert 'default-loc' not in location_names - assert 'non-default-loc' in location_names + try: + mock_user = MagicMock() + + # Test Case: System Administrator sees all locations (including default) + mock_role_sysad = MagicMock() + mock_role_sysad.name = 'System Administrator' + mock_user.roles = [mock_role_sysad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' in location_names + assert 'non-default-loc' in location_names + + # Test Case: Repository Administrator does not see default locations + mock_role_repoad = MagicMock() + mock_role_repoad.name = 'Repository Administrator' + mock_user.roles = [mock_role_repoad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' not in location_names + assert 'non-default-loc' in location_names + + # Test Case: Community Administrator does not see default locations + mock_role_commad = MagicMock() + mock_role_commad.name = 'Community Administrator' + mock_user.roles = [mock_role_commad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' not in location_names + assert 'non-default-loc' in location_names + + # Test Case: User without role does not see default locations + mock_user.roles = [] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_query() + locations = query.all() + location_names = {loc.name for loc in locations} + assert 'default-loc' not in location_names + assert 'non-default-loc' in location_names + finally: + # Clean up test locations + db.session.delete(default_loc) + db.session.delete(non_default_loc) + db.session.commit() - # def get_count_query(self) - # .tox/c1/bin/pytest --cov=invenio_files_rest tests/test_admin.py::TestLocationModelView::test_get_count_query -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/invenio-files-rest/.tox/c1/tmp def test_get_count_query(self, app, db, monkeypatch): """Test get_count_query filters locations based on user roles.""" monkeypatch.setenv('INVENIO_ROLE_SYSTEM', 'System Administrator') @@ -424,50 +426,57 @@ def test_get_count_query(self, app, db, monkeypatch): db.session.add(non_default_loc2) db.session.commit() - mock_user = MagicMock() - - # Test Case: System Administrator sees count of all locations (including default) - mock_role_sysad = MagicMock() - mock_role_sysad.name = 'System Administrator' - mock_user.roles = [mock_role_sysad] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_count_query() - count = query.count() - # Should see all locations in the database - total_locations = db.session.query(Location).count() - assert count == total_locations - - # Test Case: Repository Administrator sees count excluding default locations - mock_role_repoad = MagicMock() - mock_role_repoad.name = 'Repository Administrator' - mock_user.roles = [mock_role_repoad] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_count_query() - count = query.count() - # Should only see non-default locations - non_default_count = db.session.query(Location).filter_by(default=False).count() - assert count == non_default_count - - # Test Case: Community Administrator sees count excluding default locations - mock_role_commad = MagicMock() - mock_role_commad.name = 'Community Administrator' - mock_user.roles = [mock_role_commad] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_count_query() - count = query.count() - # Should only see non-default locations - non_default_count = db.session.query(Location).filter_by(default=False).count() - assert count == non_default_count - - # Test Case: User without role sees count excluding default locations - mock_user.roles = [] - with patch('invenio_files_rest.admin.current_user', mock_user): - view = LocationModelView(Location, db.session) - query = view.get_count_query() - count = query.count() - # Should only see non-default locations - non_default_count = db.session.query(Location).filter_by(default=False).count() - assert count == non_default_count + try: + mock_user = MagicMock() + + # Test Case: System Administrator sees count of all locations (including default) + mock_role_sysad = MagicMock() + mock_role_sysad.name = 'System Administrator' + mock_user.roles = [mock_role_sysad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should see all locations in the database + total_locations = db.session.query(Location).count() + assert count == total_locations + + # Test Case: Repository Administrator sees count excluding default locations + mock_role_repoad = MagicMock() + mock_role_repoad.name = 'Repository Administrator' + mock_user.roles = [mock_role_repoad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should only see non-default locations + non_default_count = db.session.query(Location).filter_by(default=False).count() + assert count == non_default_count + + # Test Case: Community Administrator sees count excluding default locations + mock_role_commad = MagicMock() + mock_role_commad.name = 'Community Administrator' + mock_user.roles = [mock_role_commad] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should only see non-default locations + non_default_count = db.session.query(Location).filter_by(default=False).count() + assert count == non_default_count + + # Test Case: User without role sees count excluding default locations + mock_user.roles = [] + with patch('invenio_files_rest.admin.current_user', mock_user): + view = LocationModelView(Location, db.session) + query = view.get_count_query() + count = query.count() + # Should only see non-default locations + non_default_count = db.session.query(Location).filter_by(default=False).count() + assert count == non_default_count + finally: + # Clean up test locations + db.session.delete(default_loc) + db.session.delete(non_default_loc1) + db.session.delete(non_default_loc2) + db.session.commit()