Merge branch 'master' into 930_set_null

.github/workflows/run-flake8.yml

@@ -16,5 +16,6 @@ jobs:
         with:
           checkName: "flake8"
           path: "."
+          plugins: flake8-no-types
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

db/columns/operations/create.py

@@ -19,8 +19,14 @@
 from db.tables.operations.select import reflect_table_from_oid
 from db.types.operations.cast import get_supported_alter_column_types
 
+COLUMN_NAME_TEMPLATE = 'Column'
+
 
 def create_column(engine, table_oid, column_data):
+    table = reflect_table_from_oid(table_oid, engine)
+    column_name = column_data.get(NAME, '').strip()
+    if column_name == '':
+        column_data[NAME] = gen_col_name(table)
     column_type = column_data.get(TYPE, column_data.get("type"))
     column_type_options = column_data.get("type_options", {})
     column_nullable = column_data.get(NULLABLE, True)
@@ -67,6 +73,13 @@ def create_column(engine, table_oid, column_data):
     )
 
 
+def gen_col_name(table):
+    base_name = COLUMN_NAME_TEMPLATE
+    col_num = len(table.c)
+    name = f'{base_name} {col_num}'
+    return name
+
+
 def _gen_col_name(table, column_name):
     num = 1
     new_column_name = f"{column_name}_{num}"

db/functions/operations/deserialize.py

@@ -1,9 +1,9 @@
-from db.functions.base import DBFunction, Literal, ColumnName
+from db.functions.base import Literal, ColumnName
 from db.functions.known_db_functions import known_db_functions
 from db.functions.exceptions import UnknownDBFunctionID, BadDBFunctionFormat
 
 
-def get_db_function_from_ma_function_spec(spec: dict) -> DBFunction:
+def get_db_function_from_ma_function_spec(spec):
     """
     Expects a db function specification in the following format:
 

db/tests/columns/operations/test_create.py

@@ -1,7 +1,7 @@
 import pytest
 from sqlalchemy import Integer, Column, Table, MetaData, Numeric, UniqueConstraint
 
-from db.columns.operations.create import create_column, duplicate_column
+from db.columns.operations.create import create_column, duplicate_column, gen_col_name
 from db.columns.operations.select import get_column_attnum_from_name, get_column_default
 from db.tables.operations.select import get_oid_from_table, reflect_table_from_oid
 from db.constraints.operations.select import get_column_constraints
@@ -357,3 +357,75 @@ def test_duplicate_column_default(engine_with_schema, copy_data, copy_constraint
         assert default == expt_default
     else:
         assert default is None
+
+
+def test_create_column_accepts_column_data_without_name_attribute(engine_email_type):
+    engine, schema = engine_email_type
+    table_name = "atableone"
+    initial_column_name = "Column 0"
+    expected_column_name = "Column 1"
+    table = Table(
+        table_name,
+        MetaData(bind=engine, schema=schema),
+        Column(initial_column_name, Integer),
+    )
+    table.create()
+    table_oid = get_oid_from_table(table_name, schema, engine)
+    column_data = {"type": "BOOLEAN"}
+    created_col = create_column(engine, table_oid, column_data)
+    altered_table = reflect_table_from_oid(table_oid, engine)
+    assert len(altered_table.columns) == 2
+    assert created_col.name == expected_column_name
+
+
+def test_create_column_accepts_column_data_with_name_as_empty_string(engine_email_type):
+    engine, schema = engine_email_type
+    table_name = "atableone"
+    initial_column_name = "Column 0"
+    expected_column_name = "Column 1"
+    table = Table(
+        table_name,
+        MetaData(bind=engine, schema=schema),
+        Column(initial_column_name, Integer),
+    )
+    table.create()
+    table_oid = get_oid_from_table(table_name, schema, engine)
+    column_data = {"name": "", "type": "BOOLEAN"}
+    created_col = create_column(engine, table_oid, column_data)
+    altered_table = reflect_table_from_oid(table_oid, engine)
+    assert len(altered_table.columns) == 2
+    assert created_col.name == expected_column_name
+
+
+def test_generate_column_name(engine_email_type):
+    engine, schema = engine_email_type
+    name_set = {
+        'Center',
+        'Status',
+        'Case Number',
+        'Patent Number',
+        'Application SN',
+        'Title',
+        'Patent Expiration Date',
+        ''
+    }
+    table_name = "atableone"
+    initial_column_name = "id"
+    table = Table(
+        table_name,
+        MetaData(bind=engine, schema=schema),
+        Column(initial_column_name, Integer),
+    )
+    table.create()
+    table_oid = get_oid_from_table(table_name, schema, engine)
+    for name in name_set:
+        column_data = {"name": name, "type": "BOOLEAN"}
+        create_column(engine, table_oid, column_data)
+    altered_table = reflect_table_from_oid(table_oid, engine)
+    n = len(name_set) + 1
+    # Expected column name should be 'Column n'
+    # where n is length of number of columns already in the table
+    expected_column_name = f"Column {n}"
+    generated_column_name = gen_col_name(altered_table)
+    assert len(altered_table.columns) == n
+    assert generated_column_name == expected_column_name

db/types/base.py

@@ -88,7 +88,7 @@ class MathesarCustomType(Enum):
 
 
 # Origin: https://www.python.org/dev/peps/pep-0616/#id17
-def _remove_prefix(self: str, prefix: str, /) -> str:
+def _remove_prefix(self, prefix, /):
     """
     This will remove the passed prefix, if it's there.
     Otherwise, it will return the string unchanged.

