diff --git a/mathesar/api/db/viewsets/databases.py b/mathesar/api/db/viewsets/databases.py index 795d30e21c..95d4eccbef 100644 --- a/mathesar/api/db/viewsets/databases.py +++ b/mathesar/api/db/viewsets/databases.py @@ -33,7 +33,7 @@ def get_queryset(self): def destroy(self, request, pk=None): db_object = self.get_object() - if request.query_params.get('del_msar_schemas'): + if request.query_params.get('del_msar_schemas').lower() == 'true': engine = db_object._sa_engine uninstall_mathesar_from_database(engine) db_object.delete() diff --git a/mathesar/api/exceptions/error_codes.py b/mathesar/api/exceptions/error_codes.py index 133c84d41b..b38e264e05 100644 --- a/mathesar/api/exceptions/error_codes.py +++ b/mathesar/api/exceptions/error_codes.py @@ -50,6 +50,7 @@ class ErrorCodes(Enum): URLNotReachableError = 4405 URLInvalidContentType = 4406 UnknownDBType = 4408 + InvalidColumnOrder = 4430 InvalidDateError = 4413 InvalidDateFormatError = 4414 InvalidLinkChoice = 4409 diff --git a/mathesar/api/exceptions/validation_exceptions/exceptions.py b/mathesar/api/exceptions/validation_exceptions/exceptions.py index c06f5ea271..b99e1e1f3b 100644 --- a/mathesar/api/exceptions/validation_exceptions/exceptions.py +++ b/mathesar/api/exceptions/validation_exceptions/exceptions.py @@ -194,3 +194,14 @@ def __init__( field=None, ): super().__init__(None, self.error_code, message, field) + + +class InvalidColumnOrder(MathesarValidationException): + error_code = ErrorCodes.InvalidColumnOrder.value + + def __init__( + self, + message="Invalid column order.", + field=None, + ): + super().__init__(None, self.error_code, message, field, None) diff --git a/mathesar/api/serializers/table_settings.py b/mathesar/api/serializers/table_settings.py index 5ec415297e..576fb0c569 100644 --- a/mathesar/api/serializers/table_settings.py +++ b/mathesar/api/serializers/table_settings.py @@ -1,8 +1,8 @@ from rest_framework import serializers from mathesar.api.exceptions.mixins import MathesarErrorMessageMixin - -from mathesar.models.base import PreviewColumnSettings, TableSettings, compute_default_preview_template +from mathesar.api.exceptions.validation_exceptions.exceptions import InvalidColumnOrder +from mathesar.models.base import PreviewColumnSettings, TableSettings, compute_default_preview_template, ValidationError class PreviewColumnSerializer(MathesarErrorMessageMixin, serializers.ModelSerializer): @@ -37,6 +37,9 @@ def update(self, instance, validated_data): column_order_data = validated_data.pop('column_order', None) if column_order_data is not None: - instance.column_order = column_order_data - instance.save() + try: + instance.column_order = column_order_data + instance.save() + except ValidationError: + raise InvalidColumnOrder() return instance diff --git a/mathesar/migrations/0011_auto_20240103_2120.py b/mathesar/migrations/0011_auto_20240103_2120.py new file mode 100644 index 0000000000..e6c4e0af06 --- /dev/null +++ b/mathesar/migrations/0011_auto_20240103_2120.py @@ -0,0 +1,34 @@ +# Generated by Django 3.1.14 on 2024-01-03 21:20 + +from django.db import migrations, models +import mathesar.models.base + + +class Migration(migrations.Migration): + + dependencies = [ + ('mathesar', '0010_remove_editable'), + ] + + operations = [ + migrations.AlterField( + model_name='constraint', + name='oid', + field=models.PositiveIntegerField(), + ), + migrations.AlterField( + model_name='schema', + name='oid', + field=models.PositiveIntegerField(), + ), + migrations.AlterField( + model_name='table', + name='oid', + field=models.PositiveIntegerField(), + ), + migrations.AlterField( + model_name='tablesettings', + name='column_order', + field=models.JSONField(blank=True, default=None, null=True, validators=[mathesar.models.base.validate_column_order]), + ), + ] diff --git a/mathesar/models/base.py b/mathesar/models/base.py index bf930426a0..60b2b5059b 100644 --- a/mathesar/models/base.py +++ b/mathesar/models/base.py @@ -92,7 +92,7 @@ class DatabaseObject(ReflectionManagerMixin, BaseModel): """ Objects that can be referenced using a database identifier """ - oid = models.IntegerField() + oid = models.PositiveIntegerField() class Meta: abstract = True @@ -920,10 +920,26 @@ class PreviewColumnSettings(BaseModel): template = models.CharField(max_length=255) +def validate_column_order(value): + """ + Custom validator to ensure that all elements in the list are positive integers. + """ + if not all(isinstance(item, int) and item > 0 for item in value): + raise ValidationError("All elements of column order must be positive integers.") + + class TableSettings(ReflectionManagerMixin, BaseModel): preview_settings = models.OneToOneField(PreviewColumnSettings, on_delete=models.CASCADE) table = models.OneToOneField(Table, on_delete=models.CASCADE, related_name="settings") - column_order = JSONField(null=True, default=None) + column_order = JSONField(null=True, blank=True, default=None, validators=[validate_column_order]) + + def save(self, **kwargs): + # Cleans the fields before saving by running respective field validator(s) + try: + self.clean_fields() + except ValidationError as e: + raise e + super().save(**kwargs) def _create_table_settings(tables): diff --git a/mathesar/tests/api/test_database_api.py b/mathesar/tests/api/test_database_api.py index 7add09b620..e63b384bef 100644 --- a/mathesar/tests/api/test_database_api.py +++ b/mathesar/tests/api/test_database_api.py @@ -7,6 +7,8 @@ from mathesar.state.django import reflect_db_objects from mathesar.models.base import Table, Schema, Database from django.conf import settings +from django.core.exceptions import ObjectDoesNotExist +from db.install import install_mathesar from db.engine import create_future_engine_with_custom_types @@ -143,6 +145,33 @@ def test_database_list_deleted(client, db_dj_model): check_database(db_dj_model, response_data['results'][0]) +def test_delete_dbconn_with_msar_schemas(client, db_dj_model): + # install mathesar specific schemas + install_mathesar( + db_dj_model.name, + db_dj_model.username, + db_dj_model.password, + db_dj_model.host, + db_dj_model.port, + True + ) + engine = db_dj_model._sa_engine + check_schema_exists = text( + "SELECT schema_name FROM information_schema.schemata \ + WHERE schema_name LIKE '%msar' OR schema_name = 'mathesar_types';" + ) + with engine.connect() as conn: + before_deletion = conn.execute(check_schema_exists) + response = client.delete(f'/api/db/v0/connections/{db_dj_model.id}/?del_msar_schemas=true') + after_deletion = conn.execute(check_schema_exists) + + with pytest.raises(ObjectDoesNotExist): + Database.objects.get(id=db_dj_model.id) + assert response.status_code == 204 + assert before_deletion.rowcount == 3 + assert after_deletion.rowcount == 0 + + def test_database_detail(client, db_dj_model): response = client.get(f'/api/db/v0/connections/{db_dj_model.id}/') response_database = response.json() diff --git a/mathesar/tests/api/test_table_settings_api.py b/mathesar/tests/api/test_table_settings_api.py index 54431b71e2..69a434ebef 100644 --- a/mathesar/tests/api/test_table_settings_api.py +++ b/mathesar/tests/api/test_table_settings_api.py @@ -4,6 +4,7 @@ from db.tables.operations.select import get_oid_from_table from mathesar.models import base as models_base +from mathesar.api.exceptions.error_codes import ErrorCodes @pytest.fixture @@ -133,3 +134,19 @@ def test_update_table_settings_string_in_column_order(client, column_test_table) assert response.status_code == 200 response_data = response.json() assert response_data['column_order'] == column_order_as_ints + + +def test_update_table_settings_negative_column_order(client, column_test_table): + column_order = [-4, 5, 6] + data = { + "column_order": column_order + } + response = client.patch( + f"/api/db/v0/tables/{column_test_table.id}/settings/{column_test_table.settings.id}/", + data=data, + ) + response_data = response.json()[0] + print(response_data) + assert response.status_code == 400 + assert response_data['code'] == ErrorCodes.InvalidColumnOrder.value + assert response_data['message'] == 'Invalid column order.' diff --git a/mathesar_ui/src/pages/database/DatabaseDetails.svelte b/mathesar_ui/src/pages/database/DatabaseDetails.svelte index f854096249..26dceef43f 100644 --- a/mathesar_ui/src/pages/database/DatabaseDetails.svelte +++ b/mathesar_ui/src/pages/database/DatabaseDetails.svelte @@ -35,11 +35,13 @@ DeleteConnectionModal, } from '@mathesar/systems/connections'; import { CONNECTIONS_URL } from '@mathesar/routes/urls'; + import ErrorBox from '@mathesar/components/message-boxes/ErrorBox.svelte'; import { RichText } from '@mathesar/components/rich-text'; import { router } from 'tinro'; import AddEditSchemaModal from './AddEditSchemaModal.svelte'; import DbAccessControlModal from './DbAccessControlModal.svelte'; import SchemaRow from './SchemaRow.svelte'; + import SchemaListSkeleton from './SchemaListSkeleton.svelte'; const addEditModal = modal.spawnModalController(); const accessControlModal = modal.spawnModalController(); @@ -52,6 +54,7 @@ export let database: Database; $: schemasMap = $schemasStore.data; + $: schemasRequestStatus = $schemasStore.requestStatus; $: canExecuteDDL = userProfile?.hasPermission({ database }, 'canExecuteDDL'); $: canEditPermissions = userProfile?.hasPermission( @@ -215,20 +218,30 @@

