mirror of
https://github.com/apache/superset.git
synced 2026-04-19 08:04:53 +00:00
feat(docs): Populate Developer Portal with comprehensive documentation framework (#35217)
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
318
docs/developer_portal/guidelines/backend-style-guidelines.md
Normal file
318
docs/developer_portal/guidelines/backend-style-guidelines.md
Normal file
@@ -0,0 +1,318 @@
|
||||
---
|
||||
title: Backend Style Guidelines
|
||||
sidebar_position: 3
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# Backend Style Guidelines
|
||||
|
||||
This is a list of statements that describe how we do backend development in Superset. While they might not be 100% true for all files in the repo, they represent the gold standard we strive towards for backend quality and style.
|
||||
|
||||
* We use a monolithic Python/Flask/Flask-AppBuilder backend, with small single-responsibility satellite services where necessary.
|
||||
* Files are generally organized by feature or object type. Within each domain, we can have api controllers, models, schemas, commands, and data access objects (dao).
|
||||
* See: [Proposal for Improving Superset's Python Code Organization](https://github.com/apache/superset/issues/9077)
|
||||
* API controllers use Marshmallow Schemas to serialize/deserialize data.
|
||||
* Authentication and authorization are controlled by the [security manager](https://github.com/apache/superset/blob/master/superset/security/manager).
|
||||
* We use Pytest for unit and integration tests. These live in the `tests` directory.
|
||||
* We add tests for every new piece of functionality added to the backend.
|
||||
* We use pytest fixtures to share setup between tests.
|
||||
* We use sqlalchemy to access both Superset's application database, and users' analytics databases.
|
||||
* We make changes backwards compatible whenever possible.
|
||||
* If a change cannot be made backwards compatible, it goes into a major release.
|
||||
* See: [Proposal For Semantic Versioning](https://github.com/apache/superset/issues/12566)
|
||||
* We use Swagger for API documentation, with docs written inline on the API endpoint code.
|
||||
* We prefer thin ORM models, putting shared functionality in other utilities.
|
||||
* Several linters/checkers are used to maintain consistent code style and type safety: pylint, pypy, black, isort.
|
||||
* `__init__.py` files are kept empty to avoid implicit dependencies.
|
||||
|
||||
## Code Organization
|
||||
|
||||
### Domain-Driven Structure
|
||||
|
||||
Organize code by business domain rather than technical layers:
|
||||
|
||||
```
|
||||
superset/
|
||||
├── dashboards/
|
||||
│ ├── api.py # API endpoints
|
||||
│ ├── commands/ # Business logic
|
||||
│ ├── dao.py # Data access layer
|
||||
│ ├── models.py # Database models
|
||||
│ └── schemas.py # Serialization schemas
|
||||
├── charts/
|
||||
│ ├── api.py
|
||||
│ ├── commands/
|
||||
│ ├── dao.py
|
||||
│ ├── models.py
|
||||
│ └── schemas.py
|
||||
```
|
||||
|
||||
### API Controllers
|
||||
|
||||
Use Flask-AppBuilder with Marshmallow schemas:
|
||||
|
||||
```python
|
||||
from flask_appbuilder.api import BaseApi
|
||||
from marshmallow import Schema, fields
|
||||
|
||||
class DashboardSchema(Schema):
|
||||
id = fields.Integer()
|
||||
dashboard_title = fields.String(required=True)
|
||||
created_on = fields.DateTime(dump_only=True)
|
||||
|
||||
class DashboardApi(BaseApi):
|
||||
resource_name = "dashboard"
|
||||
openapi_spec_tag = "Dashboards"
|
||||
|
||||
@expose("/", methods=["GET"])
|
||||
@protect()
|
||||
@safe
|
||||
def get_list(self) -> Response:
|
||||
"""Get a list of dashboards"""
|
||||
# Implementation
|
||||
```
|
||||
|
||||
### Commands Pattern
|
||||
|
||||
Use commands for business logic:
|
||||
|
||||
```python
|
||||
from typing import Optional
|
||||
from superset.commands.base import BaseCommand
|
||||
from superset.dashboards.dao import DashboardDAO
|
||||
|
||||
class CreateDashboardCommand(BaseCommand):
|
||||
def __init__(self, properties: Dict[str, Any]):
|
||||
self._properties = properties
|
||||
|
||||
def run(self) -> Dashboard:
|
||||
self.validate()
|
||||
return DashboardDAO.create(self._properties)
|
||||
|
||||
def validate(self) -> None:
|
||||
if not self._properties.get("dashboard_title"):
|
||||
raise ValidationError("Title is required")
|
||||
```
|
||||
|
||||
### Data Access Objects (DAOs)
|
||||
|
||||
See: [DAO Style Guidelines and Best Practices](./backend/dao-style-guidelines)
|
||||
|
||||
## Testing
|
||||
|
||||
### Unit Tests
|
||||
|
||||
```python
|
||||
import pytest
|
||||
from unittest.mock import patch
|
||||
from superset.dashboards.commands.create import CreateDashboardCommand
|
||||
|
||||
def test_create_dashboard_success():
|
||||
properties = {
|
||||
"dashboard_title": "Test Dashboard",
|
||||
"owners": [1]
|
||||
}
|
||||
|
||||
command = CreateDashboardCommand(properties)
|
||||
dashboard = command.run()
|
||||
|
||||
assert dashboard.dashboard_title == "Test Dashboard"
|
||||
|
||||
def test_create_dashboard_validation_error():
|
||||
properties = {} # Missing required title
|
||||
|
||||
command = CreateDashboardCommand(properties)
|
||||
|
||||
with pytest.raises(ValidationError):
|
||||
command.run()
|
||||
```
|
||||
|
||||
### Integration Tests
|
||||
|
||||
```python
|
||||
import pytest
|
||||
from superset.app import create_app
|
||||
from superset.extensions import db
|
||||
|
||||
@pytest.fixture
|
||||
def app():
|
||||
app = create_app()
|
||||
app.config["TESTING"] = True
|
||||
with app.app_context():
|
||||
db.create_all()
|
||||
yield app
|
||||
db.drop_all()
|
||||
|
||||
def test_dashboard_api_create(app, auth_headers):
|
||||
with app.test_client() as client:
|
||||
response = client.post(
|
||||
"/api/v1/dashboard/",
|
||||
json={"dashboard_title": "Test Dashboard"},
|
||||
headers=auth_headers
|
||||
)
|
||||
assert response.status_code == 201
|
||||
```
|
||||
|
||||
## Security
|
||||
|
||||
### Authorization Decorators
|
||||
|
||||
```python
|
||||
from flask_appbuilder.security.decorators import has_access
|
||||
|
||||
class DashboardApi(BaseApi):
|
||||
@expose("/", methods=["POST"])
|
||||
@protect()
|
||||
@has_access("can_write", "Dashboard")
|
||||
def post(self) -> Response:
|
||||
"""Create a new dashboard"""
|
||||
# Implementation
|
||||
```
|
||||
|
||||
### Input Validation
|
||||
|
||||
```python
|
||||
from marshmallow import Schema, fields, validate
|
||||
|
||||
class DashboardSchema(Schema):
|
||||
dashboard_title = fields.String(
|
||||
required=True,
|
||||
validate=validate.Length(min=1, max=500)
|
||||
)
|
||||
slug = fields.String(
|
||||
validate=validate.Regexp(r'^[a-z0-9-]+$')
|
||||
)
|
||||
```
|
||||
|
||||
## Database Operations
|
||||
|
||||
### Use SQLAlchemy ORM
|
||||
|
||||
```python
|
||||
from sqlalchemy import Column, Integer, String, Text
|
||||
from superset.models.helpers import AuditMixinNullable
|
||||
|
||||
class Dashboard(Model, AuditMixinNullable):
|
||||
__tablename__ = "dashboards"
|
||||
|
||||
id = Column(Integer, primary_key=True)
|
||||
dashboard_title = Column(String(500))
|
||||
position_json = Column(Text)
|
||||
|
||||
def __repr__(self) -> str:
|
||||
return f"<Dashboard {self.dashboard_title}>"
|
||||
```
|
||||
|
||||
### Database Migrations
|
||||
|
||||
```python
|
||||
# migration file
|
||||
def upgrade():
|
||||
op.add_column(
|
||||
"dashboards",
|
||||
sa.Column("new_column", sa.String(255), nullable=True)
|
||||
)
|
||||
|
||||
def downgrade():
|
||||
op.drop_column("dashboards", "new_column")
|
||||
```
|
||||
|
||||
## Error Handling
|
||||
|
||||
### Custom Exceptions
|
||||
|
||||
```python
|
||||
class SupersetException(Exception):
|
||||
"""Base exception for Superset"""
|
||||
pass
|
||||
|
||||
class ValidationError(SupersetException):
|
||||
"""Raised when validation fails"""
|
||||
pass
|
||||
|
||||
class SecurityException(SupersetException):
|
||||
"""Raised when security check fails"""
|
||||
pass
|
||||
```
|
||||
|
||||
### Error Responses
|
||||
|
||||
```python
|
||||
from flask import jsonify
|
||||
from werkzeug.exceptions import BadRequest
|
||||
|
||||
@app.errorhandler(ValidationError)
|
||||
def handle_validation_error(error):
|
||||
return jsonify({
|
||||
"message": str(error),
|
||||
"error_type": "VALIDATION_ERROR"
|
||||
}), 400
|
||||
```
|
||||
|
||||
## Type Hints and Documentation
|
||||
|
||||
### Use Type Hints
|
||||
|
||||
```python
|
||||
from typing import List, Optional, Dict, Any
|
||||
from superset.models.dashboard import Dashboard
|
||||
|
||||
def get_dashboards_by_owner(owner_id: int) -> List[Dashboard]:
|
||||
"""Get all dashboards owned by a specific user"""
|
||||
return db.session.query(Dashboard).filter_by(owner_id=owner_id).all()
|
||||
|
||||
def create_dashboard(properties: Dict[str, Any]) -> Optional[Dashboard]:
|
||||
"""Create a new dashboard with the given properties"""
|
||||
# Implementation
|
||||
```
|
||||
|
||||
### API Documentation
|
||||
|
||||
```python
|
||||
from flask_appbuilder.api import BaseApi
|
||||
from flask_appbuilder.api.schemas import get_list_schema
|
||||
|
||||
class DashboardApi(BaseApi):
|
||||
@expose("/", methods=["GET"])
|
||||
@protect()
|
||||
@safe
|
||||
def get_list(self) -> Response:
|
||||
"""Get a list of dashboards
|
||||
---
|
||||
get:
|
||||
description: >-
|
||||
Get a list of dashboards
|
||||
parameters:
|
||||
- in: query
|
||||
schema:
|
||||
type: integer
|
||||
name: page_size
|
||||
description: Number of results per page
|
||||
responses:
|
||||
200:
|
||||
description: Success
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: '#/components/schemas/DashboardListResponse'
|
||||
"""
|
||||
# Implementation
|
||||
```
|
||||
365
docs/developer_portal/guidelines/backend/dao-style-guidelines.md
Normal file
365
docs/developer_portal/guidelines/backend/dao-style-guidelines.md
Normal file
@@ -0,0 +1,365 @@
|
||||
---
|
||||
title: DAO Style Guidelines and Best Practices
|
||||
sidebar_position: 1
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# DAO Style Guidelines and Best Practices
|
||||
|
||||
A Data Access Object (DAO) is a pattern that provides an abstract interface to the SQLAlchemy Object Relational Mapper (ORM). The DAOs are critical as they form the building block of the application which are wrapped by the associated commands and RESTful API endpoints.
|
||||
|
||||
Currently there are numerous inconsistencies and violation of the DRY principal within the codebase as it relates to DAOs and ORMs—unnecessary commits, non-ACID transactions, etc.—which makes the code unnecessarily complex and convoluted. Addressing the underlying issues with the DAOs _should_ help simplify the downstream operations and improve the developer experience.
|
||||
|
||||
To ensure consistency the following rules should be adhered to:
|
||||
|
||||
1. All database operations (including testing) should be defined within a DAO, i.e., there should not be any explicit `db.session.add`, `db.session.merge`, etc. calls outside of a DAO.
|
||||
|
||||
2. A DAO should use `create`, `update`, `delete`, `upsert` terms—typical database operations which ensure consistency with commands—rather than action based terms like `save`, `saveas`, `override`, etc.
|
||||
|
||||
3. Sessions should be managed via a [context manager](https://docs.sqlalchemy.org/en/20/orm/session_transaction.html#begin-once) which auto-commits on success and rolls back on failure, i.e., there should be no explicit `db.session.commit` or `db.session.rollback` calls within the DAO.
|
||||
|
||||
4. There should be a single atomic transaction representing the entirety of the operation, i.e., when creating a dataset with associated columns and metrics either all the changes succeed when the transaction is committed, or all the changes are undone when the transaction is rolled back. SQLAlchemy supports [nested transactions](https://docs.sqlalchemy.org/en/20/orm/session_transaction.html#nested-transaction) via the `begin_nested` method which can be nested—inline with how DAOs are invoked.
|
||||
|
||||
5. The database layer should adopt a "shift left" mentality i.e., uniqueness/foreign key constraints, relationships, cascades, etc. should all be defined in the database layer rather than being enforced in the application layer.
|
||||
|
||||
## DAO Implementation Examples
|
||||
|
||||
### Basic DAO Structure
|
||||
|
||||
```python
|
||||
from typing import List, Optional, Dict, Any
|
||||
from sqlalchemy.orm import Session
|
||||
from superset.extensions import db
|
||||
from superset.models.dashboard import Dashboard
|
||||
|
||||
class DashboardDAO:
|
||||
"""Data Access Object for Dashboard operations"""
|
||||
|
||||
@classmethod
|
||||
def find_by_id(cls, dashboard_id: int) -> Optional[Dashboard]:
|
||||
"""Find dashboard by ID"""
|
||||
return db.session.query(Dashboard).filter_by(id=dashboard_id).first()
|
||||
|
||||
@classmethod
|
||||
def find_by_ids(cls, dashboard_ids: List[int]) -> List[Dashboard]:
|
||||
"""Find dashboards by list of IDs"""
|
||||
return db.session.query(Dashboard).filter(
|
||||
Dashboard.id.in_(dashboard_ids)
|
||||
).all()
|
||||
|
||||
@classmethod
|
||||
def create(cls, properties: Dict[str, Any]) -> Dashboard:
|
||||
"""Create a new dashboard"""
|
||||
with db.session.begin():
|
||||
dashboard = Dashboard(**properties)
|
||||
db.session.add(dashboard)
|
||||
db.session.flush() # Get the ID
|
||||
return dashboard
|
||||
|
||||
@classmethod
|
||||
def update(cls, dashboard: Dashboard, properties: Dict[str, Any]) -> Dashboard:
|
||||
"""Update an existing dashboard"""
|
||||
with db.session.begin():
|
||||
for key, value in properties.items():
|
||||
setattr(dashboard, key, value)
|
||||
return dashboard
|
||||
|
||||
@classmethod
|
||||
def delete(cls, dashboard: Dashboard) -> None:
|
||||
"""Delete a dashboard"""
|
||||
with db.session.begin():
|
||||
db.session.delete(dashboard)
|
||||
```
|
||||
|
||||
### Complex DAO Operations
|
||||
|
||||
```python
|
||||
class DatasetDAO:
|
||||
"""Data Access Object for Dataset operations"""
|
||||
|
||||
@classmethod
|
||||
def create_with_columns_and_metrics(
|
||||
cls,
|
||||
dataset_properties: Dict[str, Any],
|
||||
columns: List[Dict[str, Any]],
|
||||
metrics: List[Dict[str, Any]]
|
||||
) -> Dataset:
|
||||
"""Create dataset with associated columns and metrics atomically"""
|
||||
with db.session.begin():
|
||||
# Create the dataset
|
||||
dataset = Dataset(**dataset_properties)
|
||||
db.session.add(dataset)
|
||||
db.session.flush() # Get the dataset ID
|
||||
|
||||
# Create columns
|
||||
for column_props in columns:
|
||||
column_props['dataset_id'] = dataset.id
|
||||
column = TableColumn(**column_props)
|
||||
db.session.add(column)
|
||||
|
||||
# Create metrics
|
||||
for metric_props in metrics:
|
||||
metric_props['dataset_id'] = dataset.id
|
||||
metric = SqlMetric(**metric_props)
|
||||
db.session.add(metric)
|
||||
|
||||
return dataset
|
||||
|
||||
@classmethod
|
||||
def bulk_delete(cls, dataset_ids: List[int]) -> int:
|
||||
"""Delete multiple datasets and return count"""
|
||||
with db.session.begin():
|
||||
count = db.session.query(Dataset).filter(
|
||||
Dataset.id.in_(dataset_ids)
|
||||
).delete(synchronize_session=False)
|
||||
return count
|
||||
```
|
||||
|
||||
### Query Methods
|
||||
|
||||
```python
|
||||
class DashboardDAO:
|
||||
@classmethod
|
||||
def find_by_slug(cls, slug: str) -> Optional[Dashboard]:
|
||||
"""Find dashboard by slug"""
|
||||
return db.session.query(Dashboard).filter_by(slug=slug).first()
|
||||
|
||||
@classmethod
|
||||
def find_by_owner(cls, owner_id: int) -> List[Dashboard]:
|
||||
"""Find all dashboards owned by a user"""
|
||||
return db.session.query(Dashboard).filter_by(
|
||||
created_by_fk=owner_id
|
||||
).all()
|
||||
|
||||
@classmethod
|
||||
def search(
|
||||
cls,
|
||||
query: str,
|
||||
page: int = 0,
|
||||
page_size: int = 25
|
||||
) -> Tuple[List[Dashboard], int]:
|
||||
"""Search dashboards with pagination"""
|
||||
base_query = db.session.query(Dashboard).filter(
|
||||
Dashboard.dashboard_title.ilike(f"%{query}%")
|
||||
)
|
||||
|
||||
total_count = base_query.count()
|
||||
dashboards = base_query.offset(page * page_size).limit(page_size).all()
|
||||
|
||||
return dashboards, total_count
|
||||
```
|
||||
|
||||
### Error Handling in DAOs
|
||||
|
||||
```python
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from superset.exceptions import DAOCreateFailedError, DAODeleteFailedError
|
||||
|
||||
class DashboardDAO:
|
||||
@classmethod
|
||||
def create(cls, properties: Dict[str, Any]) -> Dashboard:
|
||||
"""Create a new dashboard with error handling"""
|
||||
try:
|
||||
with db.session.begin():
|
||||
dashboard = Dashboard(**properties)
|
||||
db.session.add(dashboard)
|
||||
db.session.flush()
|
||||
return dashboard
|
||||
except IntegrityError as ex:
|
||||
raise DAOCreateFailedError(str(ex)) from ex
|
||||
|
||||
@classmethod
|
||||
def delete(cls, dashboard: Dashboard) -> None:
|
||||
"""Delete a dashboard with error handling"""
|
||||
try:
|
||||
with db.session.begin():
|
||||
db.session.delete(dashboard)
|
||||
except IntegrityError as ex:
|
||||
raise DAODeleteFailedError(
|
||||
f"Cannot delete dashboard {dashboard.id}: {str(ex)}"
|
||||
) from ex
|
||||
```
|
||||
|
||||
## Best Practices
|
||||
|
||||
### 1. Use Class Methods
|
||||
|
||||
All DAO methods should be class methods (`@classmethod`) rather than instance methods:
|
||||
|
||||
```python
|
||||
# ✅ Good
|
||||
class DashboardDAO:
|
||||
@classmethod
|
||||
def find_by_id(cls, dashboard_id: int) -> Optional[Dashboard]:
|
||||
return db.session.query(Dashboard).filter_by(id=dashboard_id).first()
|
||||
|
||||
# ❌ Avoid
|
||||
class DashboardDAO:
|
||||
def find_by_id(self, dashboard_id: int) -> Optional[Dashboard]:
|
||||
return db.session.query(Dashboard).filter_by(id=dashboard_id).first()
|
||||
```
|
||||
|
||||
### 2. Use Context Managers for Transactions
|
||||
|
||||
Always use context managers to ensure proper transaction handling:
|
||||
|
||||
```python
|
||||
# ✅ Good - automatic commit/rollback
|
||||
@classmethod
|
||||
def create(cls, properties: Dict[str, Any]) -> Dashboard:
|
||||
with db.session.begin():
|
||||
dashboard = Dashboard(**properties)
|
||||
db.session.add(dashboard)
|
||||
return dashboard
|
||||
|
||||
# ❌ Avoid - manual commit/rollback
|
||||
@classmethod
|
||||
def create(cls, properties: Dict[str, Any]) -> Dashboard:
|
||||
try:
|
||||
dashboard = Dashboard(**properties)
|
||||
db.session.add(dashboard)
|
||||
db.session.commit()
|
||||
return dashboard
|
||||
except Exception:
|
||||
db.session.rollback()
|
||||
raise
|
||||
```
|
||||
|
||||
### 3. Use Descriptive Method Names
|
||||
|
||||
Method names should clearly indicate the operation:
|
||||
|
||||
```python
|
||||
# ✅ Good - clear CRUD operations
|
||||
create()
|
||||
update()
|
||||
delete()
|
||||
find_by_id()
|
||||
find_by_slug()
|
||||
|
||||
# ❌ Avoid - ambiguous names
|
||||
save()
|
||||
remove()
|
||||
get()
|
||||
```
|
||||
|
||||
### 4. Type Hints
|
||||
|
||||
Always include type hints for parameters and return values:
|
||||
|
||||
```python
|
||||
@classmethod
|
||||
def find_by_ids(cls, dashboard_ids: List[int]) -> List[Dashboard]:
|
||||
"""Find dashboards by list of IDs"""
|
||||
return db.session.query(Dashboard).filter(
|
||||
Dashboard.id.in_(dashboard_ids)
|
||||
).all()
|
||||
```
|
||||
|
||||
### 5. Batch Operations
|
||||
|
||||
Provide efficient batch operations when needed:
|
||||
|
||||
```python
|
||||
@classmethod
|
||||
def bulk_update_published_status(
|
||||
cls,
|
||||
dashboard_ids: List[int],
|
||||
published: bool
|
||||
) -> int:
|
||||
"""Update published status for multiple dashboards"""
|
||||
with db.session.begin():
|
||||
count = db.session.query(Dashboard).filter(
|
||||
Dashboard.id.in_(dashboard_ids)
|
||||
).update(
|
||||
{Dashboard.published: published},
|
||||
synchronize_session=False
|
||||
)
|
||||
return count
|
||||
```
|
||||
|
||||
## Testing DAOs
|
||||
|
||||
### Unit Tests for DAOs
|
||||
|
||||
```python
|
||||
import pytest
|
||||
from superset.dashboards.dao import DashboardDAO
|
||||
from superset.models.dashboard import Dashboard
|
||||
|
||||
def test_dashboard_create(session):
|
||||
"""Test creating a dashboard"""
|
||||
properties = {
|
||||
"dashboard_title": "Test Dashboard",
|
||||
"slug": "test-dashboard"
|
||||
}
|
||||
|
||||
dashboard = DashboardDAO.create(properties)
|
||||
|
||||
assert dashboard.id is not None
|
||||
assert dashboard.dashboard_title == "Test Dashboard"
|
||||
assert dashboard.slug == "test-dashboard"
|
||||
|
||||
def test_dashboard_find_by_slug(session):
|
||||
"""Test finding dashboard by slug"""
|
||||
# Create test data
|
||||
dashboard = Dashboard(
|
||||
dashboard_title="Test Dashboard",
|
||||
slug="test-dashboard"
|
||||
)
|
||||
session.add(dashboard)
|
||||
session.commit()
|
||||
|
||||
# Test the DAO method
|
||||
found_dashboard = DashboardDAO.find_by_slug("test-dashboard")
|
||||
|
||||
assert found_dashboard is not None
|
||||
assert found_dashboard.dashboard_title == "Test Dashboard"
|
||||
|
||||
def test_dashboard_delete(session):
|
||||
"""Test deleting a dashboard"""
|
||||
dashboard = Dashboard(dashboard_title="Test Dashboard")
|
||||
session.add(dashboard)
|
||||
session.commit()
|
||||
dashboard_id = dashboard.id
|
||||
|
||||
DashboardDAO.delete(dashboard)
|
||||
|
||||
deleted_dashboard = DashboardDAO.find_by_id(dashboard_id)
|
||||
assert deleted_dashboard is None
|
||||
```
|
||||
|
||||
### Integration Tests
|
||||
|
||||
```python
|
||||
def test_create_dataset_with_columns_atomic(app_context):
|
||||
"""Test that creating dataset with columns is atomic"""
|
||||
dataset_properties = {"table_name": "test_table"}
|
||||
columns = [{"column_name": "col1"}, {"column_name": "col2"}]
|
||||
|
||||
# This should succeed completely or fail completely
|
||||
dataset = DatasetDAO.create_with_columns_and_metrics(
|
||||
dataset_properties, columns, []
|
||||
)
|
||||
|
||||
assert dataset.id is not None
|
||||
assert len(dataset.columns) == 2
|
||||
```
|
||||
148
docs/developer_portal/guidelines/design-guidelines.md
Normal file
148
docs/developer_portal/guidelines/design-guidelines.md
Normal file
@@ -0,0 +1,148 @@
|
||||
---
|
||||
title: Design Guidelines
|
||||
sidebar_position: 1
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# Design Guidelines
|
||||
|
||||
This is an area to host resources and documentation supporting the evolution and proper use of Superset design system elements. If content is to be added to this section or requires revisiting, a proposal should be submitted to the `dev@superset.apache.org` email list with either a text proposal or a link to a GitHub issue providing the markdown that will be added to this wiki. The Dev list will have the chance to review the proposal and arrive at lazy consensus. A committer may then copy/paste the markdown to this wiki, and make it public.
|
||||
|
||||
## Capitalization Guidelines
|
||||
|
||||
### Sentence case
|
||||
|
||||
Use sentence-case capitalization for everything in the UI (except these **).
|
||||
|
||||
Sentence case is predominantly lowercase. Capitalize only the initial character of the first word, and other words that require capitalization, like:
|
||||
|
||||
* **Proper nouns.** Objects in the product _are not_ considered proper nouns e.g. dashboards, charts, saved queries etc. Proprietary feature names eg. SQL Lab, Preset Manager _are_ considered proper nouns
|
||||
* **Acronyms** (e.g. CSS, HTML)
|
||||
* When referring to **UI labels that are themselves capitalized** from sentence case (e.g. page titles - Dashboards page, Charts page, Saved queries page, etc.)
|
||||
* User input that is reflected in the UI. E.g. a user-named a dashboard tab
|
||||
|
||||
**Sentence case vs. Title case:** Title case: "A Dog Takes a Walk in Paris" Sentence case: "A dog takes a walk in Paris"
|
||||
|
||||
**Why sentence case?**
|
||||
|
||||
* It's generally accepted as the quickest to read
|
||||
* It's the easiest form to distinguish between common and proper nouns
|
||||
|
||||
### How to refer to UI elements
|
||||
|
||||
When writing about a UI element, use the same capitalization as used in the UI.
|
||||
|
||||
For example, if an input field is labeled "Name" then you refer to this as the "Name input field". Similarly, if a button has the label "Save" in it, then it is correct to refer to the "Save button".
|
||||
|
||||
Where a product page is titled "Settings", you refer to this in writing as follows: "Edit your personal information on the Settings page".
|
||||
|
||||
Often a product page will have the same title as the objects it contains. In this case, refer to the page as it appears in the UI, and the objects as common nouns:
|
||||
|
||||
* Upload a dashboard on the Dashboards page
|
||||
* Go to Dashboards
|
||||
* View dashboard
|
||||
* View all dashboards
|
||||
* Upload CSS templates on the CSS templates page
|
||||
* Queries that you save will appear on the Saved queries page
|
||||
* Create custom queries in SQL Lab then create dashboards
|
||||
|
||||
### **Exceptions to sentence case:**
|
||||
|
||||
1. Acronyms and abbreviations. Examples: URL, CSV, XML
|
||||
|
||||
## Button Design Guidelines
|
||||
|
||||
### Overview
|
||||
|
||||
**Button variants:**
|
||||
|
||||
1. Primary
|
||||
2. Secondary
|
||||
3. Tertiary
|
||||
4. Destructive
|
||||
|
||||
**Button styles:** Each button variant has three styles:
|
||||
|
||||
1. Text
|
||||
2. Icon+text
|
||||
3. Icon only
|
||||
|
||||
Primary buttons have a fourth style: dropdown.
|
||||
|
||||
**Usage:** Buttons communicate actions that users can take. Do not use for navigations, instead use links.
|
||||
|
||||
**Purpose:**
|
||||
|
||||
| Button Type | Description |
|
||||
|------------|-------------|
|
||||
| Primary | Main call to action, just 1 per page not including modals or main headers |
|
||||
| Secondary | Secondary actions, always in conjunction with a primary |
|
||||
| Tertiary | For less prominent actions; can be used in isolation or paired with a primary button |
|
||||
| Destructive | For actions that could have destructive effects on the user's data |
|
||||
|
||||
## Error Message Design Guidelines
|
||||
|
||||
### Definition
|
||||
|
||||
Interface errors appear when the application can't do what the user wants, typically because:
|
||||
|
||||
- The app technically fails to complete the request
|
||||
- The app can't understand the user input
|
||||
- The user tries to combine operations that can't work together
|
||||
|
||||
In all cases, encountering errors increases user friction and frustration while trying to use the application. Providing an error experience that helps the user understand what happened and their next steps is key to building user confidence and increasing engagement.
|
||||
|
||||
### General best practices
|
||||
|
||||
**The best error experience is no error at all.** Before implementing error patterns, consider what you might do in the interface before the user would encounter an error to prevent it from happening at all. This might look like:
|
||||
|
||||
- Providing tooltips or microcopy to help users understand how to interact with the interface
|
||||
- Disabling parts of the UI until desired conditions are met (e.g. disabling a Save button until mandatory fields in a form are completed)
|
||||
- Correcting an error automatically (e.g. autocorrecting spelling errors)
|
||||
|
||||
**Only report errors users care about.** The only errors that should appear in the interface are errors that require user acknowledgement and action (even if that action is "try again later" or "contact support").
|
||||
|
||||
**Do not start the user in an error state.** If user inputs are required to display an initial interface (e.g. a chart in Explore), use empty states or field highlighting to drive users to the required action.
|
||||
|
||||
### Patterns
|
||||
|
||||
#### Pattern selection
|
||||
|
||||
Select one pattern per error (e.g. do not implement an inline and banner pattern for the same error).
|
||||
|
||||
When the error... | Use...
|
||||
---------------- | ------
|
||||
Is directly related to a UI control | Inline error
|
||||
Is not directly related to a UI control | Banner error
|
||||
|
||||
#### Inline
|
||||
|
||||
Inline errors are used when the source of the error is directly related to a UI control (text input, selector, etc.) such as the user not populating a required field or entering a number in a text field.
|
||||
|
||||
##### Anatomy
|
||||
|
||||
Use the `LabeledErrorBoundInput` component for this error pattern.
|
||||
|
||||
1. **Message** Provides direction on how to populate the input correctly.
|
||||
|
||||
##### Implementation details
|
||||
|
||||
- Where and when relevant, scroll the screen to the UI control with the error
|
||||
@@ -0,0 +1,44 @@
|
||||
---
|
||||
title: Frontend Style Guidelines
|
||||
sidebar_position: 2
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# Frontend Style Guidelines
|
||||
|
||||
This is a list of statements that describe how we do frontend development in Superset. While they might not be 100% true for all files in the repo, they represent the gold standard we strive towards for frontend quality and style.
|
||||
|
||||
* We develop using TypeScript.
|
||||
* We use React for building components, and Redux to manage app/global state.
|
||||
* See: [Component Style Guidelines and Best Practices](./frontend/component-style-guidelines)
|
||||
* We prefer functional components to class components and use hooks for local component state.
|
||||
* We use [Ant Design](https://ant.design/) components from our component library whenever possible, only building our own custom components when it's required.
|
||||
* We use [@emotion](https://emotion.sh/docs/introduction) to provide styling for our components, co-locating styling within component files.
|
||||
* See: [Emotion Styling Guidelines and Best Practices](./frontend/emotion-styling-guidelines)
|
||||
* We use Jest for unit tests, React Testing Library for component tests, and Cypress for end-to-end tests.
|
||||
* See: [Testing Guidelines and Best Practices](./frontend/testing-guidelines)
|
||||
* We add tests for every new component or file added to the frontend.
|
||||
* We organize our repo so similar files live near each other, and tests are co-located with the files they test.
|
||||
* We prefer small, easily testable files and components.
|
||||
* We use ESLint and Prettier to automatically fix lint errors and format the code.
|
||||
* We do not debate code formatting style in PRs, instead relying on automated tooling to enforce it.
|
||||
* If there's not a linting rule, we don't have a rule!
|
||||
* We use [React Storybook](https://storybook.js.org/) and [Applitools](https://applitools.com/) to help preview/test and stabilize our components
|
||||
@@ -0,0 +1,258 @@
|
||||
---
|
||||
title: Component Style Guidelines and Best Practices
|
||||
sidebar_position: 1
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# Component Style Guidelines and Best Practices
|
||||
|
||||
This documentation illustrates how we approach component development in Superset and provides examples to help you in writing new components or updating existing ones by following our community-approved standards.
|
||||
|
||||
This guide is intended primarily for reusable components. Whenever possible, all new components should be designed with reusability in mind.
|
||||
|
||||
## General Guidelines
|
||||
|
||||
- We use [Ant Design](https://ant.design/) as our component library. Do not build a new component if Ant Design provides one but rather instead extend or customize what the library provides
|
||||
- Always style your component using Emotion and always prefer the theme variables whenever applicable. See: [Emotion Styling Guidelines and Best Practices](./emotion-styling-guidelines)
|
||||
- All components should be made to be reusable whenever possible
|
||||
- All components should follow the structure and best practices as detailed below
|
||||
|
||||
## Directory and component structure
|
||||
|
||||
```
|
||||
superset-frontend/src/components
|
||||
{ComponentName}/
|
||||
index.tsx
|
||||
{ComponentName}.test.tsx
|
||||
{ComponentName}.stories.tsx
|
||||
```
|
||||
|
||||
**Components root directory:** Components that are meant to be re-used across different parts of the application should go in the `superset-frontend/src/components` directory. Components that are meant to be specific for a single part of the application should be located in the nearest directory where the component is used, for example, `superset-frontend/src/Explore/components`
|
||||
|
||||
**Exporting the component:** All components within the `superset-frontend/src/components` directory should be exported from `superset-frontend/src/components/index.ts` to facilitate their imports by other components
|
||||
|
||||
**Component directory name:** Use `PascalCase` for the component directory name
|
||||
|
||||
**Storybook:** Components should come with a storybook file whenever applicable, with the following naming convention `{ComponentName}.stories.tsx`. More details about Storybook below
|
||||
|
||||
**Unit and end-to-end tests:** All components should come with unit tests using Jest and React Testing Library. The file name should follow this naming convention `{ComponentName}.test.tsx.` Read the [Testing Guidelines and Best Practices](./testing-guidelines) for more details about tests
|
||||
|
||||
## Component Development Best Practices
|
||||
|
||||
### Use TypeScript
|
||||
|
||||
All new components should be written in TypeScript. This helps catch errors early and provides better development experience with IDE support.
|
||||
|
||||
```tsx
|
||||
interface ComponentProps {
|
||||
title: string;
|
||||
isVisible?: boolean;
|
||||
onClose?: () => void;
|
||||
}
|
||||
|
||||
export const MyComponent: React.FC<ComponentProps> = ({
|
||||
title,
|
||||
isVisible = true,
|
||||
onClose
|
||||
}) => {
|
||||
// Component implementation
|
||||
};
|
||||
```
|
||||
|
||||
### Prefer Functional Components
|
||||
|
||||
Use functional components with hooks instead of class components:
|
||||
|
||||
```tsx
|
||||
// ✅ Good - Functional component with hooks
|
||||
export const MyComponent: React.FC<Props> = ({ data }) => {
|
||||
const [loading, setLoading] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
// Effect logic
|
||||
}, []);
|
||||
|
||||
return <div>{/* Component JSX */}</div>;
|
||||
};
|
||||
|
||||
// ❌ Avoid - Class component
|
||||
class MyComponent extends React.Component {
|
||||
// Class implementation
|
||||
}
|
||||
```
|
||||
|
||||
### Follow Ant Design Patterns
|
||||
|
||||
Extend Ant Design components rather than building from scratch:
|
||||
|
||||
```tsx
|
||||
import { Button } from 'antd';
|
||||
import styled from '@emotion/styled';
|
||||
|
||||
const StyledButton = styled(Button)`
|
||||
// Custom styling using emotion
|
||||
`;
|
||||
```
|
||||
|
||||
### Reusability and Props Design
|
||||
|
||||
Design components with reusability in mind:
|
||||
|
||||
```tsx
|
||||
interface ButtonProps {
|
||||
variant?: 'primary' | 'secondary' | 'tertiary';
|
||||
size?: 'small' | 'medium' | 'large';
|
||||
loading?: boolean;
|
||||
disabled?: boolean;
|
||||
children: React.ReactNode;
|
||||
onClick?: () => void;
|
||||
}
|
||||
|
||||
export const CustomButton: React.FC<ButtonProps> = ({
|
||||
variant = 'primary',
|
||||
size = 'medium',
|
||||
...props
|
||||
}) => {
|
||||
// Implementation
|
||||
};
|
||||
```
|
||||
|
||||
## Testing Components
|
||||
|
||||
Every component should include comprehensive tests:
|
||||
|
||||
```tsx
|
||||
// MyComponent.test.tsx
|
||||
import { render, screen, fireEvent } from '@testing-library/react';
|
||||
import { MyComponent } from './MyComponent';
|
||||
|
||||
test('renders component with title', () => {
|
||||
render(<MyComponent title="Test Title" />);
|
||||
expect(screen.getByText('Test Title')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('calls onClose when close button is clicked', () => {
|
||||
const mockOnClose = jest.fn();
|
||||
render(<MyComponent title="Test" onClose={mockOnClose} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: /close/i }));
|
||||
expect(mockOnClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
```
|
||||
|
||||
## Storybook Stories
|
||||
|
||||
Create stories for visual testing and documentation:
|
||||
|
||||
```tsx
|
||||
// MyComponent.stories.tsx
|
||||
import type { Meta, StoryObj } from '@storybook/react';
|
||||
import { MyComponent } from './MyComponent';
|
||||
|
||||
const meta: Meta<typeof MyComponent> = {
|
||||
title: 'Components/MyComponent',
|
||||
component: MyComponent,
|
||||
parameters: {
|
||||
layout: 'centered',
|
||||
},
|
||||
tags: ['autodocs'],
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof meta>;
|
||||
|
||||
export const Default: Story = {
|
||||
args: {
|
||||
title: 'Default Component',
|
||||
isVisible: true,
|
||||
},
|
||||
};
|
||||
|
||||
export const Hidden: Story = {
|
||||
args: {
|
||||
title: 'Hidden Component',
|
||||
isVisible: false,
|
||||
},
|
||||
};
|
||||
```
|
||||
|
||||
## Performance Considerations
|
||||
|
||||
### Use React.memo for Expensive Components
|
||||
|
||||
```tsx
|
||||
import React, { memo } from 'react';
|
||||
|
||||
export const ExpensiveComponent = memo<Props>(({ data }) => {
|
||||
// Expensive rendering logic
|
||||
return <div>{/* Component content */}</div>;
|
||||
});
|
||||
```
|
||||
|
||||
### Optimize Re-renders
|
||||
|
||||
Use `useCallback` and `useMemo` appropriately:
|
||||
|
||||
```tsx
|
||||
export const OptimizedComponent: React.FC<Props> = ({ items, onSelect }) => {
|
||||
const expensiveValue = useMemo(() => {
|
||||
return items.reduce((acc, item) => acc + item.value, 0);
|
||||
}, [items]);
|
||||
|
||||
const handleSelect = useCallback((id: string) => {
|
||||
onSelect(id);
|
||||
}, [onSelect]);
|
||||
|
||||
return <div>{/* Component content */}</div>;
|
||||
};
|
||||
```
|
||||
|
||||
## Accessibility
|
||||
|
||||
Ensure components are accessible:
|
||||
|
||||
```tsx
|
||||
export const AccessibleButton: React.FC<Props> = ({ children, onClick }) => {
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
aria-label="Descriptive label"
|
||||
onClick={onClick}
|
||||
>
|
||||
{children}
|
||||
</button>
|
||||
);
|
||||
};
|
||||
```
|
||||
|
||||
## Error Boundaries
|
||||
|
||||
For components that might fail, consider error boundaries:
|
||||
|
||||
```tsx
|
||||
export const SafeComponent: React.FC<Props> = ({ children }) => {
|
||||
return (
|
||||
<ErrorBoundary fallback={<div>Something went wrong</div>}>
|
||||
{children}
|
||||
</ErrorBoundary>
|
||||
);
|
||||
};
|
||||
```
|
||||
@@ -0,0 +1,346 @@
|
||||
---
|
||||
title: Emotion Styling Guidelines and Best Practices
|
||||
sidebar_position: 2
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# Emotion Styling Guidelines and Best Practices
|
||||
|
||||
## Emotion Styling Guidelines
|
||||
|
||||
### DO these things:
|
||||
|
||||
- **DO** use `styled` when you want to include additional (nested) class selectors in your styles
|
||||
- **DO** use `styled` components when you intend to export a styled component for re-use elsewhere
|
||||
- **DO** use `css` when you want to amend/merge sets of styles compositionally
|
||||
- **DO** use `css` when you're making a small, or single-use set of styles for a component
|
||||
- **DO** move your style definitions from direct usage in the `css` prop to an external variable when they get long
|
||||
- **DO** prefer tagged template literals (`css={css\`...\`}`) over style objects wherever possible for maximum style portability/consistency
|
||||
- **DO** use `useTheme` to get theme variables
|
||||
|
||||
### DON'T do these things:
|
||||
|
||||
- **DON'T** use `styled` for small, single-use style tweaks that would be easier to read/review if they were inline
|
||||
- **DON'T** export incomplete Ant Design components
|
||||
|
||||
## When to use `css` or `styled`
|
||||
|
||||
### Use `css` for:
|
||||
|
||||
1. **Small, single-use styles**
|
||||
```tsx
|
||||
import { css } from '@emotion/react';
|
||||
|
||||
const MyComponent = () => (
|
||||
<div
|
||||
css={css`
|
||||
margin: 8px;
|
||||
padding: 16px;
|
||||
`}
|
||||
>
|
||||
Content
|
||||
</div>
|
||||
);
|
||||
```
|
||||
|
||||
2. **Composing styles**
|
||||
```tsx
|
||||
const baseStyles = css`
|
||||
padding: 16px;
|
||||
border-radius: 4px;
|
||||
`;
|
||||
|
||||
const primaryStyles = css`
|
||||
${baseStyles}
|
||||
background-color: blue;
|
||||
color: white;
|
||||
`;
|
||||
|
||||
const secondaryStyles = css`
|
||||
${baseStyles}
|
||||
background-color: gray;
|
||||
color: black;
|
||||
`;
|
||||
```
|
||||
|
||||
3. **Conditional styling**
|
||||
```tsx
|
||||
const MyComponent = ({ isActive }: { isActive: boolean }) => (
|
||||
<div
|
||||
css={[
|
||||
baseStyles,
|
||||
isActive && activeStyles,
|
||||
]}
|
||||
>
|
||||
Content
|
||||
</div>
|
||||
);
|
||||
```
|
||||
|
||||
### Use `styled` for:
|
||||
|
||||
1. **Reusable components**
|
||||
```tsx
|
||||
import styled from '@emotion/styled';
|
||||
|
||||
const StyledButton = styled.button`
|
||||
padding: 12px 24px;
|
||||
border: none;
|
||||
border-radius: 4px;
|
||||
background-color: ${({ theme }) => theme.colors.primary};
|
||||
color: white;
|
||||
|
||||
&:hover {
|
||||
background-color: ${({ theme }) => theme.colors.primaryDark};
|
||||
}
|
||||
`;
|
||||
```
|
||||
|
||||
2. **Components with complex nested selectors**
|
||||
```tsx
|
||||
const StyledCard = styled.div`
|
||||
padding: 16px;
|
||||
border: 1px solid ${({ theme }) => theme.colors.border};
|
||||
|
||||
.card-header {
|
||||
font-weight: bold;
|
||||
margin-bottom: 8px;
|
||||
}
|
||||
|
||||
.card-content {
|
||||
color: ${({ theme }) => theme.colors.text};
|
||||
|
||||
p {
|
||||
margin-bottom: 12px;
|
||||
}
|
||||
}
|
||||
`;
|
||||
```
|
||||
|
||||
3. **Extending Ant Design components**
|
||||
```tsx
|
||||
import { Button } from 'antd';
|
||||
import styled from '@emotion/styled';
|
||||
|
||||
const CustomButton = styled(Button)`
|
||||
border-radius: 8px;
|
||||
font-weight: 600;
|
||||
|
||||
&.ant-btn-primary {
|
||||
background: linear-gradient(45deg, #1890ff, #722ed1);
|
||||
}
|
||||
`;
|
||||
```
|
||||
|
||||
## Using Theme Variables
|
||||
|
||||
Always use theme variables for consistent styling:
|
||||
|
||||
```tsx
|
||||
import { useTheme } from '@emotion/react';
|
||||
|
||||
const MyComponent = () => {
|
||||
const theme = useTheme();
|
||||
|
||||
return (
|
||||
<div
|
||||
css={css`
|
||||
background-color: ${theme.colors.grayscale.light5};
|
||||
color: ${theme.colors.grayscale.dark2};
|
||||
padding: ${theme.gridUnit * 4}px;
|
||||
border-radius: ${theme.borderRadius}px;
|
||||
`}
|
||||
>
|
||||
Content
|
||||
</div>
|
||||
);
|
||||
};
|
||||
```
|
||||
|
||||
## Common Patterns
|
||||
|
||||
### Responsive Design
|
||||
```tsx
|
||||
const ResponsiveContainer = styled.div`
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
|
||||
${({ theme }) => theme.breakpoints.up('md')} {
|
||||
flex-direction: row;
|
||||
}
|
||||
`;
|
||||
```
|
||||
|
||||
### Animation
|
||||
```tsx
|
||||
const FadeInComponent = styled.div`
|
||||
opacity: 0;
|
||||
transition: opacity 0.3s ease-in-out;
|
||||
|
||||
&.visible {
|
||||
opacity: 1;
|
||||
}
|
||||
`;
|
||||
```
|
||||
|
||||
### Conditional Props
|
||||
```tsx
|
||||
interface StyledDivProps {
|
||||
isHighlighted?: boolean;
|
||||
size?: 'small' | 'medium' | 'large';
|
||||
}
|
||||
|
||||
const StyledDiv = styled.div<StyledDivProps>`
|
||||
padding: 16px;
|
||||
background-color: ${({ isHighlighted, theme }) =>
|
||||
isHighlighted ? theme.colors.primary : theme.colors.grayscale.light5};
|
||||
|
||||
${({ size }) => {
|
||||
switch (size) {
|
||||
case 'small':
|
||||
return css`font-size: 12px;`;
|
||||
case 'large':
|
||||
return css`font-size: 18px;`;
|
||||
default:
|
||||
return css`font-size: 14px;`;
|
||||
}
|
||||
}}
|
||||
`;
|
||||
```
|
||||
|
||||
## Best Practices
|
||||
|
||||
### 1. Use Semantic CSS Properties
|
||||
```tsx
|
||||
// ✅ Good - semantic property names
|
||||
const Header = styled.h1`
|
||||
font-size: ${({ theme }) => theme.typography.sizes.xl};
|
||||
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
|
||||
`;
|
||||
|
||||
// ❌ Avoid - magic numbers
|
||||
const Header = styled.h1`
|
||||
font-size: 24px;
|
||||
margin-bottom: 16px;
|
||||
`;
|
||||
```
|
||||
|
||||
### 2. Group Related Styles
|
||||
```tsx
|
||||
// ✅ Good - grouped styles
|
||||
const Card = styled.div`
|
||||
/* Layout */
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
padding: ${({ theme }) => theme.gridUnit * 4}px;
|
||||
|
||||
/* Appearance */
|
||||
background-color: ${({ theme }) => theme.colors.grayscale.light5};
|
||||
border: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
|
||||
border-radius: ${({ theme }) => theme.borderRadius}px;
|
||||
|
||||
/* Typography */
|
||||
font-family: ${({ theme }) => theme.typography.families.sansSerif};
|
||||
color: ${({ theme }) => theme.colors.grayscale.dark2};
|
||||
`;
|
||||
```
|
||||
|
||||
### 3. Extract Complex Styles
|
||||
```tsx
|
||||
// ✅ Good - extract complex styles to variables
|
||||
const complexGradient = css`
|
||||
background: linear-gradient(
|
||||
135deg,
|
||||
${({ theme }) => theme.colors.primary} 0%,
|
||||
${({ theme }) => theme.colors.secondary} 100%
|
||||
);
|
||||
`;
|
||||
|
||||
const GradientButton = styled.button`
|
||||
${complexGradient}
|
||||
padding: 12px 24px;
|
||||
border: none;
|
||||
color: white;
|
||||
`;
|
||||
```
|
||||
|
||||
### 4. Avoid Deep Nesting
|
||||
```tsx
|
||||
// ✅ Good - shallow nesting
|
||||
const Navigation = styled.nav`
|
||||
.nav-item {
|
||||
padding: 8px 16px;
|
||||
}
|
||||
|
||||
.nav-link {
|
||||
color: ${({ theme }) => theme.colors.text};
|
||||
text-decoration: none;
|
||||
}
|
||||
`;
|
||||
|
||||
// ❌ Avoid - deep nesting
|
||||
const Navigation = styled.nav`
|
||||
ul {
|
||||
li {
|
||||
a {
|
||||
span {
|
||||
color: blue; /* Too nested */
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
`;
|
||||
```
|
||||
|
||||
## Performance Tips
|
||||
|
||||
### 1. Avoid Inline Functions in CSS
|
||||
```tsx
|
||||
// ✅ Good - external function
|
||||
const getBackgroundColor = (isActive: boolean, theme: any) =>
|
||||
isActive ? theme.colors.primary : theme.colors.secondary;
|
||||
|
||||
const Button = styled.button<{ isActive: boolean }>`
|
||||
background-color: ${({ isActive, theme }) => getBackgroundColor(isActive, theme)};
|
||||
`;
|
||||
|
||||
// ❌ Avoid - inline function (creates new function on each render)
|
||||
const Button = styled.button<{ isActive: boolean }>`
|
||||
background-color: ${({ isActive, theme }) =>
|
||||
isActive ? theme.colors.primary : theme.colors.secondary};
|
||||
`;
|
||||
```
|
||||
|
||||
### 2. Use CSS Objects for Dynamic Styles
|
||||
```tsx
|
||||
// For highly dynamic styles, consider CSS objects
|
||||
const dynamicStyles = (props: Props) => ({
|
||||
backgroundColor: props.color,
|
||||
fontSize: `${props.size}px`,
|
||||
// ... other dynamic properties
|
||||
});
|
||||
|
||||
const DynamicComponent = (props: Props) => (
|
||||
<div css={dynamicStyles(props)}>
|
||||
Content
|
||||
</div>
|
||||
);
|
||||
```
|
||||
297
docs/developer_portal/guidelines/frontend/testing-guidelines.md
Normal file
297
docs/developer_portal/guidelines/frontend/testing-guidelines.md
Normal file
@@ -0,0 +1,297 @@
|
||||
---
|
||||
title: Testing Guidelines and Best Practices
|
||||
sidebar_position: 3
|
||||
---
|
||||
|
||||
<!--
|
||||
Licensed to the Apache Software Foundation (ASF) under one
|
||||
or more contributor license agreements. See the NOTICE file
|
||||
distributed with this work for additional information
|
||||
regarding copyright ownership. The ASF licenses this file
|
||||
to you under the Apache License, Version 2.0 (the
|
||||
"License"); you may not use this file except in compliance
|
||||
with the License. You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing,
|
||||
software distributed under the License is distributed on an
|
||||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
KIND, either express or implied. See the License for the
|
||||
specific language governing permissions and limitations
|
||||
under the License.
|
||||
-->
|
||||
|
||||
# Testing Guidelines and Best Practices
|
||||
|
||||
We feel that tests are an important part of a feature and not an additional or optional effort. That's why we colocate test files with functionality and sometimes write tests upfront to help validate requirements and shape the API of our components. Every new component or file added should have an associated test file with the `.test` extension.
|
||||
|
||||
We use Jest, React Testing Library (RTL), and Cypress to write our unit, integration, and end-to-end tests. For each type, we have a set of best practices/tips described below:
|
||||
|
||||
## Jest
|
||||
|
||||
### Write simple, standalone tests
|
||||
|
||||
The importance of simplicity is often overlooked in test cases. Clear, dumb code should always be preferred over complex ones. The test cases should be pretty much standalone and should not involve any external logic if not absolutely necessary. That's because you want the corpus of the tests to be easy to read and understandable at first sight.
|
||||
|
||||
### Avoid nesting when you're testing
|
||||
|
||||
Avoid the use of `describe` blocks in favor of inlined tests. If your tests start to grow and you feel the need to group tests, prefer to break them into multiple test files. Check this awesome [article](https://kentcdodds.com/blog/avoid-nesting-when-youre-testing) written by [Kent C. Dodds](https://kentcdodds.com/) about this topic.
|
||||
|
||||
### No warnings!
|
||||
|
||||
Your tests shouldn't trigger warnings. This is really common when testing async functionality. It's really difficult to read test results when we have a bunch of warnings.
|
||||
|
||||
## React Testing Library (RTL)
|
||||
|
||||
### Keep accessibility in mind when writing your tests
|
||||
|
||||
One of the most important points of RTL is accessibility and this is also a very important point for us. We should try our best to follow the RTL [Priority](https://testing-library.com/docs/queries/about/#priority) when querying for elements in our tests. `getByTestId` is not viewable by the user and should only be used when the element isn't accessible in any other way.
|
||||
|
||||
### Don't use `act` unnecessarily
|
||||
|
||||
`render` and `fireEvent` are already wrapped in `act`, so wrapping them in `act` again is a common mistake. Some solutions to the warnings related to async testing can be found in the RTL docs.
|
||||
|
||||
## Example Test Structure
|
||||
|
||||
```tsx
|
||||
// MyComponent.test.tsx
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { MyComponent } from './MyComponent';
|
||||
|
||||
// ✅ Good - Simple, standalone test
|
||||
test('renders loading state initially', () => {
|
||||
render(<MyComponent />);
|
||||
expect(screen.getByText('Loading...')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// ✅ Good - Tests user interaction
|
||||
test('calls onSubmit when form is submitted', async () => {
|
||||
const user = userEvent.setup();
|
||||
const mockOnSubmit = jest.fn();
|
||||
|
||||
render(<MyComponent onSubmit={mockOnSubmit} />);
|
||||
|
||||
await user.type(screen.getByLabelText('Username'), 'testuser');
|
||||
await user.click(screen.getByRole('button', { name: 'Submit' }));
|
||||
|
||||
expect(mockOnSubmit).toHaveBeenCalledWith({ username: 'testuser' });
|
||||
});
|
||||
|
||||
// ✅ Good - Tests async behavior
|
||||
test('displays error message when API call fails', async () => {
|
||||
const mockFetch = jest.fn().mockRejectedValue(new Error('API Error'));
|
||||
global.fetch = mockFetch;
|
||||
|
||||
render(<MyComponent />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Error: API Error')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Testing Best Practices
|
||||
|
||||
### Use appropriate queries in priority order
|
||||
|
||||
1. **Accessible to everyone**: `getByRole`, `getByLabelText`, `getByPlaceholderText`, `getByText`
|
||||
2. **Semantic queries**: `getByAltText`, `getByTitle`
|
||||
3. **Test IDs**: `getByTestId` (last resort)
|
||||
|
||||
```tsx
|
||||
// ✅ Good - using accessible queries
|
||||
test('user can submit form', () => {
|
||||
render(<LoginForm />);
|
||||
|
||||
const usernameInput = screen.getByLabelText('Username');
|
||||
const passwordInput = screen.getByLabelText('Password');
|
||||
const submitButton = screen.getByRole('button', { name: 'Log in' });
|
||||
|
||||
// Test implementation
|
||||
});
|
||||
|
||||
// ❌ Avoid - using test IDs when better options exist
|
||||
test('user can submit form', () => {
|
||||
render(<LoginForm />);
|
||||
|
||||
const usernameInput = screen.getByTestId('username-input');
|
||||
const passwordInput = screen.getByTestId('password-input');
|
||||
const submitButton = screen.getByTestId('submit-button');
|
||||
|
||||
// Test implementation
|
||||
});
|
||||
```
|
||||
|
||||
### Test behavior, not implementation details
|
||||
|
||||
```tsx
|
||||
// ✅ Good - tests what the user experiences
|
||||
test('shows success message after successful form submission', async () => {
|
||||
render(<ContactForm />);
|
||||
|
||||
await userEvent.type(screen.getByLabelText('Email'), 'test@example.com');
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Submit' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Message sent successfully!')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// ❌ Avoid - testing implementation details
|
||||
test('calls setState when form is submitted', () => {
|
||||
const component = shallow(<ContactForm />);
|
||||
const instance = component.instance();
|
||||
const spy = jest.spyOn(instance, 'setState');
|
||||
|
||||
instance.handleSubmit();
|
||||
expect(spy).toHaveBeenCalled();
|
||||
});
|
||||
```
|
||||
|
||||
### Mock external dependencies appropriately
|
||||
|
||||
```tsx
|
||||
// Mock API calls
|
||||
jest.mock('../api/userService', () => ({
|
||||
getUser: jest.fn(),
|
||||
createUser: jest.fn(),
|
||||
}));
|
||||
|
||||
// Mock components that aren't relevant to the test
|
||||
jest.mock('../Chart/Chart', () => {
|
||||
return function MockChart() {
|
||||
return <div data-testid="mock-chart">Chart Component</div>;
|
||||
};
|
||||
});
|
||||
```
|
||||
|
||||
## Async Testing Patterns
|
||||
|
||||
### Testing async operations
|
||||
|
||||
```tsx
|
||||
test('loads and displays user data', async () => {
|
||||
const mockUser = { id: 1, name: 'John Doe' };
|
||||
const mockGetUser = jest.fn().mockResolvedValue(mockUser);
|
||||
|
||||
render(<UserProfile getUserData={mockGetUser} />);
|
||||
|
||||
// Wait for the async operation to complete
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('John Doe')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
expect(mockGetUser).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
```
|
||||
|
||||
### Testing loading states
|
||||
|
||||
```tsx
|
||||
test('shows loading spinner while fetching data', async () => {
|
||||
const mockGetUser = jest.fn().mockImplementation(
|
||||
() => new Promise(resolve => setTimeout(resolve, 100))
|
||||
);
|
||||
|
||||
render(<UserProfile getUserData={mockGetUser} />);
|
||||
|
||||
expect(screen.getByText('Loading...')).toBeInTheDocument();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByText('Loading...')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Component-Specific Testing
|
||||
|
||||
### Testing form components
|
||||
|
||||
```tsx
|
||||
test('validates required fields', async () => {
|
||||
render(<RegistrationForm />);
|
||||
|
||||
const submitButton = screen.getByRole('button', { name: 'Register' });
|
||||
await userEvent.click(submitButton);
|
||||
|
||||
expect(screen.getByText('Username is required')).toBeInTheDocument();
|
||||
expect(screen.getByText('Email is required')).toBeInTheDocument();
|
||||
});
|
||||
```
|
||||
|
||||
### Testing modals and overlays
|
||||
|
||||
```tsx
|
||||
test('opens and closes modal', async () => {
|
||||
render(<ModalContainer />);
|
||||
|
||||
const openButton = screen.getByRole('button', { name: 'Open Modal' });
|
||||
await userEvent.click(openButton);
|
||||
|
||||
expect(screen.getByRole('dialog')).toBeInTheDocument();
|
||||
|
||||
const closeButton = screen.getByRole('button', { name: 'Close' });
|
||||
await userEvent.click(closeButton);
|
||||
|
||||
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
|
||||
});
|
||||
```
|
||||
|
||||
### Testing with context providers
|
||||
|
||||
```tsx
|
||||
const renderWithTheme = (component: React.ReactElement) => {
|
||||
return render(
|
||||
<ThemeProvider theme={mockTheme}>
|
||||
{component}
|
||||
</ThemeProvider>
|
||||
);
|
||||
};
|
||||
|
||||
test('applies theme colors correctly', () => {
|
||||
renderWithTheme(<ThemedButton />);
|
||||
|
||||
const button = screen.getByRole('button');
|
||||
expect(button).toHaveStyle({
|
||||
backgroundColor: mockTheme.colors.primary,
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Performance Testing
|
||||
|
||||
### Testing with large datasets
|
||||
|
||||
```tsx
|
||||
test('handles large lists efficiently', () => {
|
||||
const largeDataset = Array.from({ length: 10000 }, (_, i) => ({
|
||||
id: i,
|
||||
name: `Item ${i}`,
|
||||
}));
|
||||
|
||||
const { container } = render(<VirtualizedList items={largeDataset} />);
|
||||
|
||||
// Should only render visible items
|
||||
const renderedItems = container.querySelectorAll('[data-testid="list-item"]');
|
||||
expect(renderedItems.length).toBeLessThan(100);
|
||||
});
|
||||
```
|
||||
|
||||
## Testing Accessibility
|
||||
|
||||
```tsx
|
||||
test('is accessible to screen readers', () => {
|
||||
render(<AccessibleForm />);
|
||||
|
||||
const form = screen.getByRole('form');
|
||||
const inputs = screen.getAllByRole('textbox');
|
||||
|
||||
inputs.forEach(input => {
|
||||
expect(input).toHaveAttribute('aria-label');
|
||||
});
|
||||
|
||||
expect(form).toHaveAttribute('aria-describedby');
|
||||
});
|
||||
```
|
||||
Reference in New Issue
Block a user