mathesar/api/serializers/columns.py

@@ -95,7 +95,7 @@ class Meta(SimpleColumnSerializer.Meta):
         )
         model_fields = ('display_options',)
 
-    name = serializers.CharField(required=False)
+    name = serializers.CharField(required=False, allow_blank=True)
 
     # From scratch fields
     type = serializers.CharField(source='plain_type', required=False)
@@ -116,7 +116,7 @@ class Meta(SimpleColumnSerializer.Meta):
 
     def validate(self, data):
         if not self.partial:
-            from_scratch_required_fields = ['name', 'type']
+            from_scratch_required_fields = ['type']
             from_scratch_specific_fields = ['type', 'nullable', 'primary_key']
             from_dupe_required_fields = ['source_column']
             from_dupe_specific_fields = ['source_column', 'copy_source_data',

mathesar/errors.py

@@ -11,6 +11,6 @@ class URLNotReachable(Exception):
 
 
 class URLInvalidContentTypeError(Exception):
-    def __init__(self, content_type, *args: object):
+    def __init__(self, content_type, *args):
         self.content_type = content_type
         super().__init__(*args)

mathesar/models.py

@@ -1,5 +1,3 @@
-from typing import Any
-
 from bidict import bidict
 from django.contrib.auth.models import User
 from django.core.cache import cache
@@ -347,7 +345,7 @@ class Column(ReflectionManagerMixin, BaseModel):
     def __str__(self):
         return f"{self.__class__.__name__}: {self.table_id}-{self.attnum}"
 
-    def __getattribute__(self, name: str) -> Any:
+    def __getattribute__(self, name):
         try:
             return super().__getattribute__(name)
         except AttributeError:

mathesar/tests/api/test_column_api.py

@@ -389,6 +389,49 @@ def test_column_create_some_parameters(column_test_table, client):
     assert response_data['field'] == "type"
 
 
+def test_column_create_no_name_parameter(column_test_table, client):
+    cache.clear()
+    type_ = "BOOLEAN"
+    num_columns = len(column_test_table.sa_columns)
+    generated_name = f"Column {num_columns}"
+    data = {
+        "type": type_
+    }
+    response = client.post(
+        f"/api/db/v0/tables/{column_test_table.id}/columns/", data=data
+    )
+    assert response.status_code == 201
+    new_columns_response = client.get(
+        f"/api/db/v0/tables/{column_test_table.id}/columns/"
+    )
+    assert new_columns_response.json()["count"] == num_columns + 1
+    actual_new_col = new_columns_response.json()["results"][-1]
+    assert actual_new_col["name"] == generated_name
+    assert actual_new_col["type"] == type_
+
+
+def test_column_create_name_parameter_empty(column_test_table, client):
+    cache.clear()
+    name = ""
+    type_ = "BOOLEAN"
+    num_columns = len(column_test_table.sa_columns)
+    generated_name = f"Column {num_columns}"
+    data = {
+        "name": name, "type": type_
+    }
+    response = client.post(
+        f"/api/db/v0/tables/{column_test_table.id}/columns/", data=data
+    )
+    assert response.status_code == 201
+    new_columns_response = client.get(
+        f"/api/db/v0/tables/{column_test_table.id}/columns/"
+    )
+    assert new_columns_response.json()["count"] == num_columns + 1
+    actual_new_col = new_columns_response.json()["results"][-1]
+    assert actual_new_col["name"] == generated_name
+    assert actual_new_col["type"] == type_
+
+
 def test_column_update_name(column_test_table, client):
     cache.clear()
     name = "updatedname"
@@ -754,6 +797,4 @@ def test_column_duplicate_no_parameters(column_test_table, client):
     response_data = response.json()
     assert response.status_code == 400
     assert response_data[0]["message"] == "This field is required."
-    assert response_data[0]["field"] == "name"
-    assert response_data[1]["message"] == "This field is required."
-    assert response_data[1]["field"] == "type"
+    assert response_data[0]["field"] == "type"

mathesar/tests/conftest.py

@@ -24,7 +24,7 @@
 
 
 @pytest.fixture(scope="session", autouse=True)
-def django_db_setup(request, django_db_blocker) -> None:
+def django_db_setup(request, django_db_blocker):
     """
     A stripped down version of pytest-django's original django_db_setup fixture
     See: https://github.com/pytest-dev/pytest-django/blob/master/pytest_django/fixtures.py#L96
@@ -45,7 +45,7 @@ def django_db_setup(request, django_db_blocker) -> None:
             aliases=["default"],
         )
 
-    def teardown_database() -> None:
+    def teardown_database():
         with django_db_blocker.unblock():
             try:
                 teardown_databases(db_cfg, verbosity=request.config.option.verbose)

mathesar/tests/imports/test_csv.py

@@ -31,7 +31,7 @@ def headerless_data_file(headerless_csv_filename):
 def schema(engine, test_db_model):
     create_schema(TEST_SCHEMA, engine)
     schema_oid = get_schema_oid_from_name(TEST_SCHEMA, engine)
-    yield Schema.objects.create(oid=schema_oid, database=test_db_model)
+    yield Schema.current_objects.create(oid=schema_oid, database=test_db_model)
     with engine.begin() as conn:
         conn.execute(text(f'DROP SCHEMA "{TEST_SCHEMA}" CASCADE;'))
 

mathesar/tests/integration/utils/locators.py

@@ -1,15 +1,12 @@
-from playwright.sync_api import Page
-
-
-def get_tables_list(page: Page):
+def get_tables_list(page):
     return page.locator("#sidebar li[aria-level='1']:has(button:has-text('Tables')) ul")
 
 
-def get_table_entry(page: Page, table_name):
+def get_table_entry(page, table_name):
     return get_tables_list(page).locator(f"li:has-text('{table_name}')")
 
 
-def get_tab(page: Page, tab_text):
+def get_tab(page, tab_text):
     return page.locator(
         f".tab-container [role=tablist] [role=presentation]:has-text('{tab_text}')"
     )

mathesar_ui/.eslintrc.cjs

@@ -21,6 +21,7 @@ module.exports = {
   rules: {
     'import/no-extraneous-dependencies': ['error', { devDependencies: true }],
     'no-console': ['warn', { allow: ['error'] }],
+    '@typescript-eslint/explicit-member-accessibility': 'off',
     '@typescript-eslint/ban-ts-comment': [
       'error',
       {
@@ -106,6 +107,15 @@ module.exports = {
         '@typescript-eslint/require-await': 'off',
       },
     },
+    {
+      files: ['*.ts', '*.tsx'],
+      rules: {
+        '@typescript-eslint/explicit-member-accessibility': [
+          'error',
+          { accessibility: 'no-public' },
+        ],
+      },
+    },
   ],
   env: {
     es6: true,

mathesar_ui/src/component-library/common/actions/clickOffBounds.ts

@@ -25,15 +25,30 @@ export default function clickOffBounds(
     }
   }
 
-  document.body.addEventListener('pointerdown', outOfBoundsListener, true);
+  /**
+   * When the browser supports pointer events, we use the pointerdown event
+   * which is fired for all mouse buttons and touches. However, older Safari
+   * versions don't have pointer events, so we fallback to mouse events. Touches
+   * should fire a mousedown event too.
+   */
+  const events =
+    'onpointerdown' in document.body
+      ? ['pointerdown']
+      : ['mousedown', 'contextmenu'];
+
+  events.forEach((event) => {
+    document.body.addEventListener(event, outOfBoundsListener, true);
+  });
 
   function update(opts: Options) {
     callback = opts.callback;
     references = opts.references;
   }
 
   function destroy() {
-    document.body.removeEventListener('pointerdown', outOfBoundsListener, true);
+    events.forEach((event) => {
+      document.body.removeEventListener(event, outOfBoundsListener, true);
+    });
   }
 
   return {

mathesar_ui/src/component-library/common/utils/contextValidationUtil.ts

@@ -8,11 +8,11 @@ export type ValidationResultStore = Writable<boolean>;
 const VALIDATION_CONTEXT_KEY = 'validationContext';
 
 class ContextBasedValidator {
-  public validationResult: Writable<boolean> = writable(true);
+  validationResult: Writable<boolean> = writable(true);
 
-  public validationFunctionMap: Map<string, ValidationFunction> = new Map();
+  validationFunctionMap: Map<string, ValidationFunction> = new Map();
 
-  public validate(): boolean {
+  validate(): boolean {
     let isValid = true;
     // eslint-disable-next-line no-restricted-syntax
     for (const validationFn of this.validationFunctionMap.values()) {
@@ -22,7 +22,7 @@ class ContextBasedValidator {
     return isValid;
   }
 
-  public addValidator(key: string, fn: ValidationFunction) {
+  addValidator(key: string, fn: ValidationFunction) {
     this.validationFunctionMap.set(key, fn);
 
     onDestroy(() => {

mathesar_ui/src/component-library/context-menu/ContextMenu.svelte

@@ -88,9 +88,3 @@
     </div>
   {/if}
 </div>
-
-<style>
-  .context-menu {
-    position: fixed;
-  }
-</style>