diff --git a/mathesar_ui/src/pages/database/SchemaListSkeleton.svelte b/mathesar_ui/src/pages/database/SchemaListSkeleton.svelte new file mode 100644 index 0000000000..a87e889ae8 --- /dev/null +++ b/mathesar_ui/src/pages/database/SchemaListSkeleton.svelte @@ -0,0 +1,29 @@ + + +
+ {#each skeletons as _ (_)} +
+ +
+ {/each} +
+ + diff --git a/mathesar_ui/src/stores/schemas.ts b/mathesar_ui/src/stores/schemas.ts index 27f92ec1a2..c327f1f7da 100644 --- a/mathesar_ui/src/stores/schemas.ts +++ b/mathesar_ui/src/stores/schemas.ts @@ -2,9 +2,9 @@ import { writable, derived, get } from 'svelte/store'; import type { Writable, Readable, Unsubscriber } from 'svelte/store'; import { preloadCommonData } from '@mathesar/utils/preloadData'; -import { - States, - type PaginatedResponse, +import type { + PaginatedResponse, + RequestStatus, } from '@mathesar/api/utils/requestUtils'; import schemasApi from '@mathesar/api/schemas'; import type { Connection } from '@mathesar/api/connections'; @@ -20,9 +20,8 @@ export const currentSchemaId: Writable = writable(commonData.current_schema ?? undefined); export interface DBSchemaStoreData { - state: States; + requestStatus: RequestStatus; data: Map; - error?: string; } const dbSchemaStoreMap: Map< @@ -49,9 +48,8 @@ function setDBSchemaStore( schemaMap.set(schema.id, schema); }); const storeValue: DBSchemaStoreData = { - state: States.Done, + requestStatus: { state: 'success' }, data: schemaMap, - error: undefined, }; let store = dbSchemaStoreMap.get(connectionId); @@ -151,7 +149,7 @@ async function refetchSchemasForDB( try { store.update((currentData) => ({ ...currentData, - state: States.Loading, + requestStatus: { state: 'processing' }, })); dbSchemasRequestMap.get(connectionId)?.cancel(); @@ -167,8 +165,12 @@ async function refetchSchemasForDB( } catch (err) { store.update((currentData) => ({ ...currentData, - state: States.Error, - error: err instanceof Error ? err.message : 'Error in fetching schemas', + requestStatus: { + state: 'failure', + errors: [ + err instanceof Error ? err.message : 'Error in fetching schemas', + ], + }, })); return undefined; } @@ -205,7 +207,7 @@ function getSchemasStoreForDB( let store = dbSchemaStoreMap.get(connectionId); if (!store) { store = writable({ - state: States.Loading, + requestStatus: { state: 'processing' }, data: new Map(), }); dbSchemaStoreMap.set(connectionId, store); @@ -215,7 +217,7 @@ function getSchemasStoreForDB( void refetchSchemasForDB(connectionId); } preload = false; - } else if (get(store).error) { + } else if (get(store).requestStatus.state === 'failure') { void refetchSchemasForDB(connectionId); } return store; @@ -270,7 +272,7 @@ export const schemas: Readable = derived( if (!$currentConnectionName) { set({ - state: States.Done, + requestStatus: { state: 'success' }, data: new Map(), }); } else {