From f73f3cf3379e81f76f3a51c87c1f669a956f80b9 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 11 Nov 2024 13:29:05 +0800 Subject: [PATCH 001/122] add framework and basic test for API scenario --- api_tests/Dockerfile | 6 +++ api_tests/requirements.txt | 2 + api_tests/test_happy_library_scenario.py | 46 +++++++++++++++++++ docker-compose.dev.yml | 17 ++++--- docker-compose.test.yml | 57 ++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 api_tests/Dockerfile create mode 100644 api_tests/requirements.txt create mode 100644 api_tests/test_happy_library_scenario.py create mode 100644 docker-compose.test.yml diff --git a/api_tests/Dockerfile b/api_tests/Dockerfile new file mode 100644 index 0000000000..ea4fa0f500 --- /dev/null +++ b/api_tests/Dockerfile @@ -0,0 +1,6 @@ +ARG PYTHON_VERSION=3.9-bookworm +FROM python:3.11 +WORKDIR /code/ +COPY . . +RUN pip install -r requirements.txt +CMD pytest -svv diff --git a/api_tests/requirements.txt b/api_tests/requirements.txt new file mode 100644 index 0000000000..cbfa5a1284 --- /dev/null +++ b/api_tests/requirements.txt @@ -0,0 +1,2 @@ +requests==2.32.3 +pytest==8.3.3 diff --git a/api_tests/test_happy_library_scenario.py b/api_tests/test_happy_library_scenario.py new file mode 100644 index 0000000000..99e6c3ac22 --- /dev/null +++ b/api_tests/test_happy_library_scenario.py @@ -0,0 +1,46 @@ +import pytest +import requests +from urllib import parse as urlparse + +SERVICE_HOST = 'http://mathesar-api-test-service:8000' +RPC_ENDPOINT = f'{SERVICE_HOST}/api/rpc/v0/' +MATHESAR_DB = 'mathesar' + +@pytest.fixture(scope="session") +def admin_session(): + login_payload = {'username': 'admin', 'password': 'password'} + s = requests.Session() + s.get(f'{SERVICE_HOST}/auth/login/') + s.headers['X-CSRFToken'] = s.cookies['csrftoken'] + s.post(f'{SERVICE_HOST}/auth/login/', data=login_payload) + s.headers['X-CSRFToken'] = s.cookies['csrftoken'] + return s + + +def test_empty_db_list(admin_session): + response = admin_session.post( + RPC_ENDPOINT, + json={"jsonrpc": "2.0", "method": "databases.configured.list", "id": 0} + ) + print(response.json()) + assert response.json()['result'] == [] + + +def test_create_mathesar_db(admin_session): + response = admin_session.post( + RPC_ENDPOINT, + json={ + "jsonrpc": "2.0", + "method": "databases.setup.create_new", + "id": 0, + "params": { + "database": "mathesar", + "sample_data": ["library_management"], + } + } + ) + print(response.json()) + result = response.json()['result'] + assert set(result.keys()) == set(['configured_role', 'database', 'server']) + assert result['database']['name'] == 'mathesar' + assert result['database']['needs_upgrade_attention'] is False diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 3bede37fc4..68aca79483 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -4,8 +4,7 @@ services: # Mathesar App built with the same configurations as the production image # but with additional testing dependencies. # It is used to run automated test cases to verify if the app works as intended - dev-db: - container_name: mathesar_dev_db + dev-db-base: command: ["postgres", "-c", "shared_preload_libraries=plugin_debugger"] build: context: . @@ -17,17 +16,21 @@ services: - POSTGRES_DB=mathesar_django - POSTGRES_USER=mathesar - POSTGRES_PASSWORD=mathesar - ports: - - "5432:5432" - volumes: - - dev_postgres_data:/var/lib/postgresql/data - - ./db/sql:/sql/ healthcheck: test: [ "CMD-SHELL", "pg_isready -d $${POSTGRES_DB-mathesar_django} -U $${POSTGRES_USER-mathesar}"] interval: 5s timeout: 1s retries: 30 start_period: 5s + dev-db: + container_name: mathesar_dev_db + extends: + service: dev-db-base + ports: + - "5432:5432" + volumes: + - dev_postgres_data:/var/lib/postgresql/data + - ./db/sql:/sql/ # A Django development webserver + Svelte development server used when developing Mathesar. # The code changes are hot reloaded and debug flags are enabled to aid developers working on Mathesar. # It is not recommended to use this service in production environment. diff --git a/docker-compose.test.yml b/docker-compose.test.yml new file mode 100644 index 0000000000..1b2336522b --- /dev/null +++ b/docker-compose.test.yml @@ -0,0 +1,57 @@ +--- +services: + test-db: + container_name: mathesar-test-db + extends: + file: docker-compose.dev.yml + service: dev-db-base + expose: + - "5432" + volumes: + - ./db/sql:/sql/ + healthcheck: + interval: 1s + start_period: 2s + api-test-service: + container_name: mathesar-api-test-service + build: + context: . + target: testing + dockerfile: Dockerfile + args: + PYTHON_VERSION: ${PYTHON_VERSION-3.9-bookworm} + environment: + - MODE=DEVELOPMENT + - DEBUG=True + - ALLOWED_HOSTS=* + - SECRET_KEY=${SECRET_KEY} + - DJANGO_SUPERUSER_PASSWORD=password + - POSTGRES_DB=mathesar_django + - POSTGRES_USER=mathesar + - POSTGRES_PASSWORD=mathesar + - POSTGRES_HOST=mathesar-test-db + - POSTGRES_PORT=5432 + volumes: + - .:/code/ + depends_on: + test-db: + condition: service_healthy + healthcheck: + test: ["CMD-SHELL", "curl --fail http://localhost:8000/ || exit 1"] + interval: 1s + timeout: 1s + retries: 30 + start_period: 3s + expose: + - "8000" + test-runner: + container_name: mathesar-api-test-runner + build: + context: api_tests + depends_on: + api-test-service: + condition: service_healthy + volumes: + - ./api_tests/:/code/ +volumes: + ui_node_modules_test: From ca4352d025a7b9e18e813c9f399af10474698de6 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 11 Nov 2024 13:58:11 +0800 Subject: [PATCH 002/122] add test for connecting external DB, add test runner script --- ...ry_scenario.py => test_happy_db_setups.py} | 26 ++++++++++++++++++- docker-compose.test.yml | 12 +++++++-- run_api_tests.sh | 9 +++++++ 3 files changed, 44 insertions(+), 3 deletions(-) rename api_tests/{test_happy_library_scenario.py => test_happy_db_setups.py} (61%) create mode 100755 run_api_tests.sh diff --git a/api_tests/test_happy_library_scenario.py b/api_tests/test_happy_db_setups.py similarity index 61% rename from api_tests/test_happy_library_scenario.py rename to api_tests/test_happy_db_setups.py index 99e6c3ac22..9a877cb767 100644 --- a/api_tests/test_happy_library_scenario.py +++ b/api_tests/test_happy_db_setups.py @@ -26,7 +26,7 @@ def test_empty_db_list(admin_session): assert response.json()['result'] == [] -def test_create_mathesar_db(admin_session): +def test_create_mathesar_db_internal(admin_session): response = admin_session.post( RPC_ENDPOINT, json={ @@ -44,3 +44,27 @@ def test_create_mathesar_db(admin_session): assert set(result.keys()) == set(['configured_role', 'database', 'server']) assert result['database']['name'] == 'mathesar' assert result['database']['needs_upgrade_attention'] is False + + +def test_connect_mathesar_db_external(admin_session): + response = admin_session.post( + RPC_ENDPOINT, + json={ + "jsonrpc": "2.0", + "method": "databases.setup.connect_existing", + "id": 0, + "params": { + "host": "mathesar-test-user-db", + "port": 5432, + "database": "my_data", + "role": "data_admin", + "password": "data1234", + } + } + ) + print(response.json()) + result = response.json()['result'] + assert set(result.keys()) == set(['configured_role', 'database', 'server']) + assert result['database']['name'] == 'my_data' + assert result['database']['needs_upgrade_attention'] is False + assert result['configured_role']['name'] == 'data_admin' diff --git a/docker-compose.test.yml b/docker-compose.test.yml index 1b2336522b..d3fe558ba7 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -7,11 +7,17 @@ services: service: dev-db-base expose: - "5432" - volumes: - - ./db/sql:/sql/ healthcheck: interval: 1s start_period: 2s + test-user-db: + container_name: mathesar-test-user-db + extends: + service: test-db + environment: + - POSTGRES_DB=my_data + - POSTGRES_USER=data_admin + - POSTGRES_PASSWORD=data1234 api-test-service: container_name: mathesar-api-test-service build: @@ -51,6 +57,8 @@ services: depends_on: api-test-service: condition: service_healthy + test-user-db: + condition: service_healthy volumes: - ./api_tests/:/code/ volumes: diff --git a/run_api_tests.sh b/run_api_tests.sh new file mode 100755 index 0000000000..c20d431cee --- /dev/null +++ b/run_api_tests.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env sh + +docker compose -f docker-compose.test.yml up \ + --abort-on-container-exit \ + --timeout 0 \ + --force-recreate \ + -V \ + --exit-code-from test-runner \ + test-runner test-user-db test-db api-test-service From 21207633f4e9a62454e7289bde31a23c247ac256 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 11 Nov 2024 17:43:08 +0800 Subject: [PATCH 003/122] use instead of for test runner --- run_api_tests.sh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/run_api_tests.sh b/run_api_tests.sh index c20d431cee..cb3de7cf65 100755 --- a/run_api_tests.sh +++ b/run_api_tests.sh @@ -1,9 +1,8 @@ #!/usr/bin/env sh -docker compose -f docker-compose.test.yml up \ - --abort-on-container-exit \ - --timeout 0 \ - --force-recreate \ - -V \ - --exit-code-from test-runner \ - test-runner test-user-db test-db api-test-service +EXIT_CODE=0 +docker compose -f docker-compose.test.yml run --rm test-runner pytest -svv test_happy_db_setups.py +# Needed once (if) we have multiple scenarios to test. This accumulates the max exit code. +EXIT_CODE=$(( EXIT_CODE > $? ? EXIT_CODE : $? )) +docker compose -f docker-compose.test.yml down -v -t 1 +exit $EXIT_CODE From a1c640e7bbad99c2cd1482e8512e549bdc85fa79 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 11 Nov 2024 17:45:47 +0800 Subject: [PATCH 004/122] add tests for sql_upgrade functionality --- api_tests/test_happy_db_setups.py | 105 +++++++++++++++++++----------- 1 file changed, 67 insertions(+), 38 deletions(-) diff --git a/api_tests/test_happy_db_setups.py b/api_tests/test_happy_db_setups.py index 9a877cb767..5ffc20ea47 100644 --- a/api_tests/test_happy_db_setups.py +++ b/api_tests/test_happy_db_setups.py @@ -3,6 +3,8 @@ from urllib import parse as urlparse SERVICE_HOST = 'http://mathesar-api-test-service:8000' +INTERNAL_HOST = 'mathesar-test-db' +EXTERNAL_HOST = 'mathesar-test-user-db' RPC_ENDPOINT = f'{SERVICE_HOST}/api/rpc/v0/' MATHESAR_DB = 'mathesar' @@ -17,54 +19,81 @@ def admin_session(): return s -def test_empty_db_list(admin_session): - response = admin_session.post( - RPC_ENDPOINT, - json={"jsonrpc": "2.0", "method": "databases.configured.list", "id": 0} - ) - print(response.json()) - assert response.json()['result'] == [] +@pytest.fixture(scope="session") +def admin_rpc_call(admin_session): + def _admin_rpc_request(function, **kwargs): + return admin_session.post( + RPC_ENDPOINT, + json={ + "jsonrpc": "2.0", + "method": function, + "params": kwargs, + "id": 0, + } + ).json()['result'] + return _admin_rpc_request -def test_create_mathesar_db_internal(admin_session): - response = admin_session.post( - RPC_ENDPOINT, - json={ - "jsonrpc": "2.0", - "method": "databases.setup.create_new", - "id": 0, - "params": { - "database": "mathesar", - "sample_data": ["library_management"], - } - } +def test_empty_db_list(admin_rpc_call): + db_list = admin_rpc_call('databases.configured.list') + assert db_list == [] + + +def test_create_mathesar_db_internal(admin_rpc_call): + result = admin_rpc_call( + 'databases.setup.create_new', + database='mathesar', + sample_data=['library_management'] ) - print(response.json()) - result = response.json()['result'] assert set(result.keys()) == set(['configured_role', 'database', 'server']) assert result['database']['name'] == 'mathesar' assert result['database']['needs_upgrade_attention'] is False -def test_connect_mathesar_db_external(admin_session): - response = admin_session.post( - RPC_ENDPOINT, - json={ - "jsonrpc": "2.0", - "method": "databases.setup.connect_existing", - "id": 0, - "params": { - "host": "mathesar-test-user-db", - "port": 5432, - "database": "my_data", - "role": "data_admin", - "password": "data1234", - } - } +def test_connect_mathesar_db_external(admin_rpc_call): + result = admin_rpc_call( + 'databases.setup.connect_existing', + host='mathesar-test-user-db', + port=5432, + database='my_data', + role='data_admin', + password='data1234', ) - print(response.json()) - result = response.json()['result'] assert set(result.keys()) == set(['configured_role', 'database', 'server']) assert result['database']['name'] == 'my_data' assert result['database']['needs_upgrade_attention'] is False assert result['configured_role']['name'] == 'data_admin' + + +def test_list_databases_has_upgrade_status(admin_rpc_call): + result = admin_rpc_call('databases.configured.list') + assert all(d['needs_upgrade_attention'] is False for d in result) + + +def test_batch_sql_update_no_error(admin_rpc_call, admin_session): + servers_list = admin_rpc_call('servers.configured.list') + internal_server = [s for s in servers_list if s['host'] == INTERNAL_HOST][0] + external_server = [s for s in servers_list if s['host'] == EXTERNAL_HOST][0] + internal_db = admin_rpc_call( + 'databases.configured.list', server_id=internal_server['id'] + )[0] + external_db = admin_rpc_call( + 'databases.configured.list', server_id=external_server['id'] + )[0] + admin_session.post( + RPC_ENDPOINT, + json=[ + { + "jsonrpc": "2.0", + "method": "databases.upgrade_sql", + "id": "0", + "params": {"database_id": internal_db["id"]} + }, + { + "jsonrpc": "2.0", + "method": "databases.upgrade_sql", + "id": "2", + "params": {"database_id": external_db["id"]} + }, + ] + ) From 245b11ee0bd6d5697bd39d5eb5e4f73348bb389b Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 12 Nov 2024 17:50:27 +0800 Subject: [PATCH 005/122] use globals for response state --- api_tests/test_happy_db_setups.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/api_tests/test_happy_db_setups.py b/api_tests/test_happy_db_setups.py index 5ffc20ea47..175a65a2f5 100644 --- a/api_tests/test_happy_db_setups.py +++ b/api_tests/test_happy_db_setups.py @@ -40,17 +40,20 @@ def test_empty_db_list(admin_rpc_call): def test_create_mathesar_db_internal(admin_rpc_call): + global internal_db result = admin_rpc_call( 'databases.setup.create_new', database='mathesar', sample_data=['library_management'] ) assert set(result.keys()) == set(['configured_role', 'database', 'server']) - assert result['database']['name'] == 'mathesar' - assert result['database']['needs_upgrade_attention'] is False + internal_db = result['database'] + assert internal_db['name'] == 'mathesar' + assert internal_db['needs_upgrade_attention'] is False def test_connect_mathesar_db_external(admin_rpc_call): + global external_db result = admin_rpc_call( 'databases.setup.connect_existing', host='mathesar-test-user-db', @@ -60,8 +63,9 @@ def test_connect_mathesar_db_external(admin_rpc_call): password='data1234', ) assert set(result.keys()) == set(['configured_role', 'database', 'server']) - assert result['database']['name'] == 'my_data' - assert result['database']['needs_upgrade_attention'] is False + external_db = result['database'] + assert external_db['name'] == 'my_data' + assert external_db['needs_upgrade_attention'] is False assert result['configured_role']['name'] == 'data_admin' @@ -70,16 +74,8 @@ def test_list_databases_has_upgrade_status(admin_rpc_call): assert all(d['needs_upgrade_attention'] is False for d in result) -def test_batch_sql_update_no_error(admin_rpc_call, admin_session): - servers_list = admin_rpc_call('servers.configured.list') - internal_server = [s for s in servers_list if s['host'] == INTERNAL_HOST][0] - external_server = [s for s in servers_list if s['host'] == EXTERNAL_HOST][0] - internal_db = admin_rpc_call( - 'databases.configured.list', server_id=internal_server['id'] - )[0] - external_db = admin_rpc_call( - 'databases.configured.list', server_id=external_server['id'] - )[0] +def test_batch_sql_update_no_error(admin_session): + # globals: internal_db, external_db admin_session.post( RPC_ENDPOINT, json=[ From a7878c9351d4b259d68818f3936c391c7e8666e1 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Tue, 12 Nov 2024 22:38:20 +0530 Subject: [PATCH 006/122] add users namespace in settings --- config/settings/common_settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/config/settings/common_settings.py b/config/settings/common_settings.py index 2fda29f914..bf7b9494aa 100644 --- a/config/settings/common_settings.py +++ b/config/settings/common_settings.py @@ -81,6 +81,7 @@ def pipe_delim(pipe_string): 'mathesar.rpc.tables', 'mathesar.rpc.tables.metadata', 'mathesar.rpc.tables.privileges', + 'mathesar.rpc.users' ] TEMPLATES = [ From b5ed90526dd38c3b0e6db6276eae2214a2552998 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Wed, 13 Nov 2024 17:39:10 +0800 Subject: [PATCH 007/122] add intern privilege granting scenario --- api_tests/test_happy_db_setups.py | 308 ++++++++++++++++++++++++++++-- 1 file changed, 294 insertions(+), 14 deletions(-) diff --git a/api_tests/test_happy_db_setups.py b/api_tests/test_happy_db_setups.py index 175a65a2f5..2c6b642c94 100644 --- a/api_tests/test_happy_db_setups.py +++ b/api_tests/test_happy_db_setups.py @@ -3,26 +3,26 @@ from urllib import parse as urlparse SERVICE_HOST = 'http://mathesar-api-test-service:8000' -INTERNAL_HOST = 'mathesar-test-db' EXTERNAL_HOST = 'mathesar-test-user-db' RPC_ENDPOINT = f'{SERVICE_HOST}/api/rpc/v0/' -MATHESAR_DB = 'mathesar' +USERS_ENDPOINT = f'{SERVICE_HOST}/api/ui/v0/users/' -@pytest.fixture(scope="session") + +@pytest.fixture(scope="module") def admin_session(): - login_payload = {'username': 'admin', 'password': 'password'} + login_payload = {"username": "admin", "password": "password"} s = requests.Session() - s.get(f'{SERVICE_HOST}/auth/login/') + r = s.get(f'{SERVICE_HOST}/auth/login/') s.headers['X-CSRFToken'] = s.cookies['csrftoken'] s.post(f'{SERVICE_HOST}/auth/login/', data=login_payload) s.headers['X-CSRFToken'] = s.cookies['csrftoken'] return s -@pytest.fixture(scope="session") +@pytest.fixture(scope="module") def admin_rpc_call(admin_session): def _admin_rpc_request(function, **kwargs): - return admin_session.post( + response = admin_session.post( RPC_ENDPOINT, json={ "jsonrpc": "2.0", @@ -30,17 +30,59 @@ def _admin_rpc_request(function, **kwargs): "params": kwargs, "id": 0, } - ).json()['result'] + ).json() + # print(response) + return response['result'] return _admin_rpc_request +@pytest.fixture(scope="module") +def intern_session(): + # This function handles Django's auto-password-reset flow + # NOTE: This will only work after the admin adds `intern` as a user! + init_login_payload = {"username": "intern", "password": "password"} + s = requests.Session() + s.get(f'{SERVICE_HOST}/auth/login/') + s.headers['X-CSRFToken'] = s.cookies['csrftoken'] + s.post(f'{SERVICE_HOST}/auth/login/', data=init_login_payload) + s.headers['X-CSRFToken'] = s.cookies['csrftoken'] + reset_payload={ + "new_password1": "myinternpass1234", + "new_password2": "myinternpass1234" + } + s.post(f'{SERVICE_HOST}/auth/password_reset_confirm', data=reset_payload) + s.headers['X-CSRFToken'] = s.cookies['csrftoken'] + new_login_payload = {"username": "intern", "password": "myinternpass1234"} + s.post(f'{SERVICE_HOST}/auth/login/', data=new_login_payload) + s.headers['X-CSRFToken'] = s.cookies['csrftoken'] + return s + + +@pytest.fixture(scope="module") +def intern_rpc_call(intern_session): + def _intern_rpc_request(function, **kwargs): + response = intern_session.post( + RPC_ENDPOINT, + json={ + "jsonrpc": "2.0", + "method": function, + "params": kwargs, + "id": 0, + } + ).json() + # print(response) + return response['result'] + return _intern_rpc_request + + def test_empty_db_list(admin_rpc_call): db_list = admin_rpc_call('databases.configured.list') assert db_list == [] def test_create_mathesar_db_internal(admin_rpc_call): - global internal_db + global internal_db_id + global internal_server_id result = admin_rpc_call( 'databases.setup.create_new', database='mathesar', @@ -48,15 +90,18 @@ def test_create_mathesar_db_internal(admin_rpc_call): ) assert set(result.keys()) == set(['configured_role', 'database', 'server']) internal_db = result['database'] + internal_server = result['server'] assert internal_db['name'] == 'mathesar' assert internal_db['needs_upgrade_attention'] is False + internal_db_id = internal_db['id'] + internal_server_id = internal_server['id'] def test_connect_mathesar_db_external(admin_rpc_call): - global external_db + global external_db_id result = admin_rpc_call( 'databases.setup.connect_existing', - host='mathesar-test-user-db', + host=EXTERNAL_HOST, port=5432, database='my_data', role='data_admin', @@ -67,6 +112,7 @@ def test_connect_mathesar_db_external(admin_rpc_call): assert external_db['name'] == 'my_data' assert external_db['needs_upgrade_attention'] is False assert result['configured_role']['name'] == 'data_admin' + external_db_id = external_db['id'] def test_list_databases_has_upgrade_status(admin_rpc_call): @@ -75,7 +121,6 @@ def test_list_databases_has_upgrade_status(admin_rpc_call): def test_batch_sql_update_no_error(admin_session): - # globals: internal_db, external_db admin_session.post( RPC_ENDPOINT, json=[ @@ -83,13 +128,248 @@ def test_batch_sql_update_no_error(admin_session): "jsonrpc": "2.0", "method": "databases.upgrade_sql", "id": "0", - "params": {"database_id": internal_db["id"]} + "params": {"database_id": internal_db_id} }, { "jsonrpc": "2.0", "method": "databases.upgrade_sql", "id": "2", - "params": {"database_id": external_db["id"]} + "params": {"database_id": external_db_id} }, ] ) + + +def test_get_current_role(admin_rpc_call): + global mathesar_role_oid + result = admin_rpc_call( + 'roles.get_current_role', + database_id=internal_db_id, + ) + assert result['current_role']['super'] is True + mathesar_role_oid = result['current_role']['oid'] + + +# Now the scenario proper starts. The admin will add an `intern` user to +# Mathesar, an `intern` role to the internal `mathesar` DB, then grant +# `CONNECT` to that role on the DB, `USAGE` on "Library Management", and +# `SELECT` on "Books". +# +# We'll follow along with the API calls made by the front end. + + +def test_add_user(admin_session): + global intern_user_id + before_users = admin_session.get(USERS_ENDPOINT).json()['results'] + assert len(before_users) == 1 + intern_add = { + 'display_language': 'en', + 'email': 'intern@example.com', + 'is_superuser': False, + 'password': 'password', + 'username': 'intern', + } + admin_session.post(USERS_ENDPOINT, json=intern_add) + after_users = admin_session.get(USERS_ENDPOINT).json()['results'] + assert len(after_users) == 2 + intern_user_id = [ + u['id'] for u in after_users if u['username'] == 'intern' + ][0] + + + +def test_intern_no_databases(intern_rpc_call): + db_list = intern_rpc_call('databases.configured.list') + assert db_list == [] + + +def test_list_configured_roles(admin_rpc_call): + # The only role at this point should be the initial `mathesar` role. + result = admin_rpc_call( + 'roles.configured.list', + server_id=internal_server_id, + ) + assert len(result) == 1 + + +def test_add_role(admin_rpc_call): + global intern_role_oid + result = admin_rpc_call( + 'roles.add', + database_id=internal_db_id, + login=True, + rolename='intern', + password='internpass' + ) + intern_role_oid = result['oid'] + + +def test_configure_role(admin_rpc_call): + global intern_configured_role_id + result = admin_rpc_call( + 'roles.configured.add', + name='intern', + password='internpass', + server_id=internal_server_id, + ) + assert 'password' not in result.keys() + intern_configured_role_id = result['id'] + + +def test_add_collaborator(admin_rpc_call): + before_collaborators = admin_rpc_call( + 'collaborators.list', + database_id=internal_db_id, + ) + # Should only have the admin so far + assert len(before_collaborators) == 1 + result = admin_rpc_call( + 'collaborators.add', + configured_role_id=intern_configured_role_id, + database_id=internal_db_id, + user_id=intern_user_id, + ) + after_collaborators = admin_rpc_call( + 'collaborators.list', + database_id=internal_db_id, + ) + assert len(after_collaborators) == 2 + intern_collab_definition = [ + c for c in after_collaborators if c['user_id'] == intern_user_id + ][0] + assert intern_collab_definition['database_id'] == internal_db_id + assert intern_collab_definition['configured_role_id'] == intern_configured_role_id + + +def test_intern_has_internal_database(intern_rpc_call): + db_list = intern_rpc_call('databases.configured.list') + assert len(db_list) == 1 + assert db_list[0]['id'] == internal_db_id + + +def test_database_privileges_add(admin_rpc_call): + before_result = admin_rpc_call( + 'databases.privileges.list_direct', + database_id=internal_db_id, + ) + print(before_result) + # `intern` shouldn't have any privileges yet. + assert intern_role_oid not in [r['role_oid'] for r in before_result] + after_result = admin_rpc_call( + 'databases.privileges.replace_for_roles', + database_id=internal_db_id, + privileges=[{'direct': ['CONNECT'], 'role_oid': intern_role_oid}] + ) + # `intern` should have `CONNECT` after this point. + assert [ + d['direct'] for d in after_result if d['role_oid'] == intern_role_oid + ][0] == ['CONNECT'] + + +def test_schemas_list(admin_rpc_call): + global library_management_oid + result = admin_rpc_call( + 'schemas.list', + database_id=1 + ) + # Should have `public` and `Library Management` + assert len(result) == 2 + library_management_oid = [ + s['oid'] for s in result if s['name'] == 'Library Management' + ][0] + + +def test_tables_list(admin_rpc_call): + global books_oid + result = admin_rpc_call( + 'tables.list', + database_id=1, + schema_oid=library_management_oid + ) + assert len(result) == 7 + books_oid = [t['oid'] for t in result if t['name'] == 'Books'][0] + + +def test_intern_cannot_access_library_schema_tables(intern_session): + response = intern_session.post( + RPC_ENDPOINT, + json={ + "jsonrpc": "2.0", + "method": 'records.list', + "params": { + "table_oid": books_oid, + "database_id": internal_db_id, + "limit": 20 + }, + "id": 0, + } + ).json() + assert response['error']['code'] == -30101 # InsufficientPrivilege + assert "permission denied for schema" in response['error']['message'] + + +def test_schema_privileges_add(admin_rpc_call): + before_result = admin_rpc_call( + 'schemas.privileges.list_direct', + database_id=internal_db_id, + schema_oid=library_management_oid, + ) + # `intern` should not have any schema privileges yet. + assert intern_role_oid not in [r['role_oid'] for r in before_result] + after_result = admin_rpc_call( + 'schemas.privileges.replace_for_roles', + database_id=internal_db_id, + schema_oid=library_management_oid, + privileges=[{'direct': ['USAGE'], 'role_oid': intern_role_oid}], + ) + assert [ + d['direct'] for d in after_result if d['role_oid'] == intern_role_oid + ][0] == ['USAGE'] + + +def test_intern_still_cannot_access_books_table_data(intern_session): + response = intern_session.post( + RPC_ENDPOINT, + json={ + "jsonrpc": "2.0", + "method": 'records.list', + "params": { + "table_oid": books_oid, + "database_id": internal_db_id, + "limit": 20 + }, + "id": 0, + } + ).json() + assert response['error']['code'] == -30101 # InsufficientPrivilege + assert "permission denied for table" in response['error']['message'] + + +def test_table_privileges_add(admin_rpc_call): + before_result = admin_rpc_call( + 'tables.privileges.list_direct', + table_oid=books_oid, + database_id=internal_db_id, + ) + # `intern` should not have any schema privileges yet. + assert intern_role_oid not in [r['role_oid'] for r in before_result] + after_result = admin_rpc_call( + 'tables.privileges.replace_for_roles', + table_oid=books_oid, + database_id=internal_db_id, + privileges=[{'direct': ['SELECT'], 'role_oid': intern_role_oid}], + ) + assert [ + d['direct'] for d in after_result if d['role_oid'] == intern_role_oid + ][0] == ['SELECT'] + + +def test_intern_can_now_access_books_table(intern_rpc_call): + records = intern_rpc_call( + 'records.list', + table_oid=books_oid, + database_id=internal_db_id, + limit=20, + ) + assert records['count'] == 1410 + assert len(records['results']) == 20 From 7d8693c3e7872f24260132ece934bab78217dcca Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Wed, 13 Nov 2024 17:40:51 +0800 Subject: [PATCH 008/122] finish explanation of scenario --- api_tests/test_happy_db_setups.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api_tests/test_happy_db_setups.py b/api_tests/test_happy_db_setups.py index 2c6b642c94..176ab73e53 100644 --- a/api_tests/test_happy_db_setups.py +++ b/api_tests/test_happy_db_setups.py @@ -153,7 +153,8 @@ def test_get_current_role(admin_rpc_call): # Now the scenario proper starts. The admin will add an `intern` user to # Mathesar, an `intern` role to the internal `mathesar` DB, then grant # `CONNECT` to that role on the DB, `USAGE` on "Library Management", and -# `SELECT` on "Books". +# `SELECT` on "Books". We'll make sure the intern has the correct +# privileges at each step. # # We'll follow along with the API calls made by the front end. From eea1f915676943be157896bd580e6ecc763f909a Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Wed, 13 Nov 2024 18:20:38 +0800 Subject: [PATCH 009/122] fix linting issues --- api_tests/test_happy_db_setups.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/api_tests/test_happy_db_setups.py b/api_tests/test_happy_db_setups.py index 176ab73e53..10c6ae9aa9 100644 --- a/api_tests/test_happy_db_setups.py +++ b/api_tests/test_happy_db_setups.py @@ -1,6 +1,5 @@ import pytest import requests -from urllib import parse as urlparse SERVICE_HOST = 'http://mathesar-api-test-service:8000' EXTERNAL_HOST = 'mathesar-test-user-db' @@ -12,7 +11,7 @@ def admin_session(): login_payload = {"username": "admin", "password": "password"} s = requests.Session() - r = s.get(f'{SERVICE_HOST}/auth/login/') + s.get(f'{SERVICE_HOST}/auth/login/') s.headers['X-CSRFToken'] = s.cookies['csrftoken'] s.post(f'{SERVICE_HOST}/auth/login/', data=login_payload) s.headers['X-CSRFToken'] = s.cookies['csrftoken'] @@ -46,7 +45,7 @@ def intern_session(): s.headers['X-CSRFToken'] = s.cookies['csrftoken'] s.post(f'{SERVICE_HOST}/auth/login/', data=init_login_payload) s.headers['X-CSRFToken'] = s.cookies['csrftoken'] - reset_payload={ + reset_payload = { "new_password1": "myinternpass1234", "new_password2": "myinternpass1234" } @@ -178,7 +177,6 @@ def test_add_user(admin_session): ][0] - def test_intern_no_databases(intern_rpc_call): db_list = intern_rpc_call('databases.configured.list') assert db_list == [] @@ -224,7 +222,7 @@ def test_add_collaborator(admin_rpc_call): ) # Should only have the admin so far assert len(before_collaborators) == 1 - result = admin_rpc_call( + admin_rpc_call( 'collaborators.add', configured_role_id=intern_configured_role_id, database_id=internal_db_id, From f2da5f65700a5c0b4007589f0a173e0bafe219a9 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 13 Nov 2024 20:33:27 +0530 Subject: [PATCH 010/122] add utility functions for getting, listing, adding, updating and deleting users --- mathesar/utils/users.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 mathesar/utils/users.py diff --git a/mathesar/utils/users.py b/mathesar/utils/users.py new file mode 100644 index 0000000000..da5f750732 --- /dev/null +++ b/mathesar/utils/users.py @@ -0,0 +1,41 @@ +from django.db.models import F + +from mathesar.models import User + + +def get_user_info(user_id): + return User.objects.get(id=user_id) + + +def list_user_info(): + return User.objects.all() + + +def add_user(user_def): + return User.objects.create( + username=user_def["username"], + password=user_def["password"], + is_superuser=user_def["is_superuser"], + email=user_def.get("email", ""), + full_name=user_def.get("full_name", ""), + short_name=user_def.get("short_name", ""), + display_language=user_def.get("display_language", "en"), + ) + + +def update_user_info(user_id, user_info): + if not get_user_info(user_id).is_superuser: + user_info.pop("is_superuser") + User.objects.filter(id=user_id).update( + username=user_info.get("username", F("username")), + is_superuser=user_info.get("is_superuser", F("is_superuser")), + email=user_info.get("email", F("email")), + full_name=user_info.get("full_name", F("full_name")), + short_name=user_info.get("short_name", F("short_name")), + display_language=user_info.get("display_language", F("display_language")) + ) + return get_user_info(user_id) + + +def delete_user(user_id): + User.objects.get(id=user_id).delete() From cf013c1ad2bd720632024b3cdb7305018c44b17b Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 13 Nov 2024 20:33:55 +0530 Subject: [PATCH 011/122] add RPC functions for getting, listing, adding, updating and deleting users --- mathesar/rpc/users.py | 104 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 mathesar/rpc/users.py diff --git a/mathesar/rpc/users.py b/mathesar/rpc/users.py new file mode 100644 index 0000000000..a65e4d9250 --- /dev/null +++ b/mathesar/rpc/users.py @@ -0,0 +1,104 @@ +""" +Classes and functions exposed to the RPC endpoint for managing mathesar users. +""" +from typing import Optional, TypedDict +from modernrpc.core import rpc_method, REQUEST_KEY +from modernrpc.auth.basic import http_basic_auth_login_required, http_basic_auth_superuser_required + +from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions +from modernrpc.exceptions import AuthenticationFailed +from mathesar.utils.users import ( + get_user_info, + list_user_info, + add_user, + update_user_info, + delete_user +) + + +class UserInfo(TypedDict): + id: int + username: str + is_superuser: bool + email: str + full_name: str + short_name: str + display_language: str + + @classmethod + def from_model(cls, model): + return cls( + id=model.id, + username=model.username, + is_superuser=model.is_superuser, + email=model.email, + full_name=model.full_name, + short_name=model.short_name, + display_language=model.display_language + ) + + +class UserDef(TypedDict): + username: str + password: str + is_superuser: bool + email: Optional[str] + full_name: Optional[str] + short_name: Optional[str] + display_language: Optional[str] + + +class SettableUserInfo(TypedDict): + username: Optional[str] + is_superuser: Optional[bool] + email: Optional[str] + full_name: Optional[str] + short_name: Optional[str] + display_language: Optional[str] + + +@rpc_method(name="users.get") +@http_basic_auth_login_required +@handle_rpc_exceptions +def get(*, user_id: int) -> UserInfo: + user = get_user_info(user_id) + return UserInfo.from_model(user) + + +@rpc_method(name='users.list') +@http_basic_auth_login_required +@handle_rpc_exceptions +def list() -> list[UserInfo]: + users = list_user_info() + return [UserInfo.from_model(user) for user in users] + + +@rpc_method(name='users.add') +@http_basic_auth_superuser_required +@handle_rpc_exceptions +def add(*, user_def: UserDef) -> UserInfo: + user = add_user(user_def) + return UserInfo.from_model(user) + + +@rpc_method(name='users.patch') +@http_basic_auth_login_required +@handle_rpc_exceptions +def patch( + *, + user_id: int, + user_info: SettableUserInfo, + **kwargs +) -> UserInfo: + user = kwargs.get(REQUEST_KEY).user + if not (user.id == user_id or user.is_superuser): + raise AuthenticationFailed('users.patch') + updated_user_info = update_user_info(user_id, user_info) + return UserInfo.from_model(updated_user_info) + + +@rpc_method(name='users.delete') +@http_basic_auth_superuser_required +@handle_rpc_exceptions +def delete(*, user_id: int) -> None: + delete_user(user_id) From 9e0416d7e74dab676672596f303a6ea7c0c32e03 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 13 Nov 2024 20:41:25 +0530 Subject: [PATCH 012/122] reorganise endpoints --- mathesar/rpc/users.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/mathesar/rpc/users.py b/mathesar/rpc/users.py index a65e4d9250..2d44df2d00 100644 --- a/mathesar/rpc/users.py +++ b/mathesar/rpc/users.py @@ -57,6 +57,21 @@ class SettableUserInfo(TypedDict): display_language: Optional[str] +@rpc_method(name='users.add') +@http_basic_auth_superuser_required +@handle_rpc_exceptions +def add(*, user_def: UserDef) -> UserInfo: + user = add_user(user_def) + return UserInfo.from_model(user) + + +@rpc_method(name='users.delete') +@http_basic_auth_superuser_required +@handle_rpc_exceptions +def delete(*, user_id: int) -> None: + delete_user(user_id) + + @rpc_method(name="users.get") @http_basic_auth_login_required @handle_rpc_exceptions @@ -68,19 +83,11 @@ def get(*, user_id: int) -> UserInfo: @rpc_method(name='users.list') @http_basic_auth_login_required @handle_rpc_exceptions -def list() -> list[UserInfo]: +def list_() -> list[UserInfo]: users = list_user_info() return [UserInfo.from_model(user) for user in users] -@rpc_method(name='users.add') -@http_basic_auth_superuser_required -@handle_rpc_exceptions -def add(*, user_def: UserDef) -> UserInfo: - user = add_user(user_def) - return UserInfo.from_model(user) - - @rpc_method(name='users.patch') @http_basic_auth_login_required @handle_rpc_exceptions @@ -95,10 +102,3 @@ def patch( raise AuthenticationFailed('users.patch') updated_user_info = update_user_info(user_id, user_info) return UserInfo.from_model(updated_user_info) - - -@rpc_method(name='users.delete') -@http_basic_auth_superuser_required -@handle_rpc_exceptions -def delete(*, user_id: int) -> None: - delete_user(user_id) From ca3325cbf069caf5cbc94e3e9def88e0951a33c4 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Wed, 13 Nov 2024 20:41:40 +0530 Subject: [PATCH 013/122] add endpoint tests --- mathesar/tests/rpc/test_endpoints.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/mathesar/tests/rpc/test_endpoints.py b/mathesar/tests/rpc/test_endpoints.py index 183ed0de50..210a83472b 100644 --- a/mathesar/tests/rpc/test_endpoints.py +++ b/mathesar/tests/rpc/test_endpoints.py @@ -19,6 +19,7 @@ from mathesar.rpc import schemas from mathesar.rpc import servers from mathesar.rpc import tables +from mathesar.rpc import users METHODS = [ ( @@ -413,6 +414,32 @@ tables.metadata.set_, "tables.metadata.set", [user_is_authenticated] + ), + + ( + users.add, + "users.add", + [user_is_superuser] + ), + ( + users.delete, + "users.delete", + [user_is_superuser] + ), + ( + users.get, + "users.get", + [user_is_authenticated] + ), + ( + users.list_, + "users.list", + [user_is_authenticated] + ), + ( + users.patch, + "users.patch", + [user_is_authenticated] ) ] From e7e449e79d8fcbe651944713072730563ee31c95 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 02:16:28 +0530 Subject: [PATCH 014/122] manage imports in users.py --- mathesar/rpc/users.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mathesar/rpc/users.py b/mathesar/rpc/users.py index 2d44df2d00..a6d294d57e 100644 --- a/mathesar/rpc/users.py +++ b/mathesar/rpc/users.py @@ -3,7 +3,10 @@ """ from typing import Optional, TypedDict from modernrpc.core import rpc_method, REQUEST_KEY -from modernrpc.auth.basic import http_basic_auth_login_required, http_basic_auth_superuser_required +from modernrpc.auth.basic import ( + http_basic_auth_login_required, + http_basic_auth_superuser_required +) from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions from modernrpc.exceptions import AuthenticationFailed From 78a8e8c777528a61dede8e9f5da8d0913b91ed52 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 02:18:32 +0530 Subject: [PATCH 015/122] create a users const for rpc methods --- mathesar_ui/src/api/rpc/users.ts | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 mathesar_ui/src/api/rpc/users.ts diff --git a/mathesar_ui/src/api/rpc/users.ts b/mathesar_ui/src/api/rpc/users.ts new file mode 100644 index 0000000000..940f223585 --- /dev/null +++ b/mathesar_ui/src/api/rpc/users.ts @@ -0,0 +1,33 @@ +import { rpcMethodTypeContainer } from '@mathesar/packages/json-rpc-client-builder'; +import type { Language } from '@mathesar/i18n/languages/utils'; + +export interface UnsavedUser { + full_name: string | null; + email: string | null; + username: string; + password: string; + display_language: Language; +} + +/* export interface User extends Omit { + readonly id: number; + readonly is_superuser: boolean; +} */ + +interface UserInfo {} + +export const users = { + list: rpcMethodTypeContainer< + {}, + UserInfo[] + >(), + get: rpcMethodTypeContainer<{ user_id: number }, UserInfo>(), + add: rpcMethodTypeContainer< + {}, + UserInfo + >(), + delete: rpcMethodTypeContainer<{ user_id: number }, void>(), + /* patch: rpcMethodTypeContainer< + { user_id: number, user_info: } + >(), */ +} From 5197017b7cc753c10902a5bfdeb9f93bbc29051c Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 02:21:39 +0530 Subject: [PATCH 016/122] add users import in buildRpcApi --- mathesar_ui/src/api/rpc/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mathesar_ui/src/api/rpc/index.ts b/mathesar_ui/src/api/rpc/index.ts index 16510638df..933e8b2d4c 100644 --- a/mathesar_ui/src/api/rpc/index.ts +++ b/mathesar_ui/src/api/rpc/index.ts @@ -13,6 +13,7 @@ import { roles } from './roles'; import { schemas } from './schemas'; import { servers } from './servers'; import { tables } from './tables'; +import { users } from './users'; /** Mathesar's JSON-RPC API */ export const api = buildRpcApi({ @@ -30,5 +31,6 @@ export const api = buildRpcApi({ schemas, servers, tables, + users, }, }); From 243cbab2f300a003148785293371fdffa03ec1d6 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 02:23:24 +0530 Subject: [PATCH 017/122] transition users delete endpoint to RPC in the frontend --- mathesar_ui/src/stores/users.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mathesar_ui/src/stores/users.ts b/mathesar_ui/src/stores/users.ts index 631eadbc22..fb07b4e9ab 100644 --- a/mathesar_ui/src/stores/users.ts +++ b/mathesar_ui/src/stores/users.ts @@ -3,6 +3,7 @@ import { getContext, setContext } from 'svelte'; import { type Writable, get, writable } from 'svelte/store'; +import { api } from '@mathesar/api/rpc'; import userApi, { type UnsavedUser, type User } from '@mathesar/api/rest/users'; import type { RequestStatus } from '@mathesar/api/rest/utils/requestUtils'; import { getErrorMessage } from '@mathesar/utils/errors'; @@ -128,7 +129,7 @@ class WritableUsersStore { this.requestStatus.set({ state: 'processing', }); - await userApi.delete(userId); + await api.users.delete({ user_id: userId }).run(); this.users.update((users) => users.filter((user) => user.id !== userId)); this.count.update((count) => count - 1); this.requestStatus.set({ From 171ac96a88c102049a6f15537b879e01e8a493dd Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 03:15:06 +0530 Subject: [PATCH 018/122] use users.add RPC endpoint on the frontend --- mathesar_ui/src/api/rpc/users.ts | 15 ++++++--------- .../src/systems/users/UserDetailsForm.svelte | 11 +++++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/mathesar_ui/src/api/rpc/users.ts b/mathesar_ui/src/api/rpc/users.ts index 940f223585..5983355bb2 100644 --- a/mathesar_ui/src/api/rpc/users.ts +++ b/mathesar_ui/src/api/rpc/users.ts @@ -9,22 +9,19 @@ export interface UnsavedUser { display_language: Language; } -/* export interface User extends Omit { +export interface User extends Omit { readonly id: number; readonly is_superuser: boolean; -} */ - -interface UserInfo {} +} export const users = { list: rpcMethodTypeContainer< - {}, - UserInfo[] + void, User[] >(), - get: rpcMethodTypeContainer<{ user_id: number }, UserInfo>(), + get: rpcMethodTypeContainer<{ user_id: number }, User>(), add: rpcMethodTypeContainer< - {}, - UserInfo + {user_def: UnsavedUser}, + User >(), delete: rpcMethodTypeContainer<{ user_id: number }, void>(), /* patch: rpcMethodTypeContainer< diff --git a/mathesar_ui/src/systems/users/UserDetailsForm.svelte b/mathesar_ui/src/systems/users/UserDetailsForm.svelte index 71ae56620f..16a017e8b2 100644 --- a/mathesar_ui/src/systems/users/UserDetailsForm.svelte +++ b/mathesar_ui/src/systems/users/UserDetailsForm.svelte @@ -3,6 +3,7 @@ import { _ } from 'svelte-i18n'; import type { UnionToIntersection } from 'type-fest'; + import { api } from '@mathesar/api/rpc'; import userApi, { type User } from '@mathesar/api/rest/users'; import { extractDetailedFieldBasedErrors } from '@mathesar/api/rest/utils/errors'; import { @@ -75,10 +76,12 @@ }; if (isNewUser && hasProperty(formValues, 'password')) { - const newUser = await userApi.add({ - ...request, - password: formValues.password, - }); + const newUser = await api.users.add({ + user_def: { + ...request, + password: formValues.password, + }, + }).run(); dispatch('create', newUser); return; } From c5f64239ed401504fc62b65245ee0228d565ccab Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 23:39:19 +0530 Subject: [PATCH 019/122] fix a bug in patch logic --- mathesar/rpc/users.py | 6 +++++- mathesar/utils/users.py | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/mathesar/rpc/users.py b/mathesar/rpc/users.py index a6d294d57e..fda41ce693 100644 --- a/mathesar/rpc/users.py +++ b/mathesar/rpc/users.py @@ -103,5 +103,9 @@ def patch( user = kwargs.get(REQUEST_KEY).user if not (user.id == user_id or user.is_superuser): raise AuthenticationFailed('users.patch') - updated_user_info = update_user_info(user_id, user_info) + updated_user_info = update_user_info( + user_id, + user_info, + requesting_user=user + ) return UserInfo.from_model(updated_user_info) diff --git a/mathesar/utils/users.py b/mathesar/utils/users.py index da5f750732..151fcbe00e 100644 --- a/mathesar/utils/users.py +++ b/mathesar/utils/users.py @@ -23,9 +23,9 @@ def add_user(user_def): ) -def update_user_info(user_id, user_info): - if not get_user_info(user_id).is_superuser: - user_info.pop("is_superuser") +def update_user_info(user_id, user_info, requesting_user): + if not requesting_user.is_superuser: + user_info.pop("is_superuser", None) User.objects.filter(id=user_id).update( username=user_info.get("username", F("username")), is_superuser=user_info.get("is_superuser", F("is_superuser")), From b36ca380f3300c164c0531b8a02b0ffda84c6cb4 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 23:41:06 +0530 Subject: [PATCH 020/122] define patch rpc in frontend --- mathesar_ui/src/api/rpc/users.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/mathesar_ui/src/api/rpc/users.ts b/mathesar_ui/src/api/rpc/users.ts index 5983355bb2..75fc3fc848 100644 --- a/mathesar_ui/src/api/rpc/users.ts +++ b/mathesar_ui/src/api/rpc/users.ts @@ -18,13 +18,14 @@ export const users = { list: rpcMethodTypeContainer< void, User[] >(), - get: rpcMethodTypeContainer<{ user_id: number }, User>(), + get: rpcMethodTypeContainer<{ user_id: User['id'] }, User>(), add: rpcMethodTypeContainer< - {user_def: UnsavedUser}, + { user_def: UnsavedUser }, User >(), - delete: rpcMethodTypeContainer<{ user_id: number }, void>(), - /* patch: rpcMethodTypeContainer< - { user_id: number, user_info: } - >(), */ -} + delete: rpcMethodTypeContainer<{ user_id: User['id'] }, void>(), + patch: rpcMethodTypeContainer< + { user_id: User['id'], user_info: Partial>}, + User + >(), +}; From 3cc4f5cf3a58c9492bb2b4e924467c90fd4c169c Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 23:43:27 +0530 Subject: [PATCH 021/122] implement users.list for users stores --- mathesar_ui/src/stores/users.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/mathesar_ui/src/stores/users.ts b/mathesar_ui/src/stores/users.ts index fb07b4e9ab..2c49f6a721 100644 --- a/mathesar_ui/src/stores/users.ts +++ b/mathesar_ui/src/stores/users.ts @@ -3,9 +3,9 @@ import { getContext, setContext } from 'svelte'; import { type Writable, get, writable } from 'svelte/store'; -import { api } from '@mathesar/api/rpc'; -import userApi, { type UnsavedUser, type User } from '@mathesar/api/rest/users'; import type { RequestStatus } from '@mathesar/api/rest/utils/requestUtils'; +import { api } from '@mathesar/api/rpc'; +import { type UnsavedUser, type User } from '@mathesar/api/rpc/users'; import { getErrorMessage } from '@mathesar/utils/errors'; import type { MakeWritablePropertiesReadable } from '@mathesar/utils/typeUtils'; @@ -76,7 +76,7 @@ class WritableUsersStore { readonly count = writable(0); - private request: ReturnType | undefined; + private request: ReturnType | undefined; constructor() { void this.fetchUsers(); @@ -86,11 +86,10 @@ class WritableUsersStore { * @throws Error */ private async fetchUsersSilently() { - this.request?.cancel(); - this.request = userApi.list(); - const response = await this.request; - this.users.set(response.results.map((user) => new UserModel(user))); - this.count.set(response.count); + this.request = api.users.list(); + const response = await this.request.run(); + this.users.set(response.map((user) => new UserModel(user))); + this.count.set(response.length); } async fetchUsers() { @@ -116,8 +115,8 @@ class WritableUsersStore { return get(this.users).find((user) => user.id === userId); } if (requestStatus?.state === 'processing') { - const result = await this.request; - const user = result?.results.find((entry) => entry.id === userId); + const result = await this.request?.run(); + const user = result?.find((entry) => entry.id === userId); if (user) { return new UserModel(user); } From c8c1cf9d02a2ecc70434c9a60658854d3aef2283 Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 23:46:57 +0530 Subject: [PATCH 022/122] add users.list rpc endpoint for db settings route --- mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts b/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts index 810e4ed0d0..08621cd3bc 100644 --- a/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts +++ b/mathesar_ui/src/contexts/DatabaseSettingsRouteContext.ts @@ -1,6 +1,7 @@ import { type Readable, derived } from 'svelte/store'; -import userApi, { type User } from '@mathesar/api/rest/users'; +import { api } from '@mathesar/api/rpc'; +import { type User } from '@mathesar/api/rpc/users'; import type { Collaborator } from '@mathesar/models/Collaborator'; import type { ConfiguredRole } from '@mathesar/models/ConfiguredRole'; import type { Database } from '@mathesar/models/Database'; @@ -21,14 +22,14 @@ export type CombinedLoginRole = { // TODO: Make CancellablePromise chainable const getUsersPromise = () => { - const promise = userApi.list(); + const promise = api.users.list().run(); return new CancellablePromise>( (resolve, reject) => { promise .then( (response) => resolve( - new ImmutableMap(response.results.map((user) => [user.id, user])), + new ImmutableMap(response.map((user) => [user.id, user])), ), (err) => reject(err), ) From c24c2f454b19bc7e7a1be577c7db1540d37dc3cc Mon Sep 17 00:00:00 2001 From: Anish Umale Date: Thu, 14 Nov 2024 23:47:33 +0530 Subject: [PATCH 023/122] update imports and reorder them --- mathesar_ui/src/pages/admin-users/NewUserPage.svelte | 2 +- .../settings/collaborators/AddCollaboratorModal.svelte | 2 +- .../collaborators/EditRoleForCollaboratorModal.svelte | 2 +- mathesar_ui/src/stores/userProfile.ts | 2 +- mathesar_ui/src/systems/users/UserDetailsForm.svelte | 6 +++--- mathesar_ui/src/utils/preloadData.ts | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mathesar_ui/src/pages/admin-users/NewUserPage.svelte b/mathesar_ui/src/pages/admin-users/NewUserPage.svelte index 9d17a8b589..778e02508c 100644 --- a/mathesar_ui/src/pages/admin-users/NewUserPage.svelte +++ b/mathesar_ui/src/pages/admin-users/NewUserPage.svelte @@ -2,7 +2,7 @@ import { _ } from 'svelte-i18n'; import { router } from 'tinro'; - import type { User } from '@mathesar/api/rest/users'; + import type { User } from '@mathesar/api/rpc/users'; import AppendBreadcrumb from '@mathesar/components/breadcrumb/AppendBreadcrumb.svelte'; import FormBox from '@mathesar/components/form/FormBox.svelte'; import { iconAddUser } from '@mathesar/icons'; diff --git a/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte b/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte index 0be74c4447..df0f53275d 100644 --- a/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte +++ b/mathesar_ui/src/pages/database/settings/collaborators/AddCollaboratorModal.svelte @@ -1,7 +1,7 @@ From 5f08eeb9b7e9600fbe922424b33657029b821e11 Mon Sep 17 00:00:00 2001 From: TalhaZaheer1 Date: Mon, 2 Dec 2024 10:26:48 +0500 Subject: [PATCH 096/122] Fix double-click on selected FK cell From f40d57e2787f2afac237e6ecdfb80b5e7c4dcb6a Mon Sep 17 00:00:00 2001 From: TalhaZaheer1 Date: Mon, 2 Dec 2024 10:37:16 +0500 Subject: [PATCH 097/122] Fix double-click on selected FK cell --- .../src/systems/record-selector/RecordSelectorWindow.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte b/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte index f4f950dca5..1eb313dd6f 100644 --- a/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte +++ b/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte @@ -69,12 +69,12 @@ return false; // Parent element not found in the hierarchy } - // Prevents the RecordSelector Modal from closing for 500ms + // Prevents the RecordSelector Modal from closing for 300ms // after mounting to preserve double click behaviour onMount(() => { setTimeout(() => { controllerCanCancel = true; - }, 500); + }, 300); }); From ed9278ef5e9e38444b30b8f8fd8d0d5973b0891d Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 2 Dec 2024 16:41:37 +0800 Subject: [PATCH 098/122] add cached analytics running wrapper --- mathesar/analytics.py | 37 ++++++++++++++++++++++++++++++++++++- mathesar/rpc/decorators.py | 3 ++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/mathesar/analytics.py b/mathesar/analytics.py index e585e40622..be343aed69 100644 --- a/mathesar/analytics.py +++ b/mathesar/analytics.py @@ -7,8 +7,11 @@ Thus, the `disable_analytics` function simply deletes that ID, if it exists. """ +from functools import wraps +import threading from uuid import uuid4 +from django.core.cache import cache from django.conf import settings from django.utils import timezone import requests @@ -23,6 +26,30 @@ User, ) +ANALYTICS_DONE = "analytics_done" + + +def wire_analytics(f): + @wraps(f) + def wrapped(*args, **kwargs): + if cache.get(ANALYTICS_DONE) is None: + cache.set(ANALYTICS_DONE, True, 10) + threading.Thread(target=run_analytics).start() + return f(*args, **kwargs) + return wrapped + + +def run_analytics(): + if ( + InstallationID.objects.first() is not None + and not AnalyticsReport.objects.filter( + created_at__gte=timezone.now() - timezone.timedelta(days=1) + ) + ): + save_analytics_report() + upload_analytics_reports() + delete_stale_reports() + def initialize_analytics(): installation_id = InstallationID(value=uuid4()) @@ -92,4 +119,12 @@ def upload_analytics_reports(): def delete_stale_reports(): - AnalyticsReport.objects.filter(uploaded=True).delete() + # Delete uploaded analytics objects older than 2 days + AnalyticsReport.objects.filter( + uploaded=True, + created_on__gte=timezone.now() - timezone.timedelta(days=2) + ).delete() + # Delete analytics objects after some time regardless of upload status + AnalyticsReport.objects.filter( + updated_at__gte=timezone.now() - timezone.timedelta(days=30) + ).delete() diff --git a/mathesar/rpc/decorators.py b/mathesar/rpc/decorators.py index 060eda4bf1..763731ede4 100644 --- a/mathesar/rpc/decorators.py +++ b/mathesar/rpc/decorators.py @@ -4,6 +4,7 @@ http_basic_auth_superuser_required, ) from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions +from mathesar.analytics import wire_analytics def mathesar_rpc_method(*, name, auth="superuser"): @@ -24,5 +25,5 @@ def mathesar_rpc_method(*, name, auth="superuser"): raise Exception("`auth` must be 'superuser' or 'login'") def combo_decorator(f): - return rpc_method(name=name)(auth_wrap(handle_rpc_exceptions(f))) + return rpc_method(name=name)(auth_wrap(wire_analytics(handle_rpc_exceptions(f)))) return combo_decorator From b553a70f6df40f0c2049fb1bb9cd5dab6c418417 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 2 Dec 2024 16:50:13 +0800 Subject: [PATCH 099/122] increase cache time to 5 minutes --- mathesar/analytics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mathesar/analytics.py b/mathesar/analytics.py index be343aed69..6ca2d9b404 100644 --- a/mathesar/analytics.py +++ b/mathesar/analytics.py @@ -33,7 +33,7 @@ def wire_analytics(f): @wraps(f) def wrapped(*args, **kwargs): if cache.get(ANALYTICS_DONE) is None: - cache.set(ANALYTICS_DONE, True, 10) + cache.set(ANALYTICS_DONE, True, 300) threading.Thread(target=run_analytics).start() return f(*args, **kwargs) return wrapped From bb13f2f8819cc0926de371428947c48cbbfa2e66 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 3 Dec 2024 12:25:36 +0800 Subject: [PATCH 100/122] extract constants, add testmode check --- mathesar/analytics.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/mathesar/analytics.py b/mathesar/analytics.py index 6ca2d9b404..f4a8322d1c 100644 --- a/mathesar/analytics.py +++ b/mathesar/analytics.py @@ -27,13 +27,16 @@ ) ANALYTICS_DONE = "analytics_done" +CACHE_TIMEOUT = 1800 # seconds +ACTIVE_USER_DAYS = 14 +ANALYTICS_REPORT_MAX_AGE = 30 # days def wire_analytics(f): @wraps(f) def wrapped(*args, **kwargs): - if cache.get(ANALYTICS_DONE) is None: - cache.set(ANALYTICS_DONE, True, 300) + if settings.TEST is False and cache.get(ANALYTICS_DONE) is None: + cache.set(ANALYTICS_DONE, True, CACHE_TIMEOUT) threading.Thread(target=run_analytics).start() return f(*args, **kwargs) return wrapped @@ -83,7 +86,8 @@ def save_analytics_report(): mathesar_version=__version__, user_count=User.objects.filter(is_active=True).count(), active_user_count=User.objects.filter( - is_active=True, last_login__gte=timezone.now() - timezone.timedelta(days=14) + is_active=True, + last_login__gte=timezone.now() - timezone.timedelta(days=ACTIVE_USER_DAYS) ).count(), configured_role_count=ConfiguredRole.objects.count(), connected_database_count=connected_database_count, @@ -126,5 +130,5 @@ def delete_stale_reports(): ).delete() # Delete analytics objects after some time regardless of upload status AnalyticsReport.objects.filter( - updated_at__gte=timezone.now() - timezone.timedelta(days=30) + updated_at__gte=timezone.now() - timezone.timedelta(days=ANALYTICS_REPORT_MAX_AGE) ).delete() From f3f27ed39d2167680ed8baed2da57231ea46bbe5 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 3 Dec 2024 12:52:51 +0800 Subject: [PATCH 101/122] combine stale report queries for efficiency --- mathesar/analytics.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/mathesar/analytics.py b/mathesar/analytics.py index f4a8322d1c..00042d7ece 100644 --- a/mathesar/analytics.py +++ b/mathesar/analytics.py @@ -13,6 +13,7 @@ from django.core.cache import cache from django.conf import settings +from django.db.models import Q from django.utils import timezone import requests @@ -87,7 +88,8 @@ def save_analytics_report(): user_count=User.objects.filter(is_active=True).count(), active_user_count=User.objects.filter( is_active=True, - last_login__gte=timezone.now() - timezone.timedelta(days=ACTIVE_USER_DAYS) + last_login__gte=timezone.now() + - timezone.timedelta(days=ACTIVE_USER_DAYS) ).count(), configured_role_count=ConfiguredRole.objects.count(), connected_database_count=connected_database_count, @@ -123,12 +125,14 @@ def upload_analytics_reports(): def delete_stale_reports(): - # Delete uploaded analytics objects older than 2 days AnalyticsReport.objects.filter( - uploaded=True, - created_on__gte=timezone.now() - timezone.timedelta(days=2) - ).delete() - # Delete analytics objects after some time regardless of upload status - AnalyticsReport.objects.filter( - updated_at__gte=timezone.now() - timezone.timedelta(days=ANALYTICS_REPORT_MAX_AGE) + Q( + # Delete uploaded analytics objects older than 2 days + uploaded=True, + created_at__gte=timezone.now() - timezone.timedelta(days=2) + ) | Q( + # Delete analytics reports after a time regardless of upload status + updated_at__gte=timezone.now() + - timezone.timedelta(days=ANALYTICS_REPORT_MAX_AGE) + ) ).delete() From 2a157358796c7def1d78eff33fe6f7e663fd3e97 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 3 Dec 2024 16:37:26 +0800 Subject: [PATCH 102/122] extract ANALYTICS_FREQUENCY for clarity --- mathesar/analytics.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mathesar/analytics.py b/mathesar/analytics.py index 00042d7ece..d1853a7ea5 100644 --- a/mathesar/analytics.py +++ b/mathesar/analytics.py @@ -31,6 +31,7 @@ CACHE_TIMEOUT = 1800 # seconds ACTIVE_USER_DAYS = 14 ANALYTICS_REPORT_MAX_AGE = 30 # days +ANALYTICS_FREQUENCY = 1 # a report is saved at most once per day. def wire_analytics(f): @@ -47,7 +48,8 @@ def run_analytics(): if ( InstallationID.objects.first() is not None and not AnalyticsReport.objects.filter( - created_at__gte=timezone.now() - timezone.timedelta(days=1) + created_at__gte=timezone.now() + - timezone.timedelta(days=ANALYTICS_FREQUENCY) ) ): save_analytics_report() From 7fbc0fcd88c27dfa0a6c748061f6f774a672d855 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 3 Dec 2024 16:44:18 +0800 Subject: [PATCH 103/122] add basic test for db object getter --- db/sql/test_00_msar.sql | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 2287d39fad..820e4a4545 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -5907,3 +5907,21 @@ BEGIN ); END; $$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_get_object_counts() RETURNS SETOF TEXT AS $$ +DECLARE + object_counts jsonb; +BEGIN + CREATE SCHEMA anewone; + CREATE TABLE anewone.mytab (col1 text); + CREATE TABLE "12345" (bleh text, bleh2 numeric); + CREATE TABLE tableno3 (id INTEGER); + object_counts = msar.get_object_counts(); + RETURN NEXT is((object_counts ->> 'schema_count')::integer, 2); + RETURN NEXT is((object_counts ->> 'table_count')::integer, 3); + -- Can't check actual record count without a vacuum, since we just estimate based on catalog. + -- So, we just check that the expected key exists. + RETURN NEXT is(object_counts ? 'record_count', true); +END; +$$ LANGUAGE plpgsql; From e4ff3648ab2e7bd4cfb630a6a512cf6b406d04cf Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Wed, 4 Dec 2024 11:19:50 -0500 Subject: [PATCH 104/122] Configure spell-checking for MkDocs --- docs/README.md | 17 +++++++++++++++++ docs/mkdocs.yml | 11 +++++++++++ docs/requirements.txt | 1 + 3 files changed, 29 insertions(+) diff --git a/docs/README.md b/docs/README.md index dfa72d599c..1c0420d829 100644 --- a/docs/README.md +++ b/docs/README.md @@ -152,6 +152,23 @@ We have some custom code in `overrides/404.html` which is pretty weird! - We have some customizations to the version switcher which are applied within the `extra.css` file. - If you modify this, you'll need to port those modifications to all published versions so that the user experience is consistent when switching between versions. +## Spell-checking + +We use the [`mkdocs-spellcheck`](https://github.com/pawamoy/mkdocs-spellcheck) plugin to spell-check our documentation. + +- CI will fail if there are any spelling errors. + +- To check for spelling errors locally, run: + + ``` + mkdocs build --strict + ``` + + Spelling errors will be reported as warnings when building in strict mode. + +- To configure ignored words, see the `spellcheck` section in `mkdocs.yml` and refer to the [plugin docs](https://github.com/pawamoy/mkdocs-spellcheck) as needed. + +- If you happen to be writing documentation content using VS Code, we recommend the Code Spell Checker extension (id: `streetsidesoftware.code-spell-checker`). It will highlight spelling errors in real-time as you type. ## Page redirects diff --git a/docs/mkdocs.yml b/docs/mkdocs.yml index 25b24c0d98..060fe2c791 100644 --- a/docs/mkdocs.yml +++ b/docs/mkdocs.yml @@ -68,6 +68,17 @@ plugins: show_root_members_full_path: true show_source: false group_by_category: false + - spellcheck: + backends: + - codespell: + dictionaries: [clear] + known_words: + - Mathesar + ignore_code: yes + min_length: 2 + max_capital: 1 + allow_unicode: yes + strict_only: yes theme: name: material diff --git a/docs/requirements.txt b/docs/requirements.txt index d8a12071ad..82504b6f52 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -4,5 +4,6 @@ mkdocs-material==8.5.11 mkdocs-redirects==1.2.0 mkdocs-macros-plugin==0.7.0 mkdocs-placeholder-plugin==0.3.1 +mkdocs-spellcheck[codespell]==1.1.0 mkdocstrings==0.25.2 mkdocstrings-python==1.10.8 From 32961c1e8c5ff43c1099b35ac5de96415e56e369 Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Wed, 4 Dec 2024 11:38:16 -0500 Subject: [PATCH 105/122] Fix misspelled words caught by mkdocs-spellcheck --- db/sql/00_msar.sql | 2 +- db/sql/10_msar_joinable_tables.sql | 2 +- docs/docs/releases/0.1.3.md | 2 +- docs/docs/releases/0.1.4.md | 2 +- mathesar/rpc/constraints.py | 2 +- mathesar/rpc/databases/privileges.py | 2 +- mathesar/rpc/records.py | 4 ++-- mathesar/rpc/roles/base.py | 2 +- mathesar/rpc/schemas/privileges.py | 2 +- mathesar/rpc/tables/base.py | 2 +- mathesar/rpc/tables/privileges.py | 2 +- mathesar_ui/src/components/rich-text/RichText.svelte | 2 +- mathesar_ui/src/i18n/languages/en/dict.json | 2 +- 13 files changed, 14 insertions(+), 14 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 355b710c7f..4f6dc8f8c5 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -891,7 +891,7 @@ $$ LANGUAGE SQL RETURNS NULL ON NULL INPUT; CREATE OR REPLACE FUNCTION msar.get_fkey_map_table(tab_id oid) RETURNS TABLE (target_oid oid, conkey smallint, confkey smallint) AS $$/* -Generate a table mapping foreign key values from refererrer to referant tables. +Generate a table mapping foreign key values from refererrer to referent tables. Given an input table (identified by OID), we return a table with each row representing a foreign key constraint on that table. We return only single-column foreign keys, and only one per foreign key diff --git a/db/sql/10_msar_joinable_tables.sql b/db/sql/10_msar_joinable_tables.sql index 0b49dd99f1..1c86576897 100644 --- a/db/sql/10_msar_joinable_tables.sql +++ b/db/sql/10_msar_joinable_tables.sql @@ -25,7 +25,7 @@ A Foreign Key path gives the same information in a different form: ] In this form, `constraint_idN` is a foreign key constraint, and `reversed` is a boolean giving -whether to travel from referrer to referant (when False) or from referant to referrer (when True). +whether to travel from referrer to referent (when False) or from referent to referrer (when True). */ diff --git a/docs/docs/releases/0.1.3.md b/docs/docs/releases/0.1.3.md index 7218f790f2..3cedfee6f5 100644 --- a/docs/docs/releases/0.1.3.md +++ b/docs/docs/releases/0.1.3.md @@ -54,7 +54,7 @@ This release: - Properly detect identity columns _([#3125](https://github.com/centerofci/mathesar/pull/3125))_ - Wiring sql functions for links and tables _([#3130](https://github.com/centerofci/mathesar/pull/3130))_ - Tests for alter table _([#3139](https://github.com/centerofci/mathesar/pull/3139))_ -- Add constraint copying to column extration logic _([#3168](https://github.com/centerofci/mathesar/pull/3168))_ +- Add constraint copying to column extraction logic _([#3168](https://github.com/centerofci/mathesar/pull/3168))_ ### Summarization improvements diff --git a/docs/docs/releases/0.1.4.md b/docs/docs/releases/0.1.4.md index d2f2854eb9..f5f416b9e3 100644 --- a/docs/docs/releases/0.1.4.md +++ b/docs/docs/releases/0.1.4.md @@ -40,7 +40,7 @@ _[#3186](https://github.com/mathesar-foundation/mathesar/pull/3186) [#3219](http ### Text-only imports -When importing CSV data, Mathesar now gives you the option to use `TEXT` as the database type for all columns. This choice speeds up the import for larger data sets by skipping the process of guessing colum types. +When importing CSV data, Mathesar now gives you the option to use `TEXT` as the database type for all columns. This choice speeds up the import for larger data sets by skipping the process of guessing column types. ![image](https://github.com/mathesar-foundation/mathesar/assets/42411/6e0b5b1c-2e10-4e1f-8ad3-f4d99d28d8a9) diff --git a/mathesar/rpc/constraints.py b/mathesar/rpc/constraints.py index c5bd67ad1c..26abeeff74 100644 --- a/mathesar/rpc/constraints.py +++ b/mathesar/rpc/constraints.py @@ -77,7 +77,7 @@ class UniqueConstraint(TypedDict): CreatableConstraintInfo = list[Union[ForeignKeyConstraint, PrimaryKeyConstraint, UniqueConstraint]] """ -Type alias for a list of createable constraints which can be unique, primary key, or foreign key constraints. +Type alias for a list of creatable constraints which can be unique, primary key, or foreign key constraints. """ diff --git a/mathesar/rpc/databases/privileges.py b/mathesar/rpc/databases/privileges.py index 52e166f2d8..3bf5e9292d 100644 --- a/mathesar/rpc/databases/privileges.py +++ b/mathesar/rpc/databases/privileges.py @@ -18,7 +18,7 @@ class DBPrivileges(TypedDict): Attributes: role_oid: The `oid` of the role on the database server. - direct: A list of database privileges for the afforementioned role_oid. + direct: A list of database privileges for the aforementioned role_oid. """ role_oid: int direct: list[Literal['CONNECT', 'CREATE', 'TEMPORARY']] diff --git a/mathesar/rpc/records.py b/mathesar/rpc/records.py index 9ee488d52f..496188555f 100644 --- a/mathesar/rpc/records.py +++ b/mathesar/rpc/records.py @@ -89,7 +89,7 @@ class Grouping(TypedDict): Attributes: columns: The columns to be grouped by. - preproc: The preprocessing funtions to apply (if any). + preproc: The preprocessing functions to apply (if any). """ columns: list[int] preproc: list[str] @@ -123,7 +123,7 @@ class GroupingResponse(TypedDict): Attributes: columns: The columns to be grouped by. - preproc: The preprocessing funtions to apply (if any). + preproc: The preprocessing functions to apply (if any). groups: The groups applicable to the records being returned. """ columns: list[int] diff --git a/mathesar/rpc/roles/base.py b/mathesar/rpc/roles/base.py index a73a57bb28..814c530ef7 100644 --- a/mathesar/rpc/roles/base.py +++ b/mathesar/rpc/roles/base.py @@ -43,7 +43,7 @@ class RoleInfo(TypedDict): description: A description of the role members: The member roles that directly inherit the role. - Refer PostgreSQL documenation on: + Refer PostgreSQL documentation on: - [pg_roles table](https://www.postgresql.org/docs/current/view-pg-roles.html). - [Role attributes](https://www.postgresql.org/docs/current/role-attributes.html) - [Role membership](https://www.postgresql.org/docs/current/role-membership.html) diff --git a/mathesar/rpc/schemas/privileges.py b/mathesar/rpc/schemas/privileges.py index d5b6a134fa..1fe1927082 100644 --- a/mathesar/rpc/schemas/privileges.py +++ b/mathesar/rpc/schemas/privileges.py @@ -18,7 +18,7 @@ class SchemaPrivileges(TypedDict): Attributes: role_oid: The `oid` of the role. - direct: A list of schema privileges for the afforementioned role_oid. + direct: A list of schema privileges for the aforementioned role_oid. """ role_oid: int direct: list[Literal['USAGE', 'CREATE']] diff --git a/mathesar/rpc/tables/base.py b/mathesar/rpc/tables/base.py index 7fd5b03dd2..b56884addf 100644 --- a/mathesar/rpc/tables/base.py +++ b/mathesar/rpc/tables/base.py @@ -111,7 +111,7 @@ class JoinableTableRecord(TypedDict): ] In this form, `constraint_idN` is a foreign key constraint, and `reversed` is a boolean giving - whether to travel from referrer to referant (when False) or from referant to referrer (when True). + whether to travel from referrer to referent (when False) or from referent to referrer (when True). depth: Specifies how far to search for joinable tables. multiple_results: Specifies whether the path included is reversed. """ diff --git a/mathesar/rpc/tables/privileges.py b/mathesar/rpc/tables/privileges.py index d471c3d44f..402bbe852c 100644 --- a/mathesar/rpc/tables/privileges.py +++ b/mathesar/rpc/tables/privileges.py @@ -17,7 +17,7 @@ class TablePrivileges(TypedDict): Information about table privileges for a role. Attributes: role_oid: The `oid` of the role. - direct: A list of table privileges for the afforementioned role_oid. + direct: A list of table privileges for the aforementioned role_oid. """ role_oid: int direct: list[Literal['INSERT', 'SELECT', 'UPDATE', 'DELETE', 'TRUNCATE', 'REFERENCES', 'TRIGGER']] diff --git a/mathesar_ui/src/components/rich-text/RichText.svelte b/mathesar_ui/src/components/rich-text/RichText.svelte index ba245587ef..38094eb069 100644 --- a/mathesar_ui/src/components/rich-text/RichText.svelte +++ b/mathesar_ui/src/components/rich-text/RichText.svelte @@ -21,7 +21,7 @@ For example: ``` - The documenation can be found [anchorComponent](here). + The documentation can be found [anchorComponent](here). ``` The argument 'here' would be translated, and passed to the 'anchorComponent' slot. diff --git a/mathesar_ui/src/i18n/languages/en/dict.json b/mathesar_ui/src/i18n/languages/en/dict.json index 541c66129e..10b65dcaca 100644 --- a/mathesar_ui/src/i18n/languages/en/dict.json +++ b/mathesar_ui/src/i18n/languages/en/dict.json @@ -149,7 +149,7 @@ "data_explorer": "Data Explorer", "data_loss_warning_alert": "Data loss can result from changing the data type of a column. This action cannot be undone.", "data_source": "Data Source", - "data_tabular_format_help": "The data must be in tabular format (CSV, TSV etc) or JSON. See relevant [documentationLink](documenation).", + "data_tabular_format_help": "The data must be in tabular format (CSV, TSV etc) or JSON. See relevant [documentationLink](documentation).", "data_type": "Data Type", "data_type_linked_column_restricted": "The data type of this column must match the referenced column and cannot be changed.", "data_type_pk_column_restricted": "The data type of the primary key column is restricted and cannot be changed.", From f5289c69e386a438a082f7210351e094888b1cc8 Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Wed, 4 Dec 2024 15:21:33 -0500 Subject: [PATCH 106/122] Run Prettier --- .../src/systems/record-selector/RecordSelectorWindow.svelte | 3 --- 1 file changed, 3 deletions(-) diff --git a/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte b/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte index 1eb313dd6f..dbecc0a315 100644 --- a/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte +++ b/mathesar_ui/src/systems/record-selector/RecordSelectorWindow.svelte @@ -77,7 +77,6 @@ }, 300); }); - function onWindowClick(event: MouseEvent) { if ($nestedSelectorIsOpen) return; @@ -90,8 +89,6 @@ if (!isElementInside && controllerCanCancel) controller.cancel(); } - - From 7c1328a141b4e4496bc284bdb103a1a8340c3430 Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Wed, 4 Dec 2024 21:16:32 -0500 Subject: [PATCH 107/122] Fix bug on column type options form --- .../AbstractTypeControl.svelte | 4 +-- mathesar_ui/src/utils/columnUtils.ts | 31 ++++++++++++++++++- mathesar_ui/src/utils/objectUtils.ts | 14 --------- 3 files changed, 32 insertions(+), 17 deletions(-) delete mode 100644 mathesar_ui/src/utils/objectUtils.ts diff --git a/mathesar_ui/src/components/abstract-type-control/AbstractTypeControl.svelte b/mathesar_ui/src/components/abstract-type-control/AbstractTypeControl.svelte index 847a8566d8..8f9bce015f 100644 --- a/mathesar_ui/src/components/abstract-type-control/AbstractTypeControl.svelte +++ b/mathesar_ui/src/components/abstract-type-control/AbstractTypeControl.svelte @@ -4,7 +4,7 @@ import type { RequestStatus } from '@mathesar/api/rest/utils/requestUtils'; import { toast } from '@mathesar/stores/toast'; - import { objectsAreDeeplyEqual } from '@mathesar/utils/objectUtils'; + import { columnTypeOptionsAreEqual } from '@mathesar/utils/columnUtils'; import { CancelOrProceedButtonPair, createValidationContext, @@ -38,7 +38,7 @@ $: actionButtonsVisible = selectedAbstractType !== column.abstractType || selectedDbType !== column.type || - !objectsAreDeeplyEqual(savedTypeOptions, typeOptions); + !columnTypeOptionsAreEqual(savedTypeOptions, typeOptions ?? {}); let typeChangeState: RequestStatus; diff --git a/mathesar_ui/src/utils/columnUtils.ts b/mathesar_ui/src/utils/columnUtils.ts index ad2d65ab9d..a99797cea7 100644 --- a/mathesar_ui/src/utils/columnUtils.ts +++ b/mathesar_ui/src/utils/columnUtils.ts @@ -1,4 +1,4 @@ -import type { Column } from '@mathesar/api/rpc/columns'; +import type { Column, ColumnTypeOptions } from '@mathesar/api/rpc/columns'; import type { ConstraintType } from '@mathesar/api/rpc/constraints'; import { type ValidationFn, uniqueWith } from '@mathesar/components/form'; import { iconConstraint, iconTableLink } from '@mathesar/icons'; @@ -66,3 +66,32 @@ export function getColumnConstraintTypeByColumnId( ); return constraintsType; } + +export function columnTypeOptionsAreEqual( + a: ColumnTypeOptions, + b: ColumnTypeOptions, +): boolean { + type TypeOption = keyof ColumnTypeOptions; + // This weird object exists for type safety purposes. This way, if a new field + // is added to ColumnTypeOptions, we'll get a type error here if we don't + // update this object. + const fieldsObj: Record = { + precision: null, + scale: null, + length: null, + fields: null, + item_type: null, + }; + const fields = Object.keys(fieldsObj) as TypeOption[]; + + for (const field of fields) { + // The nullish coalescing here is important and kind of the main reason this + // function exists. We need to make sure that if a field is missing from one + // object while present but `null` in the other object, then the two objects + // are still considered equal as far as comparing column type options goes. + if ((a[field] ?? null) !== (b[field] ?? null)) { + return false; + } + } + return true; +} diff --git a/mathesar_ui/src/utils/objectUtils.ts b/mathesar_ui/src/utils/objectUtils.ts deleted file mode 100644 index 40dfb9a7ec..0000000000 --- a/mathesar_ui/src/utils/objectUtils.ts +++ /dev/null @@ -1,14 +0,0 @@ -function isObject(obj: unknown): obj is Record { - return obj !== null && typeof obj === 'object'; -} - -export function objectsAreDeeplyEqual(a: unknown, b: unknown): boolean { - if (!isObject(a) || !isObject(b)) return a === b; - - const aKeys = Object.keys(a); - const bKeys = Object.keys(b); - - if (aKeys.length !== bKeys.length) return false; - - return aKeys.every((key) => objectsAreDeeplyEqual(a[key], b[key])); -} From ea2704b3ba47f4c0f383537c8368d0ff4063c434 Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Thu, 5 Dec 2024 15:29:28 -0500 Subject: [PATCH 108/122] Improve clarity of table inspector active tab --- .../src/component-library/tabs/TabContainer.scss | 5 ++++- mathesar_ui/src/i18n/languages/en/dict.json | 4 ++++ mathesar_ui/src/icons/customIcons.ts | 12 ++++++++++++ mathesar_ui/src/icons/index.ts | 2 ++ .../src/pages/database/DatabasePageWrapper.svelte | 3 +++ mathesar_ui/src/pages/schema/SchemaPage.svelte | 3 ++- mathesar_ui/src/stores/localStorage.ts | 6 ------ .../table-inspector/column/ColumnMode.svelte | 2 +- .../column/ColumnNameAndDescription.svelte | 4 ++-- .../table-inspector/table/TableDescription.svelte | 2 +- .../table-inspector/table/TableMode.svelte | 12 +----------- .../table-inspector/table/TableName.svelte | 2 +- .../table-inspector/table/TablePermissions.svelte | 3 +++ 13 files changed, 36 insertions(+), 24 deletions(-) diff --git a/mathesar_ui/src/component-library/tabs/TabContainer.scss b/mathesar_ui/src/component-library/tabs/TabContainer.scss index 25c7186585..29ee58cdc8 100644 --- a/mathesar_ui/src/component-library/tabs/TabContainer.scss +++ b/mathesar_ui/src/component-library/tabs/TabContainer.scss @@ -23,10 +23,11 @@ list-style: none; overflow: hidden; border-radius: 2px 2px 0px 0px; - border-bottom: 2px solid transparent; + border-bottom: 0.25em solid transparent; margin-bottom: -1px; margin-right: var(--Tab_margin-right, 2rem); font-size: var(--text-size-large); + opacity: 0.75; > div, a { @@ -42,6 +43,7 @@ &:hover { border-bottom-color: var(--slate-300); + opacity: 1; } & + li.tab { @@ -50,6 +52,7 @@ &.active { border-bottom-color: var(--brand-500); + opacity: 1; } &.focused { diff --git a/mathesar_ui/src/i18n/languages/en/dict.json b/mathesar_ui/src/i18n/languages/en/dict.json index 10b65dcaca..b0ef52f7ac 100644 --- a/mathesar_ui/src/i18n/languages/en/dict.json +++ b/mathesar_ui/src/i18n/languages/en/dict.json @@ -74,6 +74,7 @@ "column": "Column", "column_added_number_of_times": "{count, plural, one {This column has been added once.} other {This column has been added {count} times.}}", "column_data_types": "Column Data Types", + "column_description": "Column Description", "column_field_options": "{columnName} Field Options", "column_from_table": "[columnName] [fromSlot](from) [tableName]", "column_id_references_column_name": "[columnId] references the column [columnName].", @@ -83,6 +84,7 @@ "column_name_cannot_be_empty": "Column name cannot be empty.", "column_names": "Column Names", "column_number_name": "Column {number} Name", + "column_properties": "Column Properties", "column_references_target_table": "The column in this table which references the target table.", "column_will_allow_duplicates": "Column {columnName} will allow duplicates.", "column_will_allow_null": "Column {columnName} will allow NULL", @@ -589,6 +591,7 @@ "table_access_read_help": "Can read table records", "table_access_write_help": "Can read, update, delete, and create table records", "table_delete_permanent_warning": "Warning: This action is permanent and once deleted, the table cannot be recovered.", + "table_description": "Table Description", "table_does_not_link": "This table does not link to any other tables", "table_name": "Table Name", "table_name_already_exists": "A table with that name already exists.", @@ -605,6 +608,7 @@ "table_privilege_trigger_help": "Allow creation of triggers on the table.", "table_privilege_truncate_help": "Allow truncation (removal of all records) of the table.", "table_privilege_update_help": "Allow updating of records within the table.", + "table_properties": "Table Properties", "table_with_name_already_exists": "A table with that name already exists.", "tables": "Tables", "tables_matching_search": "{count, plural, one {{count} table matches [searchValue]} other {{count} tables match [searchValue]}}", diff --git a/mathesar_ui/src/icons/customIcons.ts b/mathesar_ui/src/icons/customIcons.ts index 08297ce32d..8b1c9815f8 100644 --- a/mathesar_ui/src/icons/customIcons.ts +++ b/mathesar_ui/src/icons/customIcons.ts @@ -133,3 +133,15 @@ export const treeChildNodeArrowIcon: IconProps['data'] = { ], ], }; + +export const permissionsIcon: IconProps['data'] = { + icon: [ + 100, + 100, + [], + '', + [ + 'M 50 1.14 C 49.12 1.14 48.23 1.33 47.45 1.7 L 11.29 17.03 C 7.07 18.82 3.92 22.98 3.94 28.01 C 4.04 47.04 11.87 81.87 44.93 97.71 C 48.14 99.24 51.86 99.24 55.07 97.71 C 88.13 81.87 95.96 47.04 96.06 28.01 C 96.08 22.98 92.93 18.82 88.71 17.03 L 52.57 1.7 C 51.77 1.33 50.88 1.14 50 1.14 z M 50 28.19 A 15.36 15.36 0 0 1 65.36 43.55 A 15.36 15.36 0 0 1 56.37 57.53 L 56.37 74.35 A 6.36 6.36 0 0 1 50 80.72 A 6.36 6.36 0 0 1 43.63 74.35 L 43.63 57.53 A 15.36 15.36 0 0 1 34.64 43.55 A 15.36 15.36 0 0 1 50 28.19 z', + ], + ], +}; diff --git a/mathesar_ui/src/icons/index.ts b/mathesar_ui/src/icons/index.ts index 987c85524a..697b26043c 100644 --- a/mathesar_ui/src/icons/index.ts +++ b/mathesar_ui/src/icons/index.ts @@ -82,6 +82,7 @@ import { createDatabaseIcon, explorationIcon, outcomeIcon, + permissionsIcon, tableIcon, treeChildNodeArrowIcon, } from './customIcons'; @@ -194,6 +195,7 @@ export const iconUrl: IconProps = { data: faLink }; export const iconText: IconProps = { data: faT }; export const iconField: IconProps = { data: faDatabase }; export const iconFieldDelimiter: IconProps = { data: faCaretRight }; +export const iconPermissions: IconProps = { data: permissionsIcon }; // STATUSES diff --git a/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte b/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte index 5c20d303c4..c1eac63c5e 100644 --- a/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte +++ b/mathesar_ui/src/pages/database/DatabasePageWrapper.svelte @@ -9,6 +9,7 @@ iconDatabase, iconDeleteMajor, iconMoreActions, + iconPermissions, } from '@mathesar/icons'; import LayoutWithHeader from '@mathesar/layouts/LayoutWithHeader.svelte'; import { makeSimplePageTitle } from '@mathesar/pages/pageTitleUtils'; @@ -27,6 +28,7 @@ Button, ButtonMenuItem, DropdownMenu, + Icon, TabContainer, } from '@mathesar-component-library'; @@ -127,6 +129,7 @@ >
diff --git a/mathesar_ui/src/pages/schema/SchemaPage.svelte b/mathesar_ui/src/pages/schema/SchemaPage.svelte index 3f5b07b4cd..c02d5024b0 100644 --- a/mathesar_ui/src/pages/schema/SchemaPage.svelte +++ b/mathesar_ui/src/pages/schema/SchemaPage.svelte @@ -2,7 +2,7 @@ import { _ } from 'svelte-i18n'; import AppSecondaryHeader from '@mathesar/components/AppSecondaryHeader.svelte'; - import { iconEdit, iconSchema } from '@mathesar/icons'; + import { iconEdit, iconPermissions, iconSchema } from '@mathesar/icons'; import LayoutWithHeader from '@mathesar/layouts/LayoutWithHeader.svelte'; import type { Database } from '@mathesar/models/Database'; import type { Schema } from '@mathesar/models/Schema'; @@ -111,6 +111,7 @@ {$_('edit_schema')}
diff --git a/mathesar_ui/src/stores/localStorage.ts b/mathesar_ui/src/stores/localStorage.ts index 65bcd787b9..422fdcbd2d 100644 --- a/mathesar_ui/src/stores/localStorage.ts +++ b/mathesar_ui/src/stores/localStorage.ts @@ -8,7 +8,6 @@ export const LOCAL_STORAGE_KEYS = { tableInspectorVisible: 'table-inspector-visible', tableInspectorWidth: 'table-inspector-width', tableInspectorTablePropertiesVisible: 'table-inspector-table-properties-visible', - tableInspectorTablePermissionsVisible: 'table-inspector-table-permissions-visible', tableInspectorTableLinksVisible: 'table-inspector-table-links-visible', tableInspectorTableRecordSummaryVisible: 'table-inspector-table-record-summary-visible', tableInspectorTableActionsVisible: 'table-inspector-table-actions-visible', @@ -40,11 +39,6 @@ export const tableInspectorTablePropertiesVisible = new LocalStorageStore({ defaultValue: true, }); -export const tableInspectorTablePermissionsVisible = new LocalStorageStore({ - key: LOCAL_STORAGE_KEYS.tableInspectorTablePermissionsVisible, - defaultValue: true, -}); - export const tableInspectorTableLinksVisible = new LocalStorageStore({ key: LOCAL_STORAGE_KEYS.tableInspectorTableLinksVisible, defaultValue: true, diff --git a/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnMode.svelte b/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnMode.svelte index 91a33aff98..38409b7435 100644 --- a/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnMode.svelte +++ b/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnMode.svelte @@ -76,7 +76,7 @@ >
diff --git a/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnNameAndDescription.svelte b/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnNameAndDescription.svelte index 66b9e69ad7..cc938b6b0b 100644 --- a/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnNameAndDescription.svelte +++ b/mathesar_ui/src/systems/table-view/table-inspector/column/ColumnNameAndDescription.svelte @@ -51,7 +51,7 @@
- {$_('name')} + {$_('column_name')}
- {$_('description')} + {$_('column_description')}
- {$_('description')} + {$_('table_description')}
-
- - - - -
diff --git a/mathesar_ui/src/systems/table-view/table-inspector/table/TableName.svelte b/mathesar_ui/src/systems/table-view/table-inspector/table/TableName.svelte index e7256fd1c0..da1bb23880 100644 --- a/mathesar_ui/src/systems/table-view/table-inspector/table/TableName.svelte +++ b/mathesar_ui/src/systems/table-view/table-inspector/table/TableName.svelte @@ -27,7 +27,7 @@
- {$_('name')} + {$_('table_name')} controller.open()} size="small" > + {$_('table_permissions')}
From 9c1981136be026d68b091b5fbeddb8cc7a1fcc0f Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 6 Dec 2024 15:09:49 +0800 Subject: [PATCH 109/122] add tests for analytics functions --- conftest.py | 8 +++ mathesar/analytics.py | 7 +- mathesar/tests/test_analytics.py | 112 +++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 mathesar/tests/test_analytics.py diff --git a/conftest.py b/conftest.py index 41965bae8f..1b88819355 100644 --- a/conftest.py +++ b/conftest.py @@ -1,5 +1,6 @@ import pytest import random +import responses import string import os import psycopg @@ -39,6 +40,13 @@ def _get(db_name): logger.debug('exit') +@pytest.fixture(autouse=True) +def disable_http_requests(monkeypatch): + def mock_urlopen(self, method, url, *args, **kwargs): + raise Exception("Requests to 3rd party addresses make bad tests") + monkeypatch.setattr("urllib3.connectionpool.HTTPConnectionPool.urlopen", mock_urlopen) + + @pytest.fixture(scope="session") def create_db(request, engine_cache): """ diff --git a/mathesar/analytics.py b/mathesar/analytics.py index d1853a7ea5..d4d0f5400b 100644 --- a/mathesar/analytics.py +++ b/mathesar/analytics.py @@ -58,8 +58,7 @@ def run_analytics(): def initialize_analytics(): - installation_id = InstallationID(value=uuid4()) - installation_id.save() + InstallationID.objects.create(value=uuid4()) def disable_analytics(): @@ -131,10 +130,10 @@ def delete_stale_reports(): Q( # Delete uploaded analytics objects older than 2 days uploaded=True, - created_at__gte=timezone.now() - timezone.timedelta(days=2) + created_at__lte=timezone.now() - timezone.timedelta(days=2) ) | Q( # Delete analytics reports after a time regardless of upload status - updated_at__gte=timezone.now() + updated_at__lte=timezone.now() - timezone.timedelta(days=ANALYTICS_REPORT_MAX_AGE) ) ).delete() diff --git a/mathesar/tests/test_analytics.py b/mathesar/tests/test_analytics.py new file mode 100644 index 0000000000..a91272e9c4 --- /dev/null +++ b/mathesar/tests/test_analytics.py @@ -0,0 +1,112 @@ +""" +Test analytics functions. + +Fixtures: + monkeypatch(pytest): Lets you monkeypatch an object for testing. + settings(pytest): Modify a django setting for a given test +""" +import time +from unittest.mock import patch +from django.core.cache import cache +from django.db import IntegrityError +import pytest +import requests +from mathesar import analytics +from mathesar.models.analytics import AnalyticsReport, InstallationID +from mathesar.models.base import Database, Server + + +@pytest.fixture(autouse=True) +def clear_cache(): + cache.clear() + + +def test_non_blocking_wrapper(monkeypatch, settings): + """ + A bit messy. Tests that the decorated function exits without + waiting for the sleep command. + """ + settings.TEST = False + + def mock_analytics_runner(): + time.sleep(0.5) + monkeypatch.setattr(analytics, "run_analytics", mock_analytics_runner) + + @analytics.wire_analytics + def myfunc(): + return + start = time.time() + myfunc() + end = time.time() + assert end - start < 0.1 + + +def test_initialize_save_disable_analytics(): + assert len(InstallationID.objects.all()) == 0 + analytics.initialize_analytics() + assert len(InstallationID.objects.all()) == 1 + analytics.save_analytics_report() + assert len(AnalyticsReport.objects.all()) == 1 + analytics.disable_analytics() + assert len(InstallationID.objects.all()) == 0 + assert len(AnalyticsReport.objects.all()) == 0 + + +def test_analytics_installation_id_nongenerated_pkey(): + analytics.initialize_analytics() + with pytest.raises(IntegrityError): + analytics.initialize_analytics() + + +def test_analytics_upload_delete_stale(settings, monkeypatch): + server = Server.objects.create(host='example.com', port=5432) + Database.objects.create(name='db1', server=server) + analytics.initialize_analytics() + installation_id = InstallationID.objects.first() + AnalyticsReport.objects.create( + installation_id=installation_id, + mathesar_version='123abc', + user_count=3, + active_user_count=2, + configured_role_count=5, + connected_database_count=2, + connected_database_schema_count=10, + connected_database_table_count=50, + connected_database_record_count=10000, + exploration_count=5, + ) + AnalyticsReport.objects.create( + installation_id=installation_id, + mathesar_version='123abc', + user_count=4, + active_user_count=3, + configured_role_count=6, + connected_database_count=3, + connected_database_schema_count=20, + connected_database_table_count=60, + connected_database_record_count=20000, + exploration_count=10, + ) + # This should be the default, but it's good to check + assert len(AnalyticsReport.objects.filter(uploaded=False)) == 2 + with patch.object(analytics.requests, 'post') as mock_requests: + analytics.upload_analytics_reports() + mock_requests.assert_called_once() + call_args = mock_requests.call_args + assert call_args.args == (settings.MATHESAR_ANALYTICS_URL,) + assert set(call_args.kwargs.keys()) == set(['json']) + reports_blob = sorted(call_args.kwargs['json'], key=lambda d: d['id']) + assert all([d['installation_id'] == str(installation_id.value) for d in reports_blob]) + assert all([d['mathesar_version'] == '123abc' for d in reports_blob]) + assert [d['user_count'] for d in reports_blob] == [3, 4] + assert [d['active_user_count'] for d in reports_blob] == [2, 3] + assert [d['configured_role_count'] for d in reports_blob] == [5, 6] + assert [d['connected_database_count'] for d in reports_blob] == [2, 3] + assert [d['connected_database_schema_count'] for d in reports_blob] == [10, 20] + assert [d['connected_database_table_count'] for d in reports_blob] == [50, 60] + assert [d['connected_database_record_count'] for d in reports_blob] == [10000, 20000] + assert [d['exploration_count'] for d in reports_blob] == [5, 10] + assert len(AnalyticsReport.objects.filter(uploaded=True)) == 2 + analytics.delete_stale_reports() + assert len(AnalyticsReport.objects.all()) == 2 + AnalyticsReport.objects.all().update() From aad50ce7dcd122dcc9f1bf1ee62dcbbb23fe90d1 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 6 Dec 2024 15:21:11 +0800 Subject: [PATCH 110/122] remove unused imports --- conftest.py | 1 - mathesar/tests/test_analytics.py | 1 - 2 files changed, 2 deletions(-) diff --git a/conftest.py b/conftest.py index 1b88819355..4132a6191f 100644 --- a/conftest.py +++ b/conftest.py @@ -1,6 +1,5 @@ import pytest import random -import responses import string import os import psycopg diff --git a/mathesar/tests/test_analytics.py b/mathesar/tests/test_analytics.py index a91272e9c4..0f31b2d949 100644 --- a/mathesar/tests/test_analytics.py +++ b/mathesar/tests/test_analytics.py @@ -10,7 +10,6 @@ from django.core.cache import cache from django.db import IntegrityError import pytest -import requests from mathesar import analytics from mathesar.models.analytics import AnalyticsReport, InstallationID from mathesar.models.base import Database, Server From c5c0615ca86b55fd01799d82c541186b953f6cd5 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 6 Dec 2024 15:30:23 +0800 Subject: [PATCH 111/122] remove unused named variables --- conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 4132a6191f..42d8d9e784 100644 --- a/conftest.py +++ b/conftest.py @@ -41,7 +41,7 @@ def _get(db_name): @pytest.fixture(autouse=True) def disable_http_requests(monkeypatch): - def mock_urlopen(self, method, url, *args, **kwargs): + def mock_urlopen(self, *args, **kwargs): raise Exception("Requests to 3rd party addresses make bad tests") monkeypatch.setattr("urllib3.connectionpool.HTTPConnectionPool.urlopen", mock_urlopen) From 0a65135f54303abb4e5a855dfd4f7bc3e6da0905 Mon Sep 17 00:00:00 2001 From: pavish Date: Fri, 6 Dec 2024 20:46:11 +0530 Subject: [PATCH 112/122] Sort schemas in schema page and breadcrumb selector --- db/sql/00_msar.sql | 3 ++- .../breadcrumb/SchemaSelector.svelte | 2 +- .../database/schemas/SchemasSection.svelte | 2 +- mathesar_ui/src/stores/schemas.ts | 26 ++++++++++++++++++- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 4f6dc8f8c5..f8067aebbf 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -1106,7 +1106,8 @@ LEFT JOIN pg_catalog.pg_class c ON c.relnamespace = s.oid AND c.relkind = 'r' GROUP BY s.oid, s.nspname, - s.nspowner; + s.nspowner +ORDER BY s.nspname; -- Filter on relkind so that we only count tables. This must be done in the ON clause so that -- we still get a row for schemas with no tables. $$ LANGUAGE SQL STABLE; diff --git a/mathesar_ui/src/components/breadcrumb/SchemaSelector.svelte b/mathesar_ui/src/components/breadcrumb/SchemaSelector.svelte index fb7a15ad54..b7cfe908de 100644 --- a/mathesar_ui/src/components/breadcrumb/SchemaSelector.svelte +++ b/mathesar_ui/src/components/breadcrumb/SchemaSelector.svelte @@ -8,7 +8,7 @@ import { getSchemaPageUrl } from '@mathesar/routes/urls'; import { currentSchemaId, - schemas as schemasStore, + sortedSchemas as schemasStore, } from '@mathesar/stores/schemas'; import BreadcrumbSelector from './BreadcrumbSelector.svelte'; diff --git a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte index a911f24042..fb061b9c9b 100644 --- a/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte +++ b/mathesar_ui/src/pages/database/schemas/SchemasSection.svelte @@ -13,7 +13,7 @@ import { type SchemaStoreData, deleteSchema as deleteSchemaAPI, - schemas as schemasStore, + sortedSchemas as schemasStore, } from '@mathesar/stores/schemas'; import AddEditSchemaModal from '@mathesar/systems/schemas/AddEditSchemaModal.svelte'; import { diff --git a/mathesar_ui/src/stores/schemas.ts b/mathesar_ui/src/stores/schemas.ts index a3a231a1c9..0c7bd8f3d3 100644 --- a/mathesar_ui/src/stores/schemas.ts +++ b/mathesar_ui/src/stores/schemas.ts @@ -13,7 +13,11 @@ import type { Database } from '@mathesar/models/Database'; import { Schema } from '@mathesar/models/Schema'; import { getErrorMessage } from '@mathesar/utils/errors'; import { preloadCommonData } from '@mathesar/utils/preloadData'; -import { type CancellablePromise, collapse } from '@mathesar-component-library'; +import { + type CancellablePromise, + collapse, + unite, +} from '@mathesar-component-library'; import { databasesStore } from './databases'; @@ -175,6 +179,26 @@ export const schemas = collapse( }), ); +function sortSchemas(_schemas: Iterable): Schema[] { + return [..._schemas].sort((a, b) => get(a.name).localeCompare(get(b.name))); +} + +export const schemaNames = collapse( + derived(schemas, ($schemas) => + unite([...$schemas.data.values()].map((s) => s.name)), + ), +); + +export const sortedSchemas = derived([schemas, schemaNames], ([$schemas]) => { + const schemasMap = new Map( + sortSchemas($schemas.data.values()).map((s) => [s.oid, s]), + ); + return { + ...$schemas, + data: schemasMap, + }; +}); + export const currentSchema: Readable = derived( [currentSchemaId, schemas], ([$currentSchemaId, $schemas]) => From 82071285175b06dd1bba4970211667b821f8647e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 6 Dec 2024 18:59:34 +0000 Subject: [PATCH 113/122] Bump django from 4.2.16 to 4.2.17 Bumps [django](https://github.com/django/django) from 4.2.16 to 4.2.17. - [Commits](https://github.com/django/django/compare/4.2.16...4.2.17) --- updated-dependencies: - dependency-name: django dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b454166aed..caba1bd593 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ clevercsv==0.8.2 -Django==4.2.16 +Django==4.2.17 dj-database-url==2.3.0 django-filter==24.3 django-modern-rpc==1.0.3 From a952f228cfa674b76340716facb4331849f98d7b Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 9 Dec 2024 15:53:45 +0800 Subject: [PATCH 114/122] clean up migrations --- .../0022_installationid_analyticsreport.py | 7 ++--- ...sreport_recent_ddl_query_count_and_more.py | 30 ------------------- .../0024_analyticsreport_uploaded.py | 18 ----------- 3 files changed, 3 insertions(+), 52 deletions(-) delete mode 100644 mathesar/migrations/0023_remove_analyticsreport_recent_ddl_query_count_and_more.py delete mode 100644 mathesar/migrations/0024_analyticsreport_uploaded.py diff --git a/mathesar/migrations/0022_installationid_analyticsreport.py b/mathesar/migrations/0022_installationid_analyticsreport.py index 7a72117018..eb5d06dfb4 100644 --- a/mathesar/migrations/0022_installationid_analyticsreport.py +++ b/mathesar/migrations/0022_installationid_analyticsreport.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.16 on 2024-11-25 04:36 +# Generated by Django 4.2.16 on 2024-12-09 07:39 from django.db import migrations, models import django.db.models.deletion @@ -36,10 +36,9 @@ class Migration(migrations.Migration): ('connected_database_count', models.PositiveIntegerField(blank=True, null=True)), ('connected_database_schema_count', models.PositiveIntegerField(blank=True, null=True)), ('connected_database_table_count', models.PositiveIntegerField(blank=True, null=True)), + ('connected_database_record_count', models.PositiveBigIntegerField(blank=True, null=True)), ('exploration_count', models.PositiveIntegerField(blank=True, null=True)), - ('recent_dql_query_count', models.PositiveIntegerField(blank=True, null=True)), - ('recent_dml_query_count', models.PositiveIntegerField(blank=True, null=True)), - ('recent_ddl_query_count', models.PositiveIntegerField(blank=True, null=True)), + ('uploaded', models.BooleanField(default=False)), ('installation_id', models.ForeignKey(default=1, on_delete=django.db.models.deletion.CASCADE, to='mathesar.installationid')), ], options={ diff --git a/mathesar/migrations/0023_remove_analyticsreport_recent_ddl_query_count_and_more.py b/mathesar/migrations/0023_remove_analyticsreport_recent_ddl_query_count_and_more.py deleted file mode 100644 index d31e6a0d29..0000000000 --- a/mathesar/migrations/0023_remove_analyticsreport_recent_ddl_query_count_and_more.py +++ /dev/null @@ -1,30 +0,0 @@ -# Generated by Django 4.2.16 on 2024-11-26 04:11 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('mathesar', '0022_installationid_analyticsreport'), - ] - - operations = [ - migrations.RemoveField( - model_name='analyticsreport', - name='recent_ddl_query_count', - ), - migrations.RemoveField( - model_name='analyticsreport', - name='recent_dml_query_count', - ), - migrations.RemoveField( - model_name='analyticsreport', - name='recent_dql_query_count', - ), - migrations.AddField( - model_name='analyticsreport', - name='connected_database_record_count', - field=models.PositiveBigIntegerField(blank=True, null=True), - ), - ] diff --git a/mathesar/migrations/0024_analyticsreport_uploaded.py b/mathesar/migrations/0024_analyticsreport_uploaded.py deleted file mode 100644 index 6cd90d8a28..0000000000 --- a/mathesar/migrations/0024_analyticsreport_uploaded.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.2.16 on 2024-11-27 06:44 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('mathesar', '0023_remove_analyticsreport_recent_ddl_query_count_and_more'), - ] - - operations = [ - migrations.AddField( - model_name='analyticsreport', - name='uploaded', - field=models.BooleanField(default=False), - ), - ] From 14af511c91aa8369aacc554ab4718793679bd6d3 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 10 Dec 2024 21:25:53 +0800 Subject: [PATCH 115/122] Use conventional aliases in query Co-authored-by: Anish Umale --- db/sql/00_msar.sql | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index fb8e10bb5c..64563da1ba 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -1105,15 +1105,15 @@ The objects counted are: - total rows of tables included */ SELECT jsonb_build_object( - 'schema_count', COUNT(DISTINCT s.oid), - 'table_count', COUNT(c.oid), - 'record_count', SUM(c.reltuples) + 'schema_count', COUNT(DISTINCT pgn.oid), + 'table_count', COUNT(pgc.oid), + 'record_count', SUM(pgc.reltuples) ) -FROM pg_catalog.pg_namespace s -LEFT JOIN pg_catalog.pg_class c ON c.relnamespace = s.oid AND c.relkind = 'r' -WHERE s.nspname <> 'information_schema' -AND NOT (s.nspname = ANY(msar.mathesar_system_schemas())) -AND s.nspname NOT LIKE 'pg_%'; +FROM pg_catalog.pg_namespace pgn +LEFT JOIN pg_catalog.pg_class pgc ON pgc.relnamespace = pgn.oid AND pgc.relkind = 'r' +WHERE pgn.nspname <> 'information_schema' +AND NOT (pgn.nspname = ANY(msar.mathesar_system_schemas())) +AND pgn.nspname NOT LIKE 'pg_%'; $$ LANGUAGE SQL STABLE; From f6b63781546c1a363e4388099d82433bf8f0b54e Mon Sep 17 00:00:00 2001 From: zackdotcat Date: Tue, 10 Dec 2024 17:29:50 -0500 Subject: [PATCH 116/122] Rename references to mathesar/mathesar-ansible repository to mathesar/mathesar-infrastructure --- .github/sync.yml | 2 +- .github/workflows/staging-deploy.yml | 2 +- .github/workflows/sync-github-labels-milestones.yml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/sync.yml b/.github/sync.yml index 9de822878f..1a5178c85e 100644 --- a/.github/sync.yml +++ b/.github/sync.yml @@ -1,4 +1,4 @@ -mathesar-foundation/mathesar-ansible: +mathesar-foundation/mathesar-infrastructure: - .github/workflows/toc.yml - .github/workflows/stale.yml mathesar-foundation/mathesar-data-playground: diff --git a/.github/workflows/staging-deploy.yml b/.github/workflows/staging-deploy.yml index d5d170fff3..18b201fd79 100644 --- a/.github/workflows/staging-deploy.yml +++ b/.github/workflows/staging-deploy.yml @@ -11,7 +11,7 @@ jobs: - name: Checkout ansible repo uses: actions/checkout@v2 with: - repository: 'mathesar-foundation/mathesar-ansible' + repository: 'mathesar-foundation/mathesar-infrastructure' token: ${{ secrets.MATHESAR_ORG_GITHUB_TOKEN }} # Repo is private, so an access token is used # This checkout is used for getting the 'action' from the current repo - name: Checkout mathesar repo diff --git a/.github/workflows/sync-github-labels-milestones.yml b/.github/workflows/sync-github-labels-milestones.yml index 4b55fba303..190b31db6c 100644 --- a/.github/workflows/sync-github-labels-milestones.yml +++ b/.github/workflows/sync-github-labels-milestones.yml @@ -13,7 +13,7 @@ jobs: steps: - uses: actions/checkout@v2 - run: composer global require 'vanilla/github-sync' - - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-ansible -d + - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-infrastructure -d - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-data-playground -d - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-design -d - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-internal-crm -d @@ -21,7 +21,7 @@ jobs: - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-scripts -d - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-website -d - run: /home/runner/.composer/vendor/bin/github-sync labels -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-wiki -d - - run: /home/runner/.composer/vendor/bin/github-sync milestones -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-ansible -s open + - run: /home/runner/.composer/vendor/bin/github-sync milestones -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-infrastructure -s open - run: /home/runner/.composer/vendor/bin/github-sync milestones -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-data-playground -s open - run: /home/runner/.composer/vendor/bin/github-sync milestones -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-design -s open - run: /home/runner/.composer/vendor/bin/github-sync milestones -f mathesar-foundation/mathesar -t mathesar-foundation/mathesar-internal-crm -s open From 0382f4b331a9dffff0a2b8479045805b84012027 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Thu, 12 Dec 2024 23:25:26 +0800 Subject: [PATCH 117/122] extract default describing function, add execution logic --- db/sql/00_msar.sql | 55 ++++++++++++++++++++++++++++++----------- db/sql/test_00_msar.sql | 2 +- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 64563da1ba..b217227b6a 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -934,6 +934,46 @@ WHERE has_privilege; $$ LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT; +CREATE OR REPLACE FUNCTION +msar.describe_column_default(tab_id regclass, col_id smallint) RETURNS jsonb AS $$/* +Return a JSONB object describing the default (if any) of the given column in the given table. + +The returned JSON will have the form: + { + "value": , + "is_dynamic": , + } + +If the default is possibly dynamic, i.e., if "is_dynamic" is true, then "value" will be a text SQL +expression that generates the default value if evaluated. If it is not dynamic, then "value" is the +actual default value. +*/ +DECLARE + def_expr text; + def_json jsonb; +BEGIN +def_expr = CASE + WHEN attidentity='' THEN pg_catalog.pg_get_expr(adbin, tab_id) + ELSE 'identity' +END +FROM pg_catalog.pg_attribute LEFT JOIN pg_catalog.pg_attrdef ON attrelid=adrelid AND attnum=adnum +WHERE attrelid=tab_id AND attnum=col_id; +IF def_expr IS NULL THEN + RETURN NULL; +ELSIF msar.is_default_possibly_dynamic(tab_id, col_id) THEN + EXECUTE format( + 'SELECT jsonb_build_object(''value'', %L, ''is_dynamic'', true)', def_expr + ) INTO def_json; +ELSE + EXECUTE format( + 'SELECT jsonb_build_object(''value'', msar.format_data(%s), ''is_dynamic'', false)', def_expr + ) INTO def_json; +END IF; +RETURN def_json; +END; +$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT; + + CREATE OR REPLACE FUNCTION msar.get_column_info(tab_id regclass) RETURNS jsonb AS $$/* Given a table identifier, return an array of objects describing the columns of the table. @@ -965,20 +1005,7 @@ SELECT jsonb_agg( 'type_options', msar.get_type_options(atttypid, atttypmod, attndims), 'nullable', NOT attnotnull, 'primary_key', COALESCE(pgi.indisprimary, false), - 'default', - nullif( - jsonb_strip_nulls( - jsonb_build_object( - 'value', - CASE - WHEN attidentity='' THEN pg_get_expr(adbin, tab_id) - ELSE 'identity' - END, - 'is_dynamic', msar.is_default_possibly_dynamic(tab_id, attnum) - ) - ), - jsonb_build_object() - ), + 'default', msar.describe_column_default(tab_id, attnum), 'has_dependents', msar.has_dependents(tab_id, attnum), 'description', msar.col_description(tab_id, attnum), 'current_role_priv', msar.list_column_privileges_for_current_role(tab_id, attnum), diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 820e4a4545..924eb9c30c 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -2662,7 +2662,7 @@ BEGIN "name": "txt", "type": "text", "default": { - "value": "'abc'::text", + "value": "abc", "is_dynamic": false }, "nullable": true, From 2e495e7d1ea8b00cfcf16ace8aad4798e874d88c Mon Sep 17 00:00:00 2001 From: zack <6351754+zackkrida@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:12:51 -0500 Subject: [PATCH 118/122] Update .github/workflows/staging-deploy.yml Co-authored-by: Brent Moran --- .github/workflows/staging-deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/staging-deploy.yml b/.github/workflows/staging-deploy.yml index 18b201fd79..e562829589 100644 --- a/.github/workflows/staging-deploy.yml +++ b/.github/workflows/staging-deploy.yml @@ -8,7 +8,7 @@ jobs: staging-deploy: runs-on: ubuntu-latest steps: - - name: Checkout ansible repo + - name: Checkout infrastructure repo uses: actions/checkout@v2 with: repository: 'mathesar-foundation/mathesar-infrastructure' From 630b3f4110c9ab1a319688d0fbd2d59f4b10cf52 Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Thu, 12 Dec 2024 19:32:19 -0500 Subject: [PATCH 119/122] Fix label bug in CheckboxGroup --- .../src/component-library/checkbox-group/CheckboxGroup.svelte | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mathesar_ui/src/component-library/checkbox-group/CheckboxGroup.svelte b/mathesar_ui/src/component-library/checkbox-group/CheckboxGroup.svelte index 7645b87467..7f71337e04 100644 --- a/mathesar_ui/src/component-library/checkbox-group/CheckboxGroup.svelte +++ b/mathesar_ui/src/component-library/checkbox-group/CheckboxGroup.svelte @@ -56,7 +56,6 @@ valuesAreEqual(o, option))} disabled={innerDisabled} /> - + {label} From 4401a54667d85bce001fb91692260195a029e5a3 Mon Sep 17 00:00:00 2001 From: Sean Colsen Date: Thu, 12 Dec 2024 19:44:28 -0500 Subject: [PATCH 120/122] Remove logic to auto-insert schema name in page title --- mathesar_ui/src/pages/pageTitleUtils.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/mathesar_ui/src/pages/pageTitleUtils.ts b/mathesar_ui/src/pages/pageTitleUtils.ts index 4b65bea9b8..4fdc8528e6 100644 --- a/mathesar_ui/src/pages/pageTitleUtils.ts +++ b/mathesar_ui/src/pages/pageTitleUtils.ts @@ -1,17 +1,10 @@ import { get } from 'svelte/store'; import { _ } from 'svelte-i18n'; -import { currentSchema } from '@mathesar/stores/schemas'; - const SEPARATOR = ' | '; function makePageTitle(parts: string[]): string { - const schema = get(currentSchema); const allParts = [...parts]; - if (schema) { - // TODO_BETA: Make this reactive - allParts.push(get(schema.name)); - } allParts.push(get(_)('mathesar')); return allParts.join(SEPARATOR); } From 30d70bfd7085f6a56c9bd00f7dc73180862acd28 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 13 Dec 2024 16:51:39 +0800 Subject: [PATCH 121/122] use Generated Identity in Library example data --- .../resources/library_without_checkouts.sql | 101 ++++-------------- 1 file changed, 21 insertions(+), 80 deletions(-) diff --git a/mathesar/examples/resources/library_without_checkouts.sql b/mathesar/examples/resources/library_without_checkouts.sql index 7233edd35b..08420e7e8b 100644 --- a/mathesar/examples/resources/library_without_checkouts.sql +++ b/mathesar/examples/resources/library_without_checkouts.sql @@ -7,15 +7,9 @@ CREATE TABLE "Authors" ( "Website" mathesar_types.uri ); -CREATE SEQUENCE "Authors_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Authors_id_seq" OWNED BY "Authors".id; +ALTER TABLE "Authors" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Authors_id_seq" +); -- Books @@ -34,15 +28,9 @@ CREATE TABLE "Books" ( "Publisher" integer ); -CREATE SEQUENCE "Books_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Books_id_seq" OWNED BY "Books".id; +ALTER TABLE "Books" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Books_id_seq" +); -- Checkouts @@ -56,15 +44,9 @@ CREATE TABLE "Checkouts" ( "Check In Time" timestamp without time zone ); -CREATE SEQUENCE "Checkouts_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Checkouts_id_seq" OWNED BY "Checkouts".id; +ALTER TABLE "Checkouts" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Checkouts_id_seq" +); -- Items @@ -78,15 +60,9 @@ CREATE TABLE "Items" ( ); -CREATE SEQUENCE "Items_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Items_id_seq" OWNED BY "Items".id; +ALTER TABLE "Items" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Items_id_seq" +); -- Media @@ -96,15 +72,9 @@ CREATE TABLE "Media" ( "Type" text ); -CREATE SEQUENCE "Media_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Media_id_seq" OWNED BY "Media".id; +ALTER TABLE "Media" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Media_id_seq" +); -- Patrons @@ -116,15 +86,9 @@ CREATE TABLE "Patrons" ( "Email" mathesar_types.email ); -CREATE SEQUENCE "Patrons_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Patrons_id_seq" OWNED BY "Patrons".id; +ALTER TABLE "Patrons" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Patrons_id_seq" +); -- Publishers @@ -134,32 +98,9 @@ CREATE TABLE "Publishers" ( "Name" text ); -CREATE SEQUENCE "Publishers_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Publishers_id_seq" OWNED BY "Publishers".id; - - --- Set up defaults -ALTER TABLE ONLY "Authors" - ALTER COLUMN id SET DEFAULT nextval('"Authors_id_seq"'::regclass); -ALTER TABLE ONLY "Books" - ALTER COLUMN id SET DEFAULT nextval('"Books_id_seq"'::regclass); -ALTER TABLE ONLY "Checkouts" - ALTER COLUMN id SET DEFAULT nextval('"Checkouts_id_seq"'::regclass); -ALTER TABLE ONLY "Items" - ALTER COLUMN id SET DEFAULT nextval('"Items_id_seq"'::regclass); -ALTER TABLE ONLY "Media" - ALTER COLUMN id SET DEFAULT nextval('"Media_id_seq"'::regclass); -ALTER TABLE ONLY "Patrons" - ALTER COLUMN id SET DEFAULT nextval('"Patrons_id_seq"'::regclass); -ALTER TABLE ONLY "Publishers" - ALTER COLUMN id SET DEFAULT nextval('"Publishers_id_seq"'::regclass); +ALTER TABLE "Publishers" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Publishers_id_seq" +); -- -- Data for Name: Authors; Type: TABLE DATA; Schema: Library Management; Owner: - From 7a81e747728e4927c660ff8666f13a8a259449e0 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 13 Dec 2024 16:52:50 +0800 Subject: [PATCH 122/122] use Generated Identity in Movie Collection example data --- .../resources/movie_collection_tables.sql | 227 ++++-------------- 1 file changed, 45 insertions(+), 182 deletions(-) diff --git a/mathesar/examples/resources/movie_collection_tables.sql b/mathesar/examples/resources/movie_collection_tables.sql index 95e1b35a2d..6fea21db6a 100644 --- a/mathesar/examples/resources/movie_collection_tables.sql +++ b/mathesar/examples/resources/movie_collection_tables.sql @@ -7,19 +7,9 @@ CREATE TABLE "Departments" ( "Name" text ); -CREATE SEQUENCE "Departments_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Departments_id_seq" OWNED BY "Departments".id; - -ALTER TABLE ONLY "Departments" - ALTER COLUMN id SET DEFAULT nextval('"Departments_id_seq"'::regclass); - +ALTER TABLE "Departments" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Departments_id_seq" +); -- Genres @@ -28,19 +18,9 @@ CREATE TABLE "Genres" ( "Name" text ); - -CREATE SEQUENCE "Genres_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Genres_id_seq" OWNED BY "Genres".id; - -ALTER TABLE ONLY "Genres" - ALTER COLUMN id SET DEFAULT nextval('"Genres_id_seq"'::regclass); +ALTER TABLE "Genres" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Genres_id_seq" +); -- Jobs @@ -50,18 +30,9 @@ CREATE TABLE "Jobs" ( "Name" text ); -CREATE SEQUENCE "Jobs_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Jobs_id_seq" OWNED BY "Jobs".id; - -ALTER TABLE ONLY "Jobs" - ALTER COLUMN id SET DEFAULT nextval('"Jobs_id_seq"'::regclass); +ALTER TABLE "Jobs" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Jobs_id_seq" +); -- Movie Cast Map @@ -74,18 +45,9 @@ CREATE TABLE "Movie Cast Map" ( "Credit Order" integer ); -CREATE SEQUENCE "Movie Cast Map_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movie Cast Map_id_seq" OWNED BY "Movie Cast Map".id; - -ALTER TABLE ONLY "Movie Cast Map" - ALTER COLUMN id SET DEFAULT nextval('"Movie Cast Map_id_seq"'::regclass); +ALTER TABLE "Movie Cast Map" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movie Cast Map_id_seq" +); -- Movie Crew Map @@ -98,18 +60,9 @@ CREATE TABLE "Movie Crew Map" ( "Crew Member" integer NOT NULL ); -CREATE SEQUENCE "Movie Crew Map_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movie Crew Map_id_seq" OWNED BY "Movie Crew Map".id; - -ALTER TABLE ONLY "Movie Crew Map" - ALTER COLUMN id SET DEFAULT nextval('"Movie Crew Map_id_seq"'::regclass); +ALTER TABLE "Movie Crew Map" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movie Crew Map_id_seq" +); -- Movie Genre Map @@ -120,18 +73,9 @@ CREATE TABLE "Movie Genre Map" ( "Genre" integer ); -CREATE SEQUENCE "Movie Genre Map_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movie Genre Map_id_seq" OWNED BY "Movie Genre Map".id; - -ALTER TABLE ONLY "Movie Genre Map" - ALTER COLUMN id SET DEFAULT nextval('"Movie Genre Map_id_seq"'::regclass); +ALTER TABLE "Movie Genre Map" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movie Genre Map_id_seq" +); -- Movie Production Company Map @@ -142,18 +86,9 @@ CREATE TABLE "Movie Production Company Map" ( "Production Company" integer ); -CREATE SEQUENCE "Movie Production Company Map_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movie Production Company Map_id_seq" OWNED BY "Movie Production Company Map".id; - -ALTER TABLE ONLY "Movie Production Company Map" - ALTER COLUMN id SET DEFAULT nextval('"Movie Production Company Map_id_seq"'::regclass); +ALTER TABLE "Movie Production Company Map" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movie Production Company Map_id_seq" +); -- Movie Production Country Map @@ -164,18 +99,9 @@ CREATE TABLE "Movie Production Country Map" ( "Production Country" integer NOT NULL ); -CREATE SEQUENCE "Movie Production Country Map_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movie Production Country Map_id_seq" OWNED BY "Movie Production Country Map".id; - -ALTER TABLE ONLY "Movie Production Country Map" - ALTER COLUMN id SET DEFAULT nextval('"Movie Production Country Map_id_seq"'::regclass); +ALTER TABLE "Movie Production Country Map" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movie Production Country Map_id_seq" +); -- Movie Spoken Language Map @@ -186,18 +112,9 @@ CREATE TABLE "Movie Spoken Language Map" ( "Spoken Language" integer NOT NULL ); -CREATE SEQUENCE "Movie Spoken Language Map_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movie Spoken Language Map_id_seq" OWNED BY "Movie Spoken Language Map".id; - -ALTER TABLE ONLY "Movie Spoken Language Map" - ALTER COLUMN id SET DEFAULT nextval('"Movie Spoken Language Map_id_seq"'::regclass); +ALTER TABLE "Movie Spoken Language Map" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movie Spoken Language Map_id_seq" +); -- Movies @@ -218,18 +135,9 @@ CREATE TABLE "Movies" ( "Original Language" text ); -CREATE SEQUENCE "Movies_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Movies_id_seq" OWNED BY "Movies".id; - -ALTER TABLE ONLY "Movies" - ALTER COLUMN id SET DEFAULT nextval('"Movies_id_seq"'::regclass); +ALTER TABLE "Movies" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Movies_id_seq" +); -- People @@ -239,18 +147,9 @@ CREATE TABLE "People" ( "Name" text ); -CREATE SEQUENCE "People_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "People_id_seq" OWNED BY "People".id; - -ALTER TABLE ONLY "People" - ALTER COLUMN id SET DEFAULT nextval('"People_id_seq"'::regclass); +ALTER TABLE "People" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "People_id_seq" +); -- Production Companies @@ -260,18 +159,9 @@ CREATE TABLE "Production Companies" ( "Name" text ); -CREATE SEQUENCE "Production Companies_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Production Companies_id_seq" OWNED BY "Production Companies".id; - -ALTER TABLE ONLY "Production Companies" - ALTER COLUMN id SET DEFAULT nextval('"Production Companies_id_seq"'::regclass); +ALTER TABLE "Production Companies" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Production Companies_id_seq" +); -- Production Countries @@ -282,18 +172,9 @@ CREATE TABLE "Production Countries" ( "ISO 3166-1" character(2) NOT NULL ); -CREATE SEQUENCE "Production Countries_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Production Countries_id_seq" OWNED BY "Production Countries".id; - -ALTER TABLE ONLY "Production Countries" - ALTER COLUMN id SET DEFAULT nextval('"Production Countries_id_seq"'::regclass); +ALTER TABLE "Production Countries" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Production Countries_id_seq" +); -- Spoken Languages @@ -304,18 +185,9 @@ CREATE TABLE "Spoken Languages" ( "ISO 639-1" character(2) NOT NULL ); -CREATE SEQUENCE "Spoken Languages_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Spoken Languages_id_seq" OWNED BY "Spoken Languages".id; - -ALTER TABLE ONLY "Spoken Languages" - ALTER COLUMN id SET DEFAULT nextval('"Spoken Languages_id_seq"'::regclass); +ALTER TABLE "Spoken Languages" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Spoken Languages_id_seq" +); -- Sub-Collections @@ -325,15 +197,6 @@ CREATE TABLE "Sub-Collections" ( "Name" text ); -CREATE SEQUENCE "Sub-Collections_id_seq" - AS integer - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE "Sub-Collections_id_seq" OWNED BY "Sub-Collections".id; - -ALTER TABLE ONLY "Sub-Collections" - ALTER COLUMN id SET DEFAULT nextval('"Sub-Collections_id_seq"'::regclass); +ALTER TABLE "Sub-Collections" ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY ( + SEQUENCE NAME "Sub-Collections_id_seq" +);