diff --git a/backend/opportunity/serializer.py b/backend/opportunity/serializer.py index 7b14211f..f8660a05 100644 --- a/backend/opportunity/serializer.py +++ b/backend/opportunity/serializer.py @@ -4,6 +4,7 @@ from django.db.models import Sum from drf_spectacular.utils import extend_schema_field from rest_framework import serializers +from accounts.access import has_account_access from accounts.serializer import AccountPickerSerializer from common.serializer import ( OrganizationSerializer, @@ -309,11 +310,16 @@ class OpportunityCreateSerializer(serializers.ModelSerializer): request_obj = kwargs.pop("request_obj", None) super().__init__(*args, **kwargs) self.org = None + self.profile = None # Set by `validate`: whether this save puts the deal in a won or lost # stage it named, which is when the view records who closed it. self.closing = False + # An id that matches no account at all reads the same as one the + # caller may not open; see `validate_account`. + self.fields["account"].error_messages["does_not_exist"] = "No such account." if request_obj: self.org = request_obj.profile.org + self.profile = request_obj.profile # Only this org's pipelines resolve; another org's id is "does not # exist", the same answer as an id nobody has. self.fields["pipeline"].queryset = DealPipeline.objects.filter(org=self.org) @@ -335,6 +341,23 @@ class OpportunityCreateSerializer(serializers.ModelSerializer): ) return name + def validate_account(self, account): + """Only an account in this org that the caller may open, as on contacts + and tickets, and every refusal reads like an unknown id. The account the + deal already has is kept whoever saves: the phone form sends it back on + every edit, and the caller never chose it.""" + if account is None: + return account + if self.instance is not None and account.id == self.instance.account_id: + return account + if ( + self.org is None + or account.org_id != self.org.id + or not has_account_access(self.profile, account) + ): + raise serializers.ValidationError("No such account.") + return account + def _resolved(self, data, field): """The value this save will end up with, the submitted one if the request carried the field, otherwise what is already on the record. diff --git a/backend/opportunity/tests/test_deal_account_visibility.py b/backend/opportunity/tests/test_deal_account_visibility.py new file mode 100644 index 00000000..f42a5d17 --- /dev/null +++ b/backend/opportunity/tests/test_deal_account_visibility.py @@ -0,0 +1,222 @@ +"""A deal links only to an account its editor can open, as contacts and tickets do. + +`OpportunityCreateSerializer.validate_account` asks `has_account_access` on +create, PUT and PATCH. An unknown id, another org's account and a hidden one +read the same, and the account a deal already has is kept whoever saves, since +the phone form sends it back on every edit. +""" + +import uuid + +import pytest + +from accounts.models import Account +from conftest import rls_org +from opportunity.models import Opportunity + +DEALS_URL = "/api/opportunities/" + +pytestmark = pytest.mark.django_db + + +def _account(org, name, created_by=None): + account = Account.objects.create(name=name, org=org) + Account.objects.filter(pk=account.pk).update(created_by=created_by) + return account + + +def _deal(org, name, created_by, account=None): + deal = Opportunity.objects.create(name=name, org=org, account=account) + Opportunity.objects.filter(pk=deal.pk).update(created_by=created_by) + return deal + + +@pytest.fixture +def hidden(org_a, admin_user): + return _account(org_a, "Hidden Holdings", created_by=admin_user) + + +@pytest.fixture +def assigned(org_a, admin_user, user_profile): + account = _account(org_a, "Assigned Industries", created_by=admin_user) + account.assigned_to.add(user_profile) + return account + + +@pytest.fixture +def foreign(org_b, user_b): + with rls_org(org_b): + return _account(org_b, "Other Org Ltd", created_by=user_b) + + +@pytest.fixture +def superuser(regular_user): + """The `user_client` caller, with a plain USER profile, made superuser.""" + regular_user.is_superuser = True + regular_user.save(update_fields=["is_superuser"]) + return regular_user + + +@pytest.fixture +def mine(org_a, regular_user): + return _deal(org_a, "Mine", regular_user) + + +@pytest.fixture +def mine_at_hidden(org_a, regular_user, hidden): + """The member's own deal, linked by an admin to an account they cannot open.""" + return _deal(org_a, "Linked", regular_user, account=hidden) + + +def _create(client, name, account_id): + return client.post( + DEALS_URL, {"name": name, "account": str(account_id)}, format="json" + ) + + +def _refusal_errors(response): + assert response.status_code == 400, response.content + return response.json()["errors"] + + +def _refusal(response): + return _refusal_errors(response)["account"] + + +class TestDealAccountLinkOnCreate: + def test_member_may_link_an_account_they_are_assigned_to( + self, user_client, assigned + ): + response = _create(user_client, "Assigned", assigned.id) + assert response.status_code == 200, response.content + assert Opportunity.objects.get(name="Assigned").account_id == assigned.id + + def test_member_may_link_an_account_they_created( + self, user_client, org_a, regular_user + ): + own = _account(org_a, "Own Co", created_by=regular_user) + response = _create(user_client, "Own", own.id) + assert response.status_code == 200, response.content + assert Opportunity.objects.get(name="Own").account_id == own.id + + def test_hidden_account_reads_like_an_unknown_id(self, user_client, hidden): + hidden_refusal = _refusal(_create(user_client, "Hidden", hidden.id)) + unknown_refusal = _refusal(_create(user_client, "Unknown", uuid.uuid4())) + assert hidden_refusal == unknown_refusal == ["No such account."] + assert not Opportunity.objects.filter(name__in=["Hidden", "Unknown"]).exists() + + def test_malformed_account_id_is_a_400(self, user_client): + response = user_client.post( + DEALS_URL, {"name": "Garbled", "account": "not-a-uuid"}, format="json" + ) + assert "account" in _refusal_errors(response) + assert not Opportunity.objects.filter(name="Garbled").exists() + + def test_admin_may_link_any_account_in_the_org(self, admin_client, hidden): + response = _create(admin_client, "Admin", hidden.id) + assert response.status_code == 200, response.content + assert Opportunity.objects.get(name="Admin").account_id == hidden.id + + def test_superuser_may_link_any_account_in_the_org( + self, superuser, user_client, hidden + ): + response = _create(user_client, "Superuser", hidden.id) + assert response.status_code == 200, response.content + assert Opportunity.objects.get(name="Superuser").account_id == hidden.id + + @pytest.mark.parametrize("caller", ["member", "admin", "superuser"]) + def test_other_org_account_is_refused_for_everyone( + self, caller, request, user_client, admin_client, foreign + ): + if caller == "superuser": + request.getfixturevalue("superuser") + client = admin_client if caller == "admin" else user_client + assert _refusal(_create(client, "Foreign", foreign.id)) == ["No such account."] + assert not Opportunity.objects.filter(name="Foreign").exists() + + +class TestDealAccountLinkOnEdit: + def test_patch_may_not_move_a_deal_to_a_hidden_account( + self, user_client, mine, hidden + ): + response = user_client.patch( + f"{DEALS_URL}{mine.id}/", {"account": str(hidden.id)}, format="json" + ) + assert _refusal(response) == ["No such account."] + mine.refresh_from_db() + assert mine.account_id is None + + def test_put_may_not_move_a_deal_to_a_hidden_account( + self, user_client, mine, hidden + ): + response = user_client.put( + f"{DEALS_URL}{mine.id}/", + {"name": "Mine", "account": str(hidden.id)}, + format="json", + ) + assert _refusal(response) == ["No such account."] + mine.refresh_from_db() + assert mine.account_id is None + + def test_patch_may_move_a_deal_to_an_account_the_member_can_open( + self, user_client, mine, assigned + ): + response = user_client.patch( + f"{DEALS_URL}{mine.id}/", {"account": str(assigned.id)}, format="json" + ) + assert response.status_code == 200, response.content + mine.refresh_from_db() + assert mine.account_id == assigned.id + + def test_patch_may_not_move_a_deal_to_another_org_account_even_as_admin( + self, admin_client, mine, foreign + ): + response = admin_client.patch( + f"{DEALS_URL}{mine.id}/", {"account": str(foreign.id)}, format="json" + ) + assert _refusal(response) == ["No such account."] + mine.refresh_from_db() + assert mine.account_id is None + + def test_patch_resending_a_stored_hidden_account_keeps_it( + self, user_client, hidden, mine_at_hidden + ): + # What the phone form does: it sends `account` on every save. + response = user_client.patch( + f"{DEALS_URL}{mine_at_hidden.id}/", + {"description": "Renewal", "account": str(hidden.id)}, + format="json", + ) + assert response.status_code == 200, response.content + mine_at_hidden.refresh_from_db() + assert mine_at_hidden.description == "Renewal" + assert mine_at_hidden.account_id == hidden.id + + def test_put_resending_a_stored_hidden_account_keeps_it( + self, user_client, hidden, mine_at_hidden + ): + response = user_client.put( + f"{DEALS_URL}{mine_at_hidden.id}/", + {"name": "Linked", "description": "Renewal", "account": str(hidden.id)}, + format="json", + ) + assert response.status_code == 200, response.content + mine_at_hidden.refresh_from_db() + assert mine_at_hidden.account_id == hidden.id + + def test_patch_may_clear_the_account(self, user_client, mine_at_hidden): + response = user_client.patch( + f"{DEALS_URL}{mine_at_hidden.id}/", {"account": None}, format="json" + ) + assert response.status_code == 200, response.content + mine_at_hidden.refresh_from_db() + assert mine_at_hidden.account_id is None + + def test_picker_offers_exactly_what_the_save_path_accepts( + self, user_client, assigned, hidden + ): + # The web deal form builds its account picker from this list. + body = user_client.get(f"{DEALS_URL}?limit=1").json() + offered = {row["id"] for row in body["accounts_list"]} + assert str(assigned.id) in offered + assert str(hidden.id) not in offered diff --git a/backend/pyproject.toml b/backend/pyproject.toml index a39e2a1c..829897c0 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "django-crm" -version = "1.22.0" +version = "1.22.1" # The one-line summary PyPI shows under the project title and in search results, # so it carries the terms people actually search for ("open source CRM", # "self-hosted CRM", "Django CRM") and names the modules rather than the diff --git a/backend/uv.lock b/backend/uv.lock index 8b845e1d..1a5937f2 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -704,7 +704,7 @@ wheels = [ [[package]] name = "django-crm" -version = "1.22.0" +version = "1.22.1" source = { editable = "." } dependencies = [ { name = "cairocffi" }, diff --git a/docs/api/opportunities.md b/docs/api/opportunities.md index f82c3da1..462c5b22 100644 --- a/docs/api/opportunities.md +++ b/docs/api/opportunities.md @@ -201,7 +201,7 @@ accept. Only `name` is required at the model level (`opportunity/models.py:46`, | Field | Type | Required | Notes | | --- | --- | --- | --- | | `name` | string | **required** | Unique per org, case-insensitive | -| `account` | uuid | optional | | +| `account` | uuid | optional | An account in the caller's org that the caller can open (an admin can open any). Anything else, an unknown id included, is a 400 `"No such account."`. On edit, the account the deal already has is kept whoever saves | | `pipeline` | uuid | optional | One of the org's pipelines. Defaults to the deal's current pipeline, or the org's default on create. Changing it requires `stage` too | | `stage` | a stage code of the deal's pipeline | optional | Defaults to the pipeline's first open stage on create; see [Stages](#stages) | | `opportunity_type` | one of `OPPORTUNITY_TYPES` | optional | `NEW_BUSINESS`, `EXISTING_BUSINESS`, `RENEWAL`, `UPSELL`, `CROSS_SELL` | diff --git a/frontend/src/lib/server/v2/automation.js b/frontend/src/lib/server/v2/automation.js index aa1ef0b6..116cbd02 100644 --- a/frontend/src/lib/server/v2/automation.js +++ b/frontend/src/lib/server/v2/automation.js @@ -8,6 +8,9 @@ import { readableError } from '#lib/server/v2/form-errors.js'; import { checkId } from '#lib/server/v2/custom-objects.js'; export const RULE_GONE = 'That rule does not exist, or you do not have access to it.'; +// The hosted web app can run ahead of its API during a release, so an API +// without these routes reads as "not offered", as custom objects do. +export const NOT_AVAILABLE = 'Automation rules are not available here.'; const UNREADABLE = 'Could not read the rule.'; /** Runs per page on a rule's history. */ @@ -17,16 +20,30 @@ export const RUN_PAGE = 20; /** @param {Event} event */ export async function getCatalogue({ cookies }) { - return await apiRequest('/automation/catalogue/', { expectedStatuses: [403] }, { cookies }); + try { + return await apiRequest( + '/automation/catalogue/', + { expectedStatuses: [403, 404] }, + { cookies } + ); + } catch (/** @type {any} */ err) { + if (err?.status === 404) error(404, NOT_AVAILABLE); + throw err; + } } /** @param {Event} event */ export async function listRules({ cookies }) { try { - const resp = await apiRequest('/automation/rules/', { expectedStatuses: [403] }, { cookies }); + const resp = await apiRequest( + '/automation/rules/', + { expectedStatuses: [403, 404] }, + { cookies } + ); return { summary: resp?.summary ?? null, rules: resp?.results ?? [] }; } catch (/** @type {any} */ err) { if (err?.status === 403) return { forbidden: true }; + if (err?.status === 404) error(404, NOT_AVAILABLE); throw err; } } @@ -130,9 +147,10 @@ export async function reorderRules({ cookies }, ids) { } /** - * The Settings tile's numbers. Any failure is null: the tile just does not show. + * The Settings row's numbers. Null when they cannot be read, so the row shows + * without them; false when the API has no automation, so the row is hidden. * @param {Event} event - * @returns {Promise<{ active: number, failing: number, attention: boolean } | null>} + * @returns {Promise<{ active: number, failing: number, attention: boolean } | null | false>} */ export async function automationSummary({ cookies }) { try { @@ -148,8 +166,8 @@ export async function automationSummary({ cookies }) { if (bad || r.status === 'paused') attention = true; } return { active, failing, attention }; - } catch { - return null; + } catch (/** @type {any} */ err) { + return err?.status === 404 ? false : null; } } diff --git a/frontend/src/lib/server/v2/automation.test.js b/frontend/src/lib/server/v2/automation.test.js index 992e97fd..cdcf7775 100644 --- a/frontend/src/lib/server/v2/automation.test.js +++ b/frontend/src/lib/server/v2/automation.test.js @@ -15,6 +15,7 @@ const { listRuns, reorderRules, automationSummary, + NOT_AVAILABLE, ruleBody, moveRule, ruleErrors @@ -94,12 +95,22 @@ describe('requests', () => { expect(apiRequest.mock.calls[0][1].expectedStatuses).toEqual([403, 404]); }); - it('expects a 403 on the list and the catalogue so a member logs no error', async () => { + it('expects a 403 and a 404 on the list and the catalogue, so a member or an API without automation logs no error', async () => { apiRequest.mockResolvedValue({ results: [] }); await listRules(event); await getCatalogue(event); - expect(apiRequest.mock.calls[0][1].expectedStatuses).toEqual([403]); - expect(apiRequest.mock.calls[1][1].expectedStatuses).toEqual([403]); + expect(apiRequest.mock.calls[0][1].expectedStatuses).toEqual([403, 404]); + expect(apiRequest.mock.calls[1][1].expectedStatuses).toEqual([403, 404]); + }); + + it('turns a 404 on the list or the catalogue into a page 404 that says rules are not offered', async () => { + expect(NOT_AVAILABLE).toBe('Automation rules are not available here.'); + for (const call of [() => listRules(event), () => getCatalogue(event)]) { + apiRequest.mockRejectedValueOnce(status(404)); + await expect(call()).rejects.toMatchObject({ status: 404, body: { message: NOT_AVAILABLE } }); + } + apiRequest.mockRejectedValueOnce(status(403)); + await expect(getCatalogue(event)).rejects.toMatchObject({ status: 403 }); }); it('refuses a malformed id before any request', async () => { @@ -124,13 +135,18 @@ describe('requests', () => { }); describe('automationSummary', () => { - it('answers null for the summary on 403, 404, 500 and a network error', async () => { - for (const err of [status(403), status(404), status(500), new TypeError('fetch failed')]) { + it('answers null for the summary on 403, 500 and a network error', async () => { + for (const err of [status(403), status(500), new TypeError('fetch failed')]) { apiRequest.mockRejectedValueOnce(err); expect(await automationSummary(event)).toBeNull(); } }); + it('answers false on a 404: this API has no automation', async () => { + apiRequest.mockRejectedValueOnce(status(404)); + expect(await automationSummary(event)).toBe(false); + }); + it('counts active, failing and attention', async () => { apiRequest.mockResolvedValue({ results: [ diff --git a/frontend/src/lib/v2/automation.js b/frontend/src/lib/v2/automation.js index a2cbb8ab..f1929639 100644 --- a/frontend/src/lib/v2/automation.js +++ b/frontend/src/lib/v2/automation.js @@ -702,9 +702,21 @@ export function switchLabel(status) { return null; } +/** + * Whether the Settings hub offers the Automation row: admins only, and not + * where the API answered 404 (`automationSummary` returns false there). + * + * @param {{ active: number, failing: number, attention: boolean } | null | false} summary + * @param {boolean} isAdmin + * @returns {boolean} + */ +export function showAutomationSetting(summary, isAdmin) { + return isAdmin && summary !== false; +} + /** * The settings hub row's value and warning from `automationSummary`. - * @param {{ active: number, failing: number, attention: boolean } | null | undefined} summary + * @param {{ active: number, failing: number, attention: boolean } | null | false | undefined} summary * @returns {{ value: string | null, warn: boolean }} */ export function hubValue(summary) { diff --git a/frontend/src/lib/v2/automation.test.js b/frontend/src/lib/v2/automation.test.js index c09de21d..addcf825 100644 --- a/frontend/src/lib/v2/automation.test.js +++ b/frontend/src/lib/v2/automation.test.js @@ -3,6 +3,7 @@ import { actorName, notificationText, hubValue, + showAutomationSetting, objectOf, fieldOf, triggerFields, @@ -530,6 +531,20 @@ describe('moveItem', () => { }); }); +describe('showAutomationSetting', () => { + it('shows admins the row unless the API has no automation', () => { + const summary = { active: 1, failing: 0, attention: false }; + expect(showAutomationSetting(summary, true)).toBe(true); + expect(showAutomationSetting(null, true)).toBe(true); + expect(showAutomationSetting(false, true)).toBe(false); + }); + + it('never shows a member the row', () => { + expect(showAutomationSetting({ active: 1, failing: 0, attention: false }, false)).toBe(false); + expect(showAutomationSetting(null, false)).toBe(false); + }); +}); + describe('hubValue', () => { it('answers no value and no warning without a summary', () => { expect(hubValue(null)).toEqual({ value: null, warn: false }); diff --git a/frontend/src/routes/(app)/settings/+page.svelte b/frontend/src/routes/(app)/settings/+page.svelte index ee297752..787f3497 100644 --- a/frontend/src/routes/(app)/settings/+page.svelte +++ b/frontend/src/routes/(app)/settings/+page.svelte @@ -27,7 +27,7 @@ import PageHeader from '#lib/v2/components/PageHeader.svelte'; import { count, shortDate } from '#lib/v2/format.js'; import { isOrgAdmin } from '#lib/admin.js'; - import { hubValue } from '#lib/v2/automation.js'; + import { hubValue, showAutomationSetting } from '#lib/v2/automation.js'; import { showObjectsSetting } from '#lib/v2/custom-objects.js'; import { ChevronRight, ShieldAlert } from '@lucide/svelte'; @@ -256,7 +256,7 @@ } ] : []), - ...(isOrgAdmin(data) + ...(showAutomationSetting(data.automation, isOrgAdmin(data)) ? [ { href: '/settings/automation', diff --git a/mobile/lib/providers/automation_provider.dart b/mobile/lib/providers/automation_provider.dart index 4d60334e..c565e96e 100644 --- a/mobile/lib/providers/automation_provider.dart +++ b/mobile/lib/providers/automation_provider.dart @@ -17,6 +17,16 @@ final automationCatalogueProvider = return AutomationCatalogue.fromJson(data); }, retry: (_, _) => null); +/// The rules list answered 404: this API has no automation, which the hosted +/// app can meet while its API is a release behind. Read as "not offered", as +/// custom objects read a 404. +class AutomationNotOffered implements Exception { + const AutomationNotOffered(); + + @override + String toString() => 'Automation rules are not available here.'; +} + class AutomationRulesNotifier extends AsyncNotifier { @override Future build() { @@ -26,6 +36,7 @@ class AutomationRulesNotifier extends AsyncNotifier { Future _fetch() async { final response = await ApiService().get(ApiConfig.automationRules); + if (response.statusCode == 404) throw const AutomationNotOffered(); final data = response.data; if (!response.success || data == null) { throw Exception(response.message ?? 'Could not load the rules'); @@ -81,6 +92,10 @@ AutomationRulesAnswer? shownAutomationRules( AsyncValue v, ) => v.isLoading || v.hasError ? null : v.value; +/// True only when the last load found no automation on this API. +bool automationNotOffered(AsyncValue v) => + v.error is AutomationNotOffered; + final automationRulesProvider = AsyncNotifierProvider( AutomationRulesNotifier.new, diff --git a/mobile/lib/screens/settings/automation/automation_rules_screen.dart b/mobile/lib/screens/settings/automation/automation_rules_screen.dart index c605d7b1..58187e53 100644 --- a/mobile/lib/screens/settings/automation/automation_rules_screen.dart +++ b/mobile/lib/screens/settings/automation/automation_rules_screen.dart @@ -77,7 +77,13 @@ class _Body extends ConsumerWidget { final catalogue = loaded.isLoading || loaded.hasError ? null : loaded.value; if (answer == null) { if (async.hasError && !async.isLoading) { + if (automationNotOffered(async)) { + return const _ErrorState( + message: 'Automation rules are not available here.', + ); + } return _ErrorState( + message: 'Could not load automation rules', onRetry: () => ref.read(automationRulesProvider.notifier).refresh(), ); } @@ -317,9 +323,10 @@ class _MemberNotice extends StatelessWidget { } class _ErrorState extends StatelessWidget { - const _ErrorState({required this.onRetry}); + const _ErrorState({required this.message, this.onRetry}); - final VoidCallback onRetry; + final String message; + final VoidCallback? onRetry; @override Widget build(BuildContext context) => Center( @@ -334,13 +341,11 @@ class _ErrorState extends StatelessWidget { color: AppColors.textTertiary, ), const SizedBox(height: 16), - Text( - 'Could not load automation rules', - style: AppTypography.body, - textAlign: TextAlign.center, - ), - const SizedBox(height: 16), - OutlinedButton(onPressed: onRetry, child: const Text('Try again')), + Text(message, style: AppTypography.body, textAlign: TextAlign.center), + if (onRetry != null) ...[ + const SizedBox(height: 16), + OutlinedButton(onPressed: onRetry, child: const Text('Try again')), + ], ], ), ), diff --git a/mobile/lib/screens/settings/settings_hub_screen.dart b/mobile/lib/screens/settings/settings_hub_screen.dart index de049738..6914bb7e 100644 --- a/mobile/lib/screens/settings/settings_hub_screen.dart +++ b/mobile/lib/screens/settings/settings_hub_screen.dart @@ -5,6 +5,7 @@ import 'package:lucide_icons_flutter/lucide_icons.dart'; import '../../core/theme/theme.dart'; import '../../providers/auth_provider.dart'; +import '../../providers/automation_provider.dart'; import '../../providers/custom_objects_provider.dart'; import '../../routes/app_router.dart'; import '../../widgets/common/common.dart'; @@ -158,15 +159,20 @@ class SettingsHubScreen extends StatelessWidget { : const SizedBox.shrink(), ), Consumer( - builder: (context, ref, _) => ref.watch(isOrgAdminProvider) - ? MenuRow( - icon: LucideIcons.zap, - label: 'Automation', - description: - 'Rules that act when a record is created or changes', - onTap: () => context.push(AppRoutes.settingsAutomation), - ) - : const SizedBox.shrink(), + builder: (context, ref, _) { + // Members never ask: the rules answer them 403. + if (!ref.watch(isOrgAdminProvider) || + automationNotOffered(ref.watch(automationRulesProvider))) { + return const SizedBox.shrink(); + } + return MenuRow( + icon: LucideIcons.zap, + label: 'Automation', + description: + 'Rules that act when a record is created or changes', + onTap: () => context.push(AppRoutes.settingsAutomation), + ); + }, ), // Admins only, as on the web: the audit log answers a member 403. Consumer( diff --git a/mobile/test/screens/settings/automation_rules_screen_test.dart b/mobile/test/screens/settings/automation_rules_screen_test.dart index e2e6ce8d..76d43ce1 100644 --- a/mobile/test/screens/settings/automation_rules_screen_test.dart +++ b/mobile/test/screens/settings/automation_rules_screen_test.dart @@ -147,8 +147,57 @@ void main() { scrollable: find.byType(Scrollable).first, ); expect(find.text('Automation'), findsNothing); + expect(_FakeRules.builds, 0); }); + testWidgets('the hub hides Automation from admins where the API has none', ( + tester, + ) async { + await _pump( + tester, + home: const SettingsHubScreen(), + rules: () async => throw const AutomationNotOffered(), + ); + await tester.scrollUntilVisible( + find.text('Help center'), + 200, + scrollable: find.byType(Scrollable).first, + ); + expect(find.text('Automation'), findsNothing); + }); + + testWidgets( + 'the hub keeps Automation when the rules fail for another reason', + (tester) async { + await _pump( + tester, + home: const SettingsHubScreen(), + rules: () async => throw Exception('boom'), + ); + await tester.scrollUntilVisible( + find.text('Automation'), + 200, + scrollable: find.byType(Scrollable).first, + ); + expect(find.text('Automation'), findsOneWidget); + }, + ); + + testWidgets( + 'the screen says rules are not available where the API has none', + (tester) async { + await _pump( + tester, + rules: () async => throw const AutomationNotOffered(), + ); + expect( + find.text('Automation rules are not available here.'), + findsOneWidget, + ); + expect(find.text('Try again'), findsNothing); + }, + ); + testWidgets( 'a member who opens the screen sees the notice and no request is made', (tester) async { diff --git a/mobile/test/unit/automation_provider_test.dart b/mobile/test/unit/automation_provider_test.dart index 87cd67b7..d0e54633 100644 --- a/mobile/test/unit/automation_provider_test.dart +++ b/mobile/test/unit/automation_provider_test.dart @@ -100,6 +100,39 @@ void main() { ); }); + test('an API without automation answers not offered', () async { + client.routes['GET /api/automation/rules/'] = ( + 404, + '{"detail":"Not found."}', + ); + container.listen(automationRulesProvider, (_, _) {}); + await expectLater( + container.read(automationRulesProvider.future), + throwsA(isA()), + ); + expect( + automationNotOffered(container.read(automationRulesProvider)), + isTrue, + ); + expect( + const AutomationNotOffered().toString(), + 'Automation rules are not available here.', + ); + }); + + test('any other failure is not read as not offered', () async { + client.routes['GET /api/automation/rules/'] = (500, '{"detail":"boom"}'); + container.listen(automationRulesProvider, (_, _) {}); + await expectLater( + container.read(automationRulesProvider.future), + throwsA(isNot(isA())), + ); + expect( + automationNotOffered(container.read(automationRulesProvider)), + isFalse, + ); + }); + test('a failed load is an error, not an empty answer', () async { client.routes['GET /api/automation/rules/'] = (500, '{"detail":"boom"}'); await expectLater(