Refactor dashboard API response to reduce payload size and improve performance
- Removed serialization of full account, contact, lead, and opportunity objects from the dashboard API response. - Added new fields for urgent counts, pipeline by stage, revenue metrics, hot leads, tasks, activities, and goal summary. - Updated the context building logic to only include counts and essential data. Enhance JWT signing key validation in settings - Added a check to ensure the SECRET_KEY is at least 32 bytes for HS256 JWT signing. - Introduced warnings for development environments when the key is shorter than required. Update environment variable documentation - Clarified the requirements for the SECRET_KEY in the environment variables documentation, emphasizing the minimum length for security. Revise mobile app API configuration for self-hosting - Updated the mobile app to allow self-hosting by using `--dart-define` for API_BASE_URL and GOOGLE_SERVER_CLIENT_ID. - Ensured that the app enforces HTTPS in release builds to secure JWT transmission. Implement entity lookup for task linking in mobile app - Created a new EntityLookup model to handle task attachments to accounts, leads, opportunities, and tickets. - Refactored task form screen to utilize the new lookup mechanism, improving the user experience by providing real-time loading states and error handling. Add unit tests for new features and configurations - Implemented unit tests for API configuration, entity lookup, and task link picker to ensure functionality and reliability.
This commit is contained in:
Ashwin
parent
1da98d92cf
commit
ab27ea85fd
15 changed files with 960 additions and 179 deletions
|
|
@ -103,6 +103,8 @@ Create a `.env` file in the `backend/` directory:
|
|||
|
||||
```env
|
||||
# Django
|
||||
# At least 32 bytes: this key also signs the JWTs, and HS256 needs one that long.
|
||||
# python -c "import secrets; print(secrets.token_urlsafe(48))"
|
||||
SECRET_KEY=your-secret-key-here
|
||||
ENV_TYPE=dev
|
||||
|
||||
|
|
@ -423,7 +425,7 @@ uv lock --upgrade
|
|||
|
||||
| Variable | Description |
|
||||
|----------|-------------|
|
||||
| `SECRET_KEY` | Django secret key |
|
||||
| `SECRET_KEY` | Django secret key, and the JWT signing key. At least 32 bytes |
|
||||
| `ENV_TYPE` | Environment type (`dev` or `prod`) |
|
||||
| `DBNAME` | PostgreSQL database name |
|
||||
| `DBUSER` | PostgreSQL username |
|
||||
|
|
|
|||
|
|
@ -315,3 +315,71 @@ class TestDashboardScopedMetrics:
|
|||
from common.views.dashboard_views import ApiHomeView
|
||||
|
||||
assert HasOrgContext in ApiHomeView.permission_classes
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class TestDashboardSendsOnlyWhatItRenders:
|
||||
"""The dashboard used to serialize four whole tables nobody read.
|
||||
|
||||
Measured against the seeded org: 384 KB of response, of which
|
||||
``opportunities`` (162 KB), ``accounts`` (86 KB), ``leads`` (84 KB) and
|
||||
``contacts`` (41 KB) were 97%. The only caller is the mobile dashboard, and
|
||||
its model reads counts, ``urgent_counts``, ``pipeline_by_stage``,
|
||||
``revenue_metrics``, ``hot_leads``, ``tasks``, ``activities`` and
|
||||
``goal_summary``. It never touched the four lists.
|
||||
|
||||
They were also unpaged, so the payload grew with the org forever.
|
||||
"""
|
||||
|
||||
url = "/api/dashboard/"
|
||||
|
||||
# Note the trap here: ``hot_leads`` is a list on the response AND a count
|
||||
# inside ``urgent_counts``. Only the top-level list survives.
|
||||
DROPPED = ("accounts", "contacts", "leads", "opportunities")
|
||||
|
||||
KEPT = (
|
||||
"accounts_count",
|
||||
"contacts_count",
|
||||
"leads_count",
|
||||
"opportunities_count",
|
||||
"urgent_counts",
|
||||
"pipeline_by_stage",
|
||||
"revenue_metrics",
|
||||
"hot_leads",
|
||||
"tasks",
|
||||
"activities",
|
||||
"goal_summary",
|
||||
)
|
||||
|
||||
def test_the_four_unread_lists_are_gone(self, admin_client, org_a):
|
||||
Account.objects.create(name="Acc1", org=org_a)
|
||||
Contact.objects.create(first_name="Con", last_name="Tact", org=org_a)
|
||||
Lead.objects.create(
|
||||
first_name="Lead", last_name="One", email="lead1@test.com", org=org_a
|
||||
)
|
||||
Opportunity.objects.create(name="Deal", org=org_a, stage="PROSPECTING")
|
||||
|
||||
response = admin_client.get(self.url)
|
||||
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
for key in self.DROPPED:
|
||||
assert key not in response.data, f"{key} is back in the response"
|
||||
|
||||
def test_everything_a_client_reads_is_still_there(self, admin_client, org_a):
|
||||
"""The other direction. Trimming is only safe while this passes."""
|
||||
response = admin_client.get(self.url)
|
||||
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
for key in self.KEPT:
|
||||
assert key in response.data, f"{key} disappeared"
|
||||
|
||||
def test_the_counts_still_count_the_rows_that_were_dropped(
|
||||
self, admin_client, org_a
|
||||
):
|
||||
"""The querysets stayed; only the serialization went."""
|
||||
Account.objects.create(name="Acc1", org=org_a)
|
||||
Account.objects.create(name="Acc2", org=org_a)
|
||||
|
||||
response = admin_client.get(self.url)
|
||||
|
||||
assert response.data["accounts_count"] == 2
|
||||
|
|
|
|||
103
backend/common/tests/test_signing_key_strength.py
Normal file
103
backend/common/tests/test_signing_key_strength.py
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
"""``SECRET_KEY`` signs every JWT, so its length is a security control.
|
||||
|
||||
``SIMPLE_JWT["SIGNING_KEY"]`` is ``SECRET_KEY``, and the algorithm is HS256.
|
||||
RFC 7518 section 3.2 requires an HMAC key at least as long as the hash function
|
||||
output, which is 32 bytes for SHA-256. PyJWT does not refuse a shorter key; it
|
||||
emits a warning, once per call, into a log nobody reads. The local ``.env`` here
|
||||
carried a 20-byte key for the life of the project and every token this server
|
||||
issued was signed with it.
|
||||
|
||||
``crm/settings.py`` now raises on a short key outside dev and warns inside it.
|
||||
These tests import the settings module in a subprocess, because the check runs at
|
||||
import time and reloading it inside the test process would rewrite settings for
|
||||
everything that runs afterwards.
|
||||
"""
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
BACKEND_DIR = Path(__file__).resolve().parents[2]
|
||||
|
||||
# 32 bytes exactly, the floor. Not a real key: it never signs anything, the
|
||||
# subprocess only imports the module and exits.
|
||||
KEY_AT_FLOOR = "k" * 32
|
||||
KEY_BELOW_FLOOR = "k" * 31
|
||||
|
||||
|
||||
def _import_settings(secret_key, env_type):
|
||||
"""Import ``crm.settings`` in a clean subprocess and report how it went.
|
||||
|
||||
``load_dotenv()`` does not override variables already in the environment, so
|
||||
what is passed here wins over the developer's own ``.env``.
|
||||
"""
|
||||
env = {
|
||||
key: value
|
||||
for key, value in os.environ.items()
|
||||
if key not in ("SECRET_KEY", "ENV_TYPE")
|
||||
}
|
||||
env["SECRET_KEY"] = secret_key
|
||||
env["ENV_TYPE"] = env_type
|
||||
return subprocess.run(
|
||||
[sys.executable, "-W", "always", "-c", "import crm.settings"],
|
||||
cwd=BACKEND_DIR,
|
||||
env=env,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
|
||||
|
||||
class TestAShortSigningKeyIsRefusedOutsideDev:
|
||||
def test_a_31_byte_key_fails_the_import(self):
|
||||
result = _import_settings(KEY_BELOW_FLOOR, "production")
|
||||
|
||||
assert result.returncode != 0
|
||||
assert "31 bytes" in result.stderr
|
||||
assert "at least 32" in result.stderr
|
||||
|
||||
def test_the_error_names_the_command_that_produces_a_good_key(self):
|
||||
"""A refusal that does not say what to do next is a support ticket."""
|
||||
result = _import_settings(KEY_BELOW_FLOOR, "production")
|
||||
|
||||
assert "secrets.token_urlsafe" in result.stderr
|
||||
|
||||
def test_a_32_byte_key_is_accepted(self):
|
||||
"""The other direction: the check has to be able to pass."""
|
||||
result = _import_settings(KEY_AT_FLOOR, "production")
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
|
||||
def test_a_long_key_is_accepted(self):
|
||||
result = _import_settings("k" * 64, "production")
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
|
||||
|
||||
class TestDevIsWarnedRatherThanBlocked:
|
||||
def test_a_short_key_warns_in_dev_and_still_imports(self):
|
||||
"""Blocking dev would mean every existing checkout stops running."""
|
||||
result = _import_settings(KEY_BELOW_FLOOR, "dev")
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert "31 bytes" in result.stderr
|
||||
|
||||
def test_no_warning_in_dev_once_the_key_is_long_enough(self):
|
||||
result = _import_settings(KEY_AT_FLOOR, "dev")
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
assert "SECRET_KEY" not in result.stderr
|
||||
|
||||
|
||||
class TestTheKeyIsMeasuredInBytesNotCharacters:
|
||||
def test_a_31_character_key_of_multibyte_characters_passes(self):
|
||||
"""31 characters, 62 bytes. The RFC counts bytes, and so does HMAC.
|
||||
|
||||
Measuring `len(SECRET_KEY)` instead would reject this key, and would
|
||||
accept a 32-character key that was somehow shorter in bytes. Python
|
||||
strings cannot be shorter in bytes than in characters, so only the
|
||||
false rejection is reachable, which is the one this pins.
|
||||
"""
|
||||
result = _import_settings("é" * 31, "production")
|
||||
|
||||
assert result.returncode == 0, result.stderr
|
||||
|
|
@ -10,19 +10,15 @@ from rest_framework.response import Response
|
|||
from rest_framework.views import APIView
|
||||
|
||||
from accounts.models import Account
|
||||
from accounts.serializer import AccountSerializer
|
||||
from cases.models import Case
|
||||
from common import serializer, swagger_params
|
||||
from common.models import Activity
|
||||
from common.permissions import HasOrgContext, is_org_admin
|
||||
from common.utils import STAGES
|
||||
from contacts.models import Contact
|
||||
from contacts.serializer import ContactSerializer
|
||||
from invoices.models import UNPAID_STATUSES, Invoice
|
||||
from leads.models import Lead
|
||||
from leads.serializer import LeadSerializer
|
||||
from opportunity.models import Opportunity, StageAgingConfig
|
||||
from opportunity.serializer import OpportunitySerializer
|
||||
from opportunity.workflow import DEFAULT_STAGE_EXPECTED_DAYS, ROTTEN_MULTIPLIER
|
||||
from tasks.models import Task
|
||||
from tasks.serializer import TaskSerializer
|
||||
|
|
@ -94,10 +90,13 @@ class ApiHomeView(APIView):
|
|||
"contacts_count": serializers.IntegerField(),
|
||||
"leads_count": serializers.IntegerField(),
|
||||
"opportunities_count": serializers.IntegerField(),
|
||||
"accounts": AccountSerializer(many=True),
|
||||
"contacts": ContactSerializer(many=True),
|
||||
"leads": LeadSerializer(many=True),
|
||||
"opportunities": OpportunitySerializer(many=True),
|
||||
"urgent_counts": serializers.DictField(),
|
||||
"pipeline_by_stage": serializers.DictField(),
|
||||
"revenue_metrics": serializers.DictField(),
|
||||
"hot_leads": serializers.ListField(),
|
||||
"tasks": TaskSerializer(many=True),
|
||||
"activities": serializer.DashboardActivitySerializer(many=True),
|
||||
"goal_summary": serializers.ListField(),
|
||||
},
|
||||
)
|
||||
},
|
||||
|
|
@ -126,16 +125,17 @@ class ApiHomeView(APIView):
|
|||
opportunities = _owned_or_assigned(opportunities, profile)
|
||||
tasks = _owned_or_assigned(tasks, profile)
|
||||
|
||||
# Build base context (existing)
|
||||
# Counts only. This used to serialize every account, contact, lead and
|
||||
# opportunity in the org in full beside them: 372 KB of a 384 KB
|
||||
# response, measured against the seeded org, none of which any caller
|
||||
# read. The screens below want counts, the pipeline, the urgent numbers,
|
||||
# ten hot leads and ten tasks. Whoever needs a list calls its own
|
||||
# endpoint, which pages; these four never did.
|
||||
context = {}
|
||||
context["accounts_count"] = accounts.count()
|
||||
context["contacts_count"] = contacts.count()
|
||||
context["leads_count"] = leads.count()
|
||||
context["opportunities_count"] = opportunities.count()
|
||||
context["accounts"] = AccountSerializer(accounts, many=True).data
|
||||
context["contacts"] = ContactSerializer(contacts, many=True).data
|
||||
context["leads"] = LeadSerializer(leads, many=True).data
|
||||
context["opportunities"] = OpportunitySerializer(opportunities, many=True).data
|
||||
|
||||
# NEW: Urgent counts for Focus Bar
|
||||
overdue_tasks = tasks.filter(
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import os
|
||||
import warnings
|
||||
from datetime import timedelta
|
||||
|
||||
from corsheaders.defaults import default_headers
|
||||
|
|
@ -12,10 +13,32 @@ load_dotenv()
|
|||
# SECURITY WARNING: keep the secret key used in production secret!
|
||||
SECRET_KEY = os.environ.get("SECRET_KEY", "django-insecure-dev-key-please-change-in-production")
|
||||
|
||||
IS_DEV_ENV = os.environ.get("ENV_TYPE", "dev") == "dev"
|
||||
|
||||
if not SECRET_KEY or SECRET_KEY.startswith("django-insecure"):
|
||||
if os.environ.get("ENV_TYPE", "dev") != "dev":
|
||||
if not IS_DEV_ENV:
|
||||
raise ValueError("SECRET_KEY must be set to a secure value in non-dev environments")
|
||||
|
||||
# This key also signs every JWT (see SIMPLE_JWT["SIGNING_KEY"] below). RFC 7518
|
||||
# section 3.2 requires an HMAC key at least as long as the hash it produces, so
|
||||
# HS256 needs 32 bytes. PyJWT only warns about a shorter one, on every call, and
|
||||
# a warning nobody reads is not a control.
|
||||
#
|
||||
# Raising here means a deploy fails rather than quietly signing weak tokens.
|
||||
# Lengthening the key invalidates every access and refresh token already issued,
|
||||
# so everyone is signed out once when it changes.
|
||||
JWT_MIN_SIGNING_KEY_BYTES = 32
|
||||
|
||||
if len(SECRET_KEY.encode()) < JWT_MIN_SIGNING_KEY_BYTES:
|
||||
message = (
|
||||
f"SECRET_KEY is {len(SECRET_KEY.encode())} bytes. It signs every JWT, "
|
||||
f"and HS256 needs at least {JWT_MIN_SIGNING_KEY_BYTES}. Generate one with "
|
||||
"`python -c \"import secrets; print(secrets.token_urlsafe(48))\"`."
|
||||
)
|
||||
if not IS_DEV_ENV:
|
||||
raise ValueError(message)
|
||||
warnings.warn(message, RuntimeWarning, stacklevel=2)
|
||||
|
||||
# SECURITY WARNING: don't run with debug turned on in production!
|
||||
DEBUG = os.environ.get("DEBUG", "False").lower() == "true"
|
||||
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ itself, so they aren't in the table below:
|
|||
|
||||
| Variable | Default | Required | Purpose |
|
||||
| --- | --- | --- | --- |
|
||||
| `SECRET_KEY` | `django-insecure-dev-key-please-change-in-production` | Yes, once `ENV_TYPE` is anything other than `dev` (see above) | Django's cryptographic signing key. It also *is* the JWT `SIGNING_KEY`: `SIMPLE_JWT["SIGNING_KEY"]` is set to `SECRET_KEY` directly, so there is no separate JWT signing key setting anywhere in this codebase, and rotating `SECRET_KEY` invalidates every outstanding access and refresh token. See [Architecture: Authentication](../architecture/authentication.md#token-model) and [Security hardening](../self-hosting/security-hardening.md#secrets). |
|
||||
| `SECRET_KEY` | `django-insecure-dev-key-please-change-in-production` | Yes, once `ENV_TYPE` is anything other than `dev` (see above) | Django's cryptographic signing key. It also *is* the JWT `SIGNING_KEY`: `SIMPLE_JWT["SIGNING_KEY"]` is set to `SECRET_KEY` directly, so there is no separate JWT signing key setting anywhere in this codebase, and rotating `SECRET_KEY` invalidates every outstanding access and refresh token. **It must be at least 32 bytes.** HS256 signs the JWTs, and RFC 7518 section 3.2 requires an HMAC key at least as long as the hash output; a shorter one raises at import time outside `dev` and warns inside it. Generate one with `python -c "import secrets; print(secrets.token_urlsafe(48))"`. See [Architecture: Authentication](../architecture/authentication.md#token-model) and [Security hardening](../self-hosting/security-hardening.md#secrets). |
|
||||
| `DEBUG` | `False` | No | Django debug mode. Also the sole gate on the `devlogin` management command. See [Management commands](management-commands.md#devlogin). |
|
||||
| `ALLOWED_HOSTS` | `localhost,127.0.0.1` | Yes, once serving traffic under a real hostname | Comma-separated list, split on `,` into Django's `ALLOWED_HOSTS`. |
|
||||
| `TRUST_PROXY_SSL_HEADER` | `False` | Yes, if TLS is terminated by a proxy in front of Django | Set to `true` (case-insensitive) to set Django's `SECURE_PROXY_SSL_HEADER` to `("HTTP_X_FORWARDED_PROTO", "https")`. Without it, a request arriving from nginx or an ALB over plain HTTP makes `request.is_secure()` false, and `SecurityMiddleware` then emits **no** `Strict-Transport-Security` header at all, so the `SECURE_HSTS_SECONDS` / `INCLUDE_SUBDOMAINS` / `PRELOAD` settings are silently inert. Only turn it on when that proxy is the sole route to the app **and** it overwrites `X-Forwarded-Proto` rather than passing through a client-supplied one, because otherwise any caller reaching Django directly can make `request.is_secure()` return true. The http-to-https redirect belongs on the proxy; `SECURE_SSL_REDIRECT` is deliberately not set. |
|
||||
|
|
|
|||
|
|
@ -134,11 +134,26 @@ BottleCRM Mobile is designed to streamline your sales and customer management pr
|
|||
4. Configure OAuth consent screen
|
||||
|
||||
#### Environment Configuration
|
||||
The app automatically switches between environments:
|
||||
- **Debug builds**: Uses development API (ngrok URL)
|
||||
- **Release builds**: Uses production API (bottlecrm.io)
|
||||
With no build flags, the app switches on build mode:
|
||||
- **Debug builds**: development API
|
||||
- **Release builds**: production API (bottlecrm.io)
|
||||
|
||||
You can modify URLs in `lib/config/api_config.dart`
|
||||
Self-hosting points a build at your own server without editing source:
|
||||
|
||||
```bash
|
||||
flutter build apk --release \
|
||||
--dart-define=API_BASE_URL=https://crm.example.com \
|
||||
--dart-define=GOOGLE_SERVER_CLIENT_ID=<your-web-client-id>.apps.googleusercontent.com
|
||||
```
|
||||
|
||||
`API_BASE_URL` must start with `https://` in a release build. The app refuses to
|
||||
start on a plain-HTTP address rather than send its JWTs unencrypted; `http://` is
|
||||
allowed in debug, where a local Django lives. A trailing slash is trimmed.
|
||||
|
||||
Both settings are compile-time (`String.fromEnvironment`). Deliberately so: a
|
||||
runtime setting would let anything that can write the app's storage point every
|
||||
request, and every token, at a server of its choosing. The defaults still live in
|
||||
`lib/config/api_config.dart`.
|
||||
|
||||
## 🛠 Development
|
||||
|
||||
|
|
@ -312,17 +327,17 @@ For detailed architectural guidelines, see [CLAUDE.md](CLAUDE.md) and [.github/c
|
|||
|
||||
### API Configuration
|
||||
|
||||
The application uses environment-based API URLs configured in `lib/config/api_config.dart`:
|
||||
Every endpoint is built from one host, resolved in `lib/config/api_config.dart`:
|
||||
|
||||
```dart
|
||||
// Development (debug builds)
|
||||
static const String _developmentUrl = 'https://b2ad5166b831.ngrok-free.app';
|
||||
|
||||
// Production (release builds)
|
||||
static const String _productionUrl = 'https://api.bottlecrm.io';
|
||||
```bash
|
||||
# Point a build anywhere. Release builds must be https.
|
||||
flutter run --dart-define=API_BASE_URL=http://10.0.2.2:8000
|
||||
```
|
||||
|
||||
**Environment Detection**: Automatic switching based on `kDebugMode` flag
|
||||
With no `API_BASE_URL`, the host comes from `kDebugMode`: the development server
|
||||
in debug, `https://api.bottlecrm.io` in release. See
|
||||
[Environment Configuration](#environment-configuration) above for the release
|
||||
build flags and the plain-HTTP rule.
|
||||
|
||||
### Authentication Setup
|
||||
|
||||
|
|
@ -474,7 +489,8 @@ flutter doctor -v
|
|||
#### API Connection Issues
|
||||
```bash
|
||||
# Problem: API requests failing in debug mode
|
||||
# Solution: Check ngrok tunnel status and update URL in api_config.dart
|
||||
# Solution: Point the build at a server you control, no source edit needed
|
||||
flutter run --dart-define=API_BASE_URL=http://10.0.2.2:8000
|
||||
|
||||
# Problem: 401 Unauthorized errors
|
||||
# Solution: Clear app data and re-authenticate
|
||||
|
|
|
|||
|
|
@ -20,8 +20,65 @@ class ApiConfig {
|
|||
/// which would drift the moment either is edited.
|
||||
static const String marketingSite = 'https://bottlecrm.io';
|
||||
|
||||
/// Get the current base URL based on build mode
|
||||
static String get baseUrl => kDebugMode ? _developmentUrl : _productionUrl;
|
||||
/// Build-time override, so somebody self-hosting points a build at their own
|
||||
/// server without editing this file:
|
||||
///
|
||||
/// flutter build apk --release --dart-define=API_BASE_URL=https://crm.example.com
|
||||
///
|
||||
/// Compile-time on purpose. A runtime setting would mean anything able to
|
||||
/// write the app's storage could redirect every request, and every JWT with
|
||||
/// it, to a server of its own choosing.
|
||||
static const String _baseUrlOverride = String.fromEnvironment('API_BASE_URL');
|
||||
|
||||
/// Google's Web OAuth client, the audience the backend's `GOOGLE_CLIENT_ID`
|
||||
/// verifies against. Overridable the same way, because a self-hoster signs in
|
||||
/// against their own Google project:
|
||||
///
|
||||
/// --dart-define=GOOGLE_SERVER_CLIENT_ID=<id>.apps.googleusercontent.com
|
||||
///
|
||||
/// Not a secret. A client id is public by design; the backend is what decides
|
||||
/// whether an ID token is acceptable.
|
||||
static const String googleServerClientId = String.fromEnvironment(
|
||||
'GOOGLE_SERVER_CLIENT_ID',
|
||||
defaultValue:
|
||||
'1072513761792-p59rct7b1c3go7l58e51r3geuqff2tfl.apps.googleusercontent.com',
|
||||
);
|
||||
|
||||
/// The API host this build talks to.
|
||||
static final String baseUrl = resolveBaseUrl(
|
||||
override: _baseUrlOverride,
|
||||
isDebug: kDebugMode,
|
||||
);
|
||||
|
||||
/// The override rules, separated from the build constants so both branches
|
||||
/// can be tested. `flutter test` runs one build, so a `--dart-define` the
|
||||
/// suite does not pass is a branch no test could otherwise reach.
|
||||
@visibleForTesting
|
||||
static String resolveBaseUrl({
|
||||
required String override,
|
||||
required bool isDebug,
|
||||
}) {
|
||||
if (override.isEmpty) {
|
||||
return isDebug ? _developmentUrl : _productionUrl;
|
||||
}
|
||||
// A trailing slash would produce `https://host//api`, and the resulting 404
|
||||
// on every request says nothing about the cause.
|
||||
final trimmed = override.endsWith('/')
|
||||
? override.substring(0, override.length - 1)
|
||||
: override;
|
||||
// A release build talking plain HTTP puts every JWT on the wire in the
|
||||
// clear. Refusing loudly beats falling back to the default, which would
|
||||
// leave a self-hoster looking at somebody else's server and no explanation.
|
||||
// `http://` stays allowed in debug: that is how anyone runs against a local
|
||||
// Django.
|
||||
if (!isDebug && !trimmed.startsWith('https://')) {
|
||||
throw StateError(
|
||||
'API_BASE_URL must start with https:// in a release build. '
|
||||
'Got "$trimmed", which would send every token unencrypted.',
|
||||
);
|
||||
}
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
/// API base path
|
||||
static String get apiBaseUrl => '$baseUrl/api';
|
||||
|
|
|
|||
|
|
@ -191,3 +191,65 @@ class TagLookup {
|
|||
@override
|
||||
int get hashCode => id.hashCode;
|
||||
}
|
||||
|
||||
/// A record a task can be attached to: one id, one label, nothing else.
|
||||
///
|
||||
/// The task form used to source these from the leads, deals and tickets list
|
||||
/// providers, which hold whatever page the corresponding screen last loaded
|
||||
/// under whatever filter it last applied. On a cold app those are empty, so the
|
||||
/// picker said "Nothing to pick" for an org with twenty leads.
|
||||
class EntityLookup {
|
||||
final String id;
|
||||
final String label;
|
||||
|
||||
const EntityLookup({required this.id, required this.label});
|
||||
|
||||
/// A record with one naming field. [labelKeys] are tried in order and the
|
||||
/// first non-empty one wins.
|
||||
factory EntityLookup.fromJson(
|
||||
Map<String, dynamic> json, {
|
||||
required List<String> labelKeys,
|
||||
}) {
|
||||
return EntityLookup(
|
||||
id: json['id']?.toString() ?? '',
|
||||
label: _firstNonEmpty(json, labelKeys) ?? 'Untitled',
|
||||
);
|
||||
}
|
||||
|
||||
/// A person, whose name is two optional fields rather than one. Neither is
|
||||
/// required on a lead, so [fallbackKeys] is what keeps an unnamed row
|
||||
/// selectable instead of blank.
|
||||
factory EntityLookup.person(
|
||||
Map<String, dynamic> json, {
|
||||
List<String> fallbackKeys = const [],
|
||||
}) {
|
||||
final name = [
|
||||
json['first_name']?.toString().trim() ?? '',
|
||||
json['last_name']?.toString().trim() ?? '',
|
||||
].where((part) => part.isNotEmpty).join(' ');
|
||||
return EntityLookup(
|
||||
id: json['id']?.toString() ?? '',
|
||||
label: name.isNotEmpty
|
||||
? name
|
||||
: (_firstNonEmpty(json, fallbackKeys) ?? 'Unnamed'),
|
||||
);
|
||||
}
|
||||
|
||||
static String? _firstNonEmpty(Map<String, dynamic> json, List<String> keys) {
|
||||
for (final key in keys) {
|
||||
final value = json[key]?.toString().trim() ?? '';
|
||||
if (value.isNotEmpty) return value;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@override
|
||||
bool operator ==(Object other) =>
|
||||
identical(this, other) ||
|
||||
other is EntityLookup &&
|
||||
runtimeType == other.runtimeType &&
|
||||
id == other.id;
|
||||
|
||||
@override
|
||||
int get hashCode => id.hashCode;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import 'package:flutter/foundation.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import '../config/api_config.dart';
|
||||
import '../data/api_envelope.dart';
|
||||
|
|
@ -15,6 +16,26 @@ import '../services/api_service.dart';
|
|||
|
||||
final ApiService _apiService = ApiService();
|
||||
|
||||
/// How many rows a picker asks for.
|
||||
///
|
||||
/// Every paginated list endpoint here uses DRF's `LimitOffsetPagination` with
|
||||
/// `PAGE_SIZE = 10`, so a lookup that sent no `limit` offered ten rows and no
|
||||
/// hint that there were more. Confirmed against the seeded org: `/api/contacts/`
|
||||
/// answered `contacts_count: 15` beside ten rows, and five contacts could not be
|
||||
/// picked on any form in the app. Tags were 10 of 14.
|
||||
///
|
||||
/// 500 is a cap rather than a fix. An org past it still cannot pick everything,
|
||||
/// and the real answer is a searching picker backed by the server. This number
|
||||
/// is what makes the gap rare instead of routine.
|
||||
const int lookupPageLimit = 500;
|
||||
|
||||
/// The URL a lookup actually requests. Public only so a test can assert the
|
||||
/// limit is on it: without one every picker in the app silently offers ten rows.
|
||||
@visibleForTesting
|
||||
String lookupUrl(String url) => Uri.parse(
|
||||
url,
|
||||
).replace(queryParameters: {'limit': '$lookupPageLimit'}).toString();
|
||||
|
||||
/// Accounts
|
||||
class AccountsLookupNotifier extends AsyncNotifier<List<AccountLookup>> {
|
||||
@override
|
||||
|
|
@ -26,7 +47,7 @@ class AccountsLookupNotifier extends AsyncNotifier<List<AccountLookup>> {
|
|||
}
|
||||
|
||||
Future<List<AccountLookup>> _fetch() async {
|
||||
final response = await _apiService.get(ApiConfig.accounts);
|
||||
final response = await _apiService.get(lookupUrl(ApiConfig.accounts));
|
||||
if (!response.success || response.data == null) {
|
||||
throw Exception(response.message ?? 'Failed to load accounts');
|
||||
}
|
||||
|
|
@ -59,7 +80,7 @@ class ContactsLookupNotifier extends AsyncNotifier<List<ContactLookup>> {
|
|||
}
|
||||
|
||||
Future<List<ContactLookup>> _fetch() async {
|
||||
final response = await _apiService.get(ApiConfig.contacts);
|
||||
final response = await _apiService.get(lookupUrl(ApiConfig.contacts));
|
||||
if (!response.success || response.data == null) {
|
||||
throw Exception(response.message ?? 'Failed to load contacts');
|
||||
}
|
||||
|
|
@ -142,7 +163,7 @@ class TagsLookupNotifier extends AsyncNotifier<List<TagLookup>> {
|
|||
}
|
||||
|
||||
Future<List<TagLookup>> _fetch() async {
|
||||
final response = await _apiService.get(ApiConfig.tags);
|
||||
final response = await _apiService.get(lookupUrl(ApiConfig.tags));
|
||||
if (!response.success || response.data == null) {
|
||||
throw Exception(response.message ?? 'Failed to load tags');
|
||||
}
|
||||
|
|
@ -158,6 +179,79 @@ final tagsLookupProvider =
|
|||
TagsLookupNotifier.new,
|
||||
);
|
||||
|
||||
/// The three records a task can be attached to that are not accounts.
|
||||
///
|
||||
/// Each has a full module provider already, and the task form used to read
|
||||
/// those. It could not: they page (20 rows) and they carry whatever search and
|
||||
/// filters their list screen last applied, so a task form opened after filtering
|
||||
/// deals to Won offered only won deals, and a task form opened on a cold app
|
||||
/// offered nothing at all with the message "Open this section on the web or
|
||||
/// create one first."
|
||||
class _EntityLookupNotifier extends AsyncNotifier<List<EntityLookup>> {
|
||||
_EntityLookupNotifier({
|
||||
required this.url,
|
||||
required this.paths,
|
||||
required this.parse,
|
||||
required this.what,
|
||||
});
|
||||
|
||||
final String url;
|
||||
final List<String> paths;
|
||||
final EntityLookup Function(Map<String, dynamic>) parse;
|
||||
final String what;
|
||||
|
||||
@override
|
||||
Future<List<EntityLookup>> build() => _fetch();
|
||||
|
||||
Future<void> refresh() async {
|
||||
state = const AsyncValue.loading();
|
||||
state = await AsyncValue.guard(_fetch);
|
||||
}
|
||||
|
||||
Future<List<EntityLookup>> _fetch() async {
|
||||
final response = await _apiService.get(lookupUrl(url));
|
||||
if (!response.success || response.data == null) {
|
||||
throw Exception(response.message ?? 'Failed to load $what');
|
||||
}
|
||||
return listFromEnvelope(response.data!, paths).map(parse).toList();
|
||||
}
|
||||
}
|
||||
|
||||
/// Leads. Rows are nested under `open_leads.open_leads`, the same shape trap the
|
||||
/// accounts lookup hit. Closed leads sit under `close_leads` and are not offered.
|
||||
final leadsLookupProvider =
|
||||
AsyncNotifierProvider<_EntityLookupNotifier, List<EntityLookup>>(
|
||||
() => _EntityLookupNotifier(
|
||||
url: ApiConfig.leads,
|
||||
paths: const ['open_leads.open_leads'],
|
||||
parse: (json) => EntityLookup.person(
|
||||
json,
|
||||
fallbackKeys: const ['email', 'company_name'],
|
||||
),
|
||||
what: 'leads',
|
||||
),
|
||||
);
|
||||
|
||||
final opportunitiesLookupProvider =
|
||||
AsyncNotifierProvider<_EntityLookupNotifier, List<EntityLookup>>(
|
||||
() => _EntityLookupNotifier(
|
||||
url: ApiConfig.opportunities,
|
||||
paths: const ['opportunities'],
|
||||
parse: (json) => EntityLookup.fromJson(json, labelKeys: const ['name']),
|
||||
what: 'deals',
|
||||
),
|
||||
);
|
||||
|
||||
final ticketsLookupProvider =
|
||||
AsyncNotifierProvider<_EntityLookupNotifier, List<EntityLookup>>(
|
||||
() => _EntityLookupNotifier(
|
||||
url: ApiConfig.tickets,
|
||||
paths: const ['cases'],
|
||||
parse: (json) => EntityLookup.fromJson(json, labelKeys: const ['name']),
|
||||
what: 'tickets',
|
||||
),
|
||||
);
|
||||
|
||||
/// Convenience wrappers. Return the resolved list (or empty during
|
||||
/// load/error). Forms that don't care about loading state watch these.
|
||||
/// Picker options, resolved. Named for what they are rather than for the
|
||||
|
|
@ -183,6 +277,38 @@ final teamsProvider = Provider<List<TeamLookup>>((ref) {
|
|||
return ref.watch(teamsLookupProvider).value ?? const [];
|
||||
});
|
||||
|
||||
/// The four kinds a task can be attached to, in one shape.
|
||||
///
|
||||
/// These keep the `AsyncValue` rather than flattening to a list like the
|
||||
/// wrappers above, because the picker that reads them has to tell "still
|
||||
/// loading" apart from "this org has none". Flattening is what made a cold app
|
||||
/// claim there was nothing to link to.
|
||||
final accountEntityOptionsProvider = Provider<AsyncValue<List<EntityLookup>>>((
|
||||
ref,
|
||||
) {
|
||||
return ref
|
||||
.watch(accountsLookupProvider)
|
||||
.whenData(
|
||||
(accounts) => [
|
||||
for (final account in accounts)
|
||||
EntityLookup(id: account.id, label: account.name),
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
final leadEntityOptionsProvider = Provider<AsyncValue<List<EntityLookup>>>(
|
||||
(ref) => ref.watch(leadsLookupProvider),
|
||||
);
|
||||
|
||||
final opportunityEntityOptionsProvider =
|
||||
Provider<AsyncValue<List<EntityLookup>>>(
|
||||
(ref) => ref.watch(opportunitiesLookupProvider),
|
||||
);
|
||||
|
||||
final ticketEntityOptionsProvider = Provider<AsyncValue<List<EntityLookup>>>(
|
||||
(ref) => ref.watch(ticketsLookupProvider),
|
||||
);
|
||||
|
||||
/// Custom field definitions, keyed by target_model (Case, Lead, ...).
|
||||
///
|
||||
/// Each form watches the family for its entity; results are cached per-target
|
||||
|
|
|
|||
|
|
@ -4,11 +4,8 @@ import 'package:go_router/go_router.dart';
|
|||
import 'package:lucide_icons_flutter/lucide_icons.dart';
|
||||
import '../../core/theme/theme.dart';
|
||||
import '../../data/models/models.dart';
|
||||
import '../../providers/deals_provider.dart';
|
||||
import '../../providers/leads_provider.dart';
|
||||
import '../../providers/lookup_provider.dart';
|
||||
import '../../providers/tasks_provider.dart';
|
||||
import '../../providers/tickets_provider.dart';
|
||||
import '../../widgets/common/common.dart';
|
||||
import '../../widgets/forms/unsaved_changes.dart';
|
||||
import '../../widgets/forms/custom_fields_form.dart';
|
||||
|
|
@ -134,41 +131,38 @@ class _TaskFormScreenState extends ConsumerState<TaskFormScreen> {
|
|||
}
|
||||
}
|
||||
|
||||
// Resolve the label for the currently-linked entity from cached lookup
|
||||
// providers. The TaskSerializer returns FKs as plain UUIDs (not nested
|
||||
// objects), so we have no embedded name on the edit fetch. We look it up
|
||||
// in the same provider used by the picker. Returns null if the cache
|
||||
// doesn't have it yet (provider not loaded); the UI falls back to a
|
||||
// generic "Selected" hint until the provider warms up.
|
||||
/// The options for one kind, as a still-loading-aware value.
|
||||
///
|
||||
/// These are the lookup providers, not the leads/deals/tickets list
|
||||
/// providers this screen used to read. Those hold one page of whatever the
|
||||
/// corresponding list screen last fetched, under whatever filter it last
|
||||
/// applied, so linking a task to a deal was only possible if the deals list
|
||||
/// happened to be showing it.
|
||||
Provider<AsyncValue<List<EntityLookup>>> _optionsOf(_RelatedKind kind) {
|
||||
switch (kind) {
|
||||
case _RelatedKind.account:
|
||||
return accountEntityOptionsProvider;
|
||||
case _RelatedKind.lead:
|
||||
return leadEntityOptionsProvider;
|
||||
case _RelatedKind.opportunity:
|
||||
return opportunityEntityOptionsProvider;
|
||||
case _RelatedKind.ticket:
|
||||
return ticketEntityOptionsProvider;
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve the label for the currently-linked entity. The TaskSerializer
|
||||
// returns these FKs as plain UUIDs rather than nested objects, so the edit
|
||||
// fetch carries no name. Returns null while the lookup is still in flight;
|
||||
// the UI shows a generic "Selected" until then.
|
||||
String? _resolveRelatedLabel() {
|
||||
final id = _relatedId;
|
||||
final kind = _relatedKind;
|
||||
if (id == null || kind == null) return null;
|
||||
switch (kind) {
|
||||
case _RelatedKind.account:
|
||||
for (final a in ref.watch(accountOptionsProvider)) {
|
||||
if (a.id == id) return a.name;
|
||||
}
|
||||
return null;
|
||||
case _RelatedKind.lead:
|
||||
for (final l in ref.watch(leadsListProvider)) {
|
||||
if (l.id == id) {
|
||||
final n = '${l.firstName} ${l.lastName}'.trim();
|
||||
return n.isEmpty ? l.email : n;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
case _RelatedKind.opportunity:
|
||||
for (final d in ref.watch(dealsListProvider)) {
|
||||
if (d.id == id) return d.title;
|
||||
}
|
||||
return null;
|
||||
case _RelatedKind.ticket:
|
||||
for (final t in ref.watch(ticketsListProvider)) {
|
||||
if (t.id == id) return t.name;
|
||||
}
|
||||
return null;
|
||||
for (final option in ref.watch(_optionsOf(kind)).value ?? const []) {
|
||||
if (option.id == id) return option.label;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/// The form as it looked when it finished loading, against the payload it
|
||||
|
|
@ -817,80 +811,21 @@ class _TaskFormScreenState extends ConsumerState<TaskFormScreen> {
|
|||
}
|
||||
|
||||
Future<void> _pickEntityForKind(_RelatedKind kind) async {
|
||||
switch (kind) {
|
||||
case _RelatedKind.account:
|
||||
final accounts = ref.read(accountOptionsProvider);
|
||||
final pick = await _pickFromList<AccountLookup>(
|
||||
title: 'Select Account',
|
||||
items: accounts,
|
||||
labelOf: (a) => a.name,
|
||||
icon: LucideIcons.building2,
|
||||
);
|
||||
if (pick != null) {
|
||||
setState(() {
|
||||
_relatedKind = kind;
|
||||
_relatedId = pick.id;
|
||||
});
|
||||
}
|
||||
break;
|
||||
case _RelatedKind.lead:
|
||||
final leads = ref.read(leadsListProvider);
|
||||
final pick = await _pickFromList<Lead>(
|
||||
title: 'Select Lead',
|
||||
items: leads,
|
||||
labelOf: (l) {
|
||||
final n = '${l.firstName} ${l.lastName}'.trim();
|
||||
return n.isEmpty ? l.email : n;
|
||||
},
|
||||
icon: LucideIcons.user,
|
||||
);
|
||||
if (pick != null) {
|
||||
setState(() {
|
||||
_relatedKind = kind;
|
||||
_relatedId = pick.id;
|
||||
});
|
||||
}
|
||||
break;
|
||||
case _RelatedKind.opportunity:
|
||||
final deals = ref.read(dealsListProvider);
|
||||
final pick = await _pickFromList<Deal>(
|
||||
title: 'Select Opportunity',
|
||||
items: deals,
|
||||
labelOf: (d) => d.title,
|
||||
icon: LucideIcons.trendingUp,
|
||||
);
|
||||
if (pick != null) {
|
||||
setState(() {
|
||||
_relatedKind = kind;
|
||||
_relatedId = pick.id;
|
||||
});
|
||||
}
|
||||
break;
|
||||
case _RelatedKind.ticket:
|
||||
final tickets = ref.read(ticketsListProvider);
|
||||
final pick = await _pickFromList<Ticket>(
|
||||
title: 'Select Ticket',
|
||||
items: tickets,
|
||||
labelOf: (t) => t.name,
|
||||
icon: LucideIcons.lifeBuoy,
|
||||
);
|
||||
if (pick != null) {
|
||||
setState(() {
|
||||
_relatedKind = kind;
|
||||
_relatedId = pick.id;
|
||||
});
|
||||
}
|
||||
break;
|
||||
}
|
||||
final pick = await _pickFromList(kind);
|
||||
if (pick == null) return;
|
||||
setState(() {
|
||||
_relatedKind = kind;
|
||||
_relatedId = pick.id;
|
||||
});
|
||||
}
|
||||
|
||||
Future<T?> _pickFromList<T>({
|
||||
required String title,
|
||||
required List<T> items,
|
||||
required String Function(T) labelOf,
|
||||
required IconData icon,
|
||||
}) {
|
||||
return showModalBottomSheet<T>(
|
||||
/// The sheet watches the lookup rather than being handed a list, so it can
|
||||
/// say "loading" and "that failed" instead of showing the empty state for
|
||||
/// all three. The empty state used to read "Open this section on the web or
|
||||
/// create one first", which on a cold app was simply untrue.
|
||||
Future<EntityLookup?> _pickFromList(_RelatedKind kind) {
|
||||
final provider = _optionsOf(kind);
|
||||
return showModalBottomSheet<EntityLookup>(
|
||||
context: context,
|
||||
backgroundColor: AppColors.surface,
|
||||
isScrollControlled: true,
|
||||
|
|
@ -915,50 +850,63 @@ class _TaskFormScreenState extends ConsumerState<TaskFormScreen> {
|
|||
),
|
||||
Padding(
|
||||
padding: const EdgeInsets.all(16),
|
||||
child: Text(title, style: AppTypography.h3),
|
||||
child: Text('Select ${kind.label}', style: AppTypography.h3),
|
||||
),
|
||||
Expanded(
|
||||
child: items.isEmpty
|
||||
? const Center(
|
||||
child: EmptyState(
|
||||
icon: LucideIcons.inbox,
|
||||
title: 'Nothing to pick',
|
||||
description:
|
||||
'Open this section on the web or create one first.',
|
||||
child: Consumer(
|
||||
builder: (_, ref, _) => ref
|
||||
.watch(provider)
|
||||
.when(
|
||||
loading: () =>
|
||||
const Center(child: CircularProgressIndicator()),
|
||||
error: (_, _) => Center(
|
||||
child: EmptyState(
|
||||
icon: LucideIcons.alertCircle,
|
||||
title: 'Could not load ${kind.label.toLowerCase()}s',
|
||||
description:
|
||||
'Close this and try again once you are back online.',
|
||||
),
|
||||
),
|
||||
)
|
||||
: ListView.builder(
|
||||
controller: controller,
|
||||
itemCount: items.length,
|
||||
itemBuilder: (_, i) {
|
||||
final item = items[i];
|
||||
return InkWell(
|
||||
onTap: () => Navigator.pop(ctx, item),
|
||||
child: Padding(
|
||||
padding: const EdgeInsets.symmetric(
|
||||
horizontal: 16,
|
||||
vertical: 14,
|
||||
),
|
||||
child: Row(
|
||||
children: [
|
||||
Icon(
|
||||
icon,
|
||||
size: 18,
|
||||
color: AppColors.textSecondary,
|
||||
),
|
||||
const SizedBox(width: 12),
|
||||
Expanded(
|
||||
child: Text(
|
||||
labelOf(item),
|
||||
style: AppTypography.body,
|
||||
data: (items) => items.isEmpty
|
||||
? Center(
|
||||
child: EmptyState(
|
||||
icon: LucideIcons.inbox,
|
||||
title: 'No ${kind.label.toLowerCase()}s yet',
|
||||
description:
|
||||
'Create one first, then link this task to it.',
|
||||
),
|
||||
)
|
||||
: ListView.builder(
|
||||
controller: controller,
|
||||
itemCount: items.length,
|
||||
itemBuilder: (_, i) => InkWell(
|
||||
onTap: () => Navigator.pop(ctx, items[i]),
|
||||
child: Padding(
|
||||
padding: const EdgeInsets.symmetric(
|
||||
horizontal: 16,
|
||||
vertical: 14,
|
||||
),
|
||||
child: Row(
|
||||
children: [
|
||||
Icon(
|
||||
kind.icon,
|
||||
size: 18,
|
||||
color: AppColors.textSecondary,
|
||||
),
|
||||
const SizedBox(width: 12),
|
||||
Expanded(
|
||||
child: Text(
|
||||
items[i].label,
|
||||
style: AppTypography.body,
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
},
|
||||
),
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
|
|
|
|||
|
|
@ -69,11 +69,12 @@ class AuthService {
|
|||
// Initialize Google Sign-In (required in v7.1.1).
|
||||
// serverClientId is the Web OAuth client (client_type: 3 in google-services.json)
|
||||
//. This is the audience the Django backend's GOOGLE_CLIENT_ID verifies against,
|
||||
// and it's what makes Android return a usable ID token.
|
||||
// and it's what makes Android return a usable ID token. It lives in
|
||||
// ApiConfig so a self-hoster can point a build at their own Google project
|
||||
// with --dart-define instead of editing this file.
|
||||
try {
|
||||
await _googleSignIn.initialize(
|
||||
serverClientId:
|
||||
'1072513761792-p59rct7b1c3go7l58e51r3geuqff2tfl.apps.googleusercontent.com',
|
||||
serverClientId: ApiConfig.googleServerClientId,
|
||||
);
|
||||
debugPrint('AuthService: Google Sign-In initialized successfully');
|
||||
} catch (e) {
|
||||
|
|
|
|||
105
mobile/test/screens/tasks/task_link_picker_test.dart
Normal file
105
mobile/test/screens/tasks/task_link_picker_test.dart
Normal file
|
|
@ -0,0 +1,105 @@
|
|||
import 'package:bottle_crm/core/theme/theme.dart';
|
||||
import 'package:bottle_crm/data/models/lookup_models.dart';
|
||||
import 'package:bottle_crm/providers/lookup_provider.dart';
|
||||
import 'package:bottle_crm/screens/tasks/task_form_screen.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:go_router/go_router.dart';
|
||||
|
||||
/// "Link to a lead" read the leads *list* provider, which holds one page of
|
||||
/// whatever the leads screen last fetched under whatever filter it last
|
||||
/// applied. Open a fresh app, create a task, tap Lead: nothing, under the
|
||||
/// heading "Nothing to pick" and the advice "Open this section on the web or
|
||||
/// create one first". The org had twenty leads.
|
||||
///
|
||||
/// The picker now reads a lookup of its own and, because it watches rather than
|
||||
/// reads, can tell the three states apart.
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
Future<void> openLeadPicker(
|
||||
WidgetTester tester,
|
||||
AsyncValue<List<EntityLookup>> leads,
|
||||
) async {
|
||||
final router = GoRouter(
|
||||
initialLocation: '/new',
|
||||
routes: [
|
||||
GoRoute(path: '/new', builder: (_, _) => const TaskFormScreen()),
|
||||
],
|
||||
);
|
||||
await tester.pumpWidget(
|
||||
ProviderScope(
|
||||
overrides: [leadEntityOptionsProvider.overrideWithValue(leads)],
|
||||
child: MaterialApp.router(theme: AppTheme.light, routerConfig: router),
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
// The Linked Record row sits below the fold on a test-sized screen.
|
||||
await tester.ensureVisible(find.text('Not linked'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text('Not linked'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text('Lead'));
|
||||
await tester.pump();
|
||||
await tester.pump(const Duration(milliseconds: 400));
|
||||
}
|
||||
|
||||
testWidgets('a lookup still in flight shows loading, not an empty org', (
|
||||
tester,
|
||||
) async {
|
||||
await openLeadPicker(tester, const AsyncValue.loading());
|
||||
|
||||
expect(find.byType(CircularProgressIndicator), findsOneWidget);
|
||||
expect(find.textContaining('No Leads yet'), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('leads that loaded are offered by name', (tester) async {
|
||||
await openLeadPicker(
|
||||
tester,
|
||||
const AsyncValue.data([
|
||||
EntityLookup(id: 'l1', label: 'Jill Shaffer'),
|
||||
EntityLookup(id: 'l2', label: 'Cruz Group'),
|
||||
]),
|
||||
);
|
||||
|
||||
expect(find.text('Jill Shaffer'), findsOneWidget);
|
||||
expect(find.text('Cruz Group'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('picking one puts it on the form', (tester) async {
|
||||
await openLeadPicker(
|
||||
tester,
|
||||
const AsyncValue.data([EntityLookup(id: 'l1', label: 'Jill Shaffer')]),
|
||||
);
|
||||
|
||||
await tester.tap(find.text('Jill Shaffer'));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text('Lead: Jill Shaffer'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('an org with no leads is told that, and nothing else', (
|
||||
tester,
|
||||
) async {
|
||||
await openLeadPicker(tester, const AsyncValue.data([]));
|
||||
|
||||
expect(find.text('No leads yet'), findsOneWidget);
|
||||
// The old copy sent people to the web for a list that was simply not
|
||||
// fetched yet. An org with no leads has nothing to open there either.
|
||||
expect(find.textContaining('on the web'), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('a failed lookup says so instead of claiming the org is empty', (
|
||||
tester,
|
||||
) async {
|
||||
await openLeadPicker(
|
||||
tester,
|
||||
AsyncValue.error(Exception('offline'), StackTrace.empty),
|
||||
);
|
||||
|
||||
expect(find.text('Could not load leads'), findsOneWidget);
|
||||
expect(find.text('No leads yet'), findsNothing);
|
||||
});
|
||||
}
|
||||
102
mobile/test/unit/api_config_test.dart
Normal file
102
mobile/test/unit/api_config_test.dart
Normal file
|
|
@ -0,0 +1,102 @@
|
|||
import 'package:bottle_crm/config/api_config.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
/// The API host was two literals in Dart, so anyone self-hosting had to edit
|
||||
/// source and rebuild. It is now a `--dart-define`, and the branches that
|
||||
/// define adds are what these tests cover.
|
||||
///
|
||||
/// `flutter test` compiles once, without any `--dart-define`, so the override
|
||||
/// path is unreachable through `ApiConfig.baseUrl` itself. `resolveBaseUrl`
|
||||
/// takes the two inputs as arguments for exactly that reason.
|
||||
void main() {
|
||||
group('with no override, which is every build today', () {
|
||||
test('debug and release resolve to different hosts', () {
|
||||
final debug = ApiConfig.resolveBaseUrl(override: '', isDebug: true);
|
||||
final release = ApiConfig.resolveBaseUrl(override: '', isDebug: false);
|
||||
|
||||
expect(debug, isNot(release));
|
||||
expect(release, 'https://api.bottlecrm.io');
|
||||
});
|
||||
|
||||
test('the shipped default is what the app actually uses', () {
|
||||
// Pins the wiring, not just the helper: a `baseUrl` that stopped calling
|
||||
// `resolveBaseUrl` would leave every test above green and every request
|
||||
// pointed somewhere else.
|
||||
expect(
|
||||
ApiConfig.baseUrl,
|
||||
ApiConfig.resolveBaseUrl(override: '', isDebug: true),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('with an override', () {
|
||||
test('it wins over both built-in hosts', () {
|
||||
expect(
|
||||
ApiConfig.resolveBaseUrl(
|
||||
override: 'https://crm.example.com',
|
||||
isDebug: false,
|
||||
),
|
||||
'https://crm.example.com',
|
||||
);
|
||||
});
|
||||
|
||||
test('a trailing slash is dropped rather than doubled into the path', () {
|
||||
// `https://host//api` 404s on every call and says nothing about why.
|
||||
final resolved = ApiConfig.resolveBaseUrl(
|
||||
override: 'https://crm.example.com/',
|
||||
isDebug: false,
|
||||
);
|
||||
|
||||
expect(resolved, 'https://crm.example.com');
|
||||
expect('$resolved/api', 'https://crm.example.com/api');
|
||||
});
|
||||
|
||||
test('plain HTTP is refused in a release build', () {
|
||||
// The whole point of the guard: before this knob existed nobody could
|
||||
// ship a plaintext build without editing source. Now it is one flag away,
|
||||
// and every JWT would travel in the clear.
|
||||
expect(
|
||||
() => ApiConfig.resolveBaseUrl(
|
||||
override: 'http://crm.example.com',
|
||||
isDebug: false,
|
||||
),
|
||||
throwsA(isA<StateError>()),
|
||||
);
|
||||
});
|
||||
|
||||
test('the refusal names the address, so the mistake is findable', () {
|
||||
expect(
|
||||
() => ApiConfig.resolveBaseUrl(
|
||||
override: 'http://crm.example.com',
|
||||
isDebug: false,
|
||||
),
|
||||
throwsA(
|
||||
isA<StateError>().having(
|
||||
(e) => e.message,
|
||||
'message',
|
||||
contains('http://crm.example.com'),
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('plain HTTP is allowed in debug, where localhost lives', () {
|
||||
expect(
|
||||
ApiConfig.resolveBaseUrl(
|
||||
override: 'http://10.0.2.2:8000',
|
||||
isDebug: true,
|
||||
),
|
||||
'http://10.0.2.2:8000',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('the Google client id', () {
|
||||
test('has a default, so an unconfigured build still signs in', () {
|
||||
expect(
|
||||
ApiConfig.googleServerClientId,
|
||||
endsWith('.apps.googleusercontent.com'),
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
168
mobile/test/unit/entity_lookup_test.dart
Normal file
168
mobile/test/unit/entity_lookup_test.dart
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
import 'package:bottle_crm/config/api_config.dart';
|
||||
import 'package:bottle_crm/data/api_envelope.dart';
|
||||
import 'package:bottle_crm/data/models/lookup_models.dart';
|
||||
import 'package:bottle_crm/providers/lookup_provider.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
/// Every picker in this app was capped at ten rows, and none of them said so.
|
||||
///
|
||||
/// The endpoints use DRF's `LimitOffsetPagination` with `PAGE_SIZE = 10`. A
|
||||
/// lookup that sent no `limit` got ten. Confirmed live against the seeded org:
|
||||
/// `/api/contacts/` answers `contacts_count: 15` beside ten rows, so five
|
||||
/// contacts could not be attached to anything from the phone. There was no
|
||||
/// error, no truncation notice, and no way to tell from the screen.
|
||||
void main() {
|
||||
group('the lookup request', () {
|
||||
test('asks for more than one page', () {
|
||||
final url = lookupUrl(ApiConfig.contacts);
|
||||
|
||||
expect(url, contains('limit=$lookupPageLimit'));
|
||||
expect(lookupPageLimit, greaterThan(10));
|
||||
});
|
||||
|
||||
test('keeps the path it was given', () {
|
||||
expect(lookupUrl(ApiConfig.tags), startsWith(ApiConfig.tags));
|
||||
});
|
||||
});
|
||||
|
||||
group('the envelope each new lookup reads', () {
|
||||
test('leads are two levels down, under the open group', () {
|
||||
// Same shape trap as accounts. `results` is absent, and a missed path
|
||||
// reads as "this org has no leads" rather than as a failure.
|
||||
final body = <String, dynamic>{
|
||||
'open_leads': {
|
||||
'open_leads': [
|
||||
{'id': 'l1', 'first_name': 'Jill', 'last_name': 'Shaffer'},
|
||||
],
|
||||
'leads_count': 20,
|
||||
},
|
||||
'close_leads': {
|
||||
'close_leads': [
|
||||
{'id': 'l9', 'first_name': 'Gone', 'last_name': 'Away'},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
expect(listFromEnvelope(body, const ['results']), isEmpty);
|
||||
expect(
|
||||
listFromEnvelope(body, const ['open_leads.open_leads']),
|
||||
hasLength(1),
|
||||
);
|
||||
});
|
||||
|
||||
test('deals and tickets sit at the top level under their own key', () {
|
||||
expect(
|
||||
listFromEnvelope(
|
||||
{
|
||||
'opportunities': [
|
||||
{'id': 'o1', 'name': 'Renewal'},
|
||||
],
|
||||
},
|
||||
const ['opportunities'],
|
||||
),
|
||||
hasLength(1),
|
||||
);
|
||||
expect(
|
||||
listFromEnvelope(
|
||||
{
|
||||
'cases': [
|
||||
{'id': 'c1', 'name': 'Login fails'},
|
||||
],
|
||||
},
|
||||
const ['cases'],
|
||||
),
|
||||
hasLength(1),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('a labelled record', () {
|
||||
test('a deal is named by its name field, not its title field', () {
|
||||
// `title` exists on the row and is null on every deal the API returned.
|
||||
// Reading it would label every option "Untitled".
|
||||
final deal = EntityLookup.fromJson(
|
||||
{'id': 'o1', 'name': 'Stand-alone throughput Deal', 'title': null},
|
||||
labelKeys: const ['name'],
|
||||
);
|
||||
|
||||
expect(deal.label, 'Stand-alone throughput Deal');
|
||||
});
|
||||
|
||||
test('a row with nothing to show still carries its id', () {
|
||||
final deal = EntityLookup.fromJson(
|
||||
{'id': 'o2'},
|
||||
labelKeys: const ['name'],
|
||||
);
|
||||
|
||||
// Selectable, and visibly odd. An empty label would render a blank row
|
||||
// that looks like a rendering bug and cannot be tapped with confidence.
|
||||
expect(deal.id, 'o2');
|
||||
expect(deal.label, 'Untitled');
|
||||
});
|
||||
});
|
||||
|
||||
group('a person', () {
|
||||
test('is named first and last', () {
|
||||
final lead = EntityLookup.person({
|
||||
'id': 'l1',
|
||||
'first_name': 'Jill',
|
||||
'last_name': 'Shaffer',
|
||||
'email': 'jill@example.test',
|
||||
});
|
||||
|
||||
expect(lead.label, 'Jill Shaffer');
|
||||
});
|
||||
|
||||
test('with only one name part does not carry a stray space', () {
|
||||
final lead = EntityLookup.person({
|
||||
'id': 'l1',
|
||||
'first_name': 'Jill',
|
||||
'last_name': '',
|
||||
});
|
||||
|
||||
expect(lead.label, 'Jill');
|
||||
});
|
||||
|
||||
test('with no name falls back in order, email before company', () {
|
||||
final lead = EntityLookup.person(
|
||||
{
|
||||
'id': 'l1',
|
||||
'first_name': '',
|
||||
'last_name': '',
|
||||
'email': 'agarcia@example.test',
|
||||
'company_name': 'Cruz Group',
|
||||
},
|
||||
fallbackKeys: const ['email', 'company_name'],
|
||||
);
|
||||
|
||||
expect(lead.label, 'agarcia@example.test');
|
||||
});
|
||||
|
||||
test('with no name and no email falls through to the company', () {
|
||||
final lead = EntityLookup.person(
|
||||
{'id': 'l1', 'company_name': 'Cruz Group'},
|
||||
fallbackKeys: const ['email', 'company_name'],
|
||||
);
|
||||
|
||||
expect(lead.label, 'Cruz Group');
|
||||
});
|
||||
|
||||
test('with nothing at all is still tappable', () {
|
||||
final lead = EntityLookup.person({'id': 'l1'});
|
||||
|
||||
expect(lead.id, 'l1');
|
||||
expect(lead.label, 'Unnamed');
|
||||
});
|
||||
});
|
||||
|
||||
group('identity', () {
|
||||
test('two rows with the same id are the same option', () {
|
||||
// The picker compares options to mark the current selection. Comparing
|
||||
// by label would tie two people who share a name.
|
||||
expect(
|
||||
const EntityLookup(id: 'x', label: 'One'),
|
||||
const EntityLookup(id: 'x', label: 'Another'),
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
Loading…
Reference in a new issue