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 1/4] Initial plan 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 2/4] 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 3/4] 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 4/4] 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()