Link a deal only to an account its editor can open, and hide automation where the API has none (1.22.1)
This commit is contained in:
Ashwin
parent
5341cd86a3
commit
cffe004746
15 changed files with 449 additions and 35 deletions
|
|
@ -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.
|
||||
|
|
|
|||
222
backend/opportunity/tests/test_deal_account_visibility.py
Normal file
222
backend/opportunity/tests/test_deal_account_visibility.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -704,7 +704,7 @@ wheels = [
|
|||
|
||||
[[package]]
|
||||
name = "django-crm"
|
||||
version = "1.22.0"
|
||||
version = "1.22.1"
|
||||
source = { editable = "." }
|
||||
dependencies = [
|
||||
{ name = "cairocffi" },
|
||||
|
|
|
|||
|
|
@ -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` |
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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: [
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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<AutomationRulesAnswer> {
|
||||
@override
|
||||
Future<AutomationRulesAnswer> build() {
|
||||
|
|
@ -26,6 +36,7 @@ class AutomationRulesNotifier extends AsyncNotifier<AutomationRulesAnswer> {
|
|||
|
||||
Future<AutomationRulesAnswer> _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<AutomationRulesAnswer> v,
|
||||
) => v.isLoading || v.hasError ? null : v.value;
|
||||
|
||||
/// True only when the last load found no automation on this API.
|
||||
bool automationNotOffered(AsyncValue<AutomationRulesAnswer> v) =>
|
||||
v.error is AutomationNotOffered;
|
||||
|
||||
final automationRulesProvider =
|
||||
AsyncNotifierProvider<AutomationRulesNotifier, AutomationRulesAnswer>(
|
||||
AutomationRulesNotifier.new,
|
||||
|
|
|
|||
|
|
@ -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')),
|
||||
],
|
||||
],
|
||||
),
|
||||
),
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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<AutomationNotOffered>()),
|
||||
);
|
||||
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<AutomationNotOffered>())),
|
||||
);
|
||||
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(
|
||||
|
|
|
|||
Loading…
Reference in a new issue