Merge branch 'develop' into 3384_disable_connection_fields

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()

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

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)

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

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]),
+        ),
+    ]

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):

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()

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.'

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 @@
       </RichText>
     </p>
     <ul class="schema-list" slot="content">
-      {#each displayList as schema (schema.id)}
-        <li class="schema-list-item">
-          <SchemaRow
-            {database}
-            {schema}
-            canExecuteDDL={userProfile?.hasPermission(
-              { database, schema },
-              'canExecuteDDL',
-            )}
-            on:edit={() => editSchema(schema)}
-            on:delete={() => deleteSchema(schema)}
-          />
-        </li>
-      {/each}
+      {#if schemasRequestStatus.state === 'success'}
+        {#each displayList as schema (schema.id)}
+          <li class="schema-list-item">
+            <SchemaRow
+              {database}
+              {schema}
+              canExecuteDDL={userProfile?.hasPermission(
+                { database, schema },
+                'canExecuteDDL',
+              )}
+              on:edit={() => editSchema(schema)}
+              on:delete={() => deleteSchema(schema)}
+            />
+          </li>
+        {/each}
+      {:else if schemasRequestStatus.state === 'processing'}
+        <SchemaListSkeleton />
+      {:else if schemasRequestStatus.state === 'failure'}
+        <ErrorBox fullWidth>
+          {#each schemasRequestStatus.errors as error (error)}
+            <p>{error}</p>
+          {/each}
+        </ErrorBox>
+      {/if}
     </ul>
   </EntityContainerWithFilterBar>
 </div>

mathesar_ui/src/pages/database/SchemaListSkeleton.svelte

@@ -0,0 +1,29 @@
+<script lang="ts">
+  import { Skeleton } from '@mathesar/component-library';
+
+  export let numTables = 4;
+
+  $: skeletons = Array.from({ length: numTables }, (x, i) => i);
+</script>
+
+<div class="schema-skeleton-container">
+  {#each skeletons as _ (_)}
+    <div class="schema-skeleton">
+      <Skeleton loading={true} />
+    </div>
+  {/each}
+</div>
+
+<style lang="scss">
+  .schema-skeleton-container {
+    display: grid;
+    grid-gap: 1rem;
+    --minimum-item-width: 18rem;
+  }
+  .schema-skeleton {
+    position: relative;
+    height: 7.5rem;
+    border-radius: var(--border-radius-l);
+    overflow: hidden;
+  }
+</style>

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<SchemaEntry['id'] | undefined> =
   writable(commonData.current_schema ?? undefined);
 
 export interface DBSchemaStoreData {
-  state: States;
+  requestStatus: RequestStatus;
   data: Map<SchemaEntry['id'], SchemaEntry>;
-  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<DBSchemaStoreData> = derived(
 
     if (!$currentConnectionName) {
       set({
-        state: States.Done,
+        requestStatus: { state: 'success' },
         data: new Map(),
       });
     } else {