fix: Disable invite button when email config is not set (#5022)

Co-authored-by: Kim Gustyr <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

api/app/utils.py

@@ -4,6 +4,7 @@
 from typing import TypedDict
 
 import shortuuid
+from django.conf import settings
 
 UNKNOWN = "unknown"
 VERSIONS_INFO_FILE_LOCATION = ".versions.json"
@@ -12,6 +13,7 @@
 class VersionInfo(TypedDict):
     ci_commit_sha: str
     image_tag: str
+    has_email_provider: bool
     is_enterprise: bool
     is_saas: bool
 
@@ -29,6 +31,18 @@ def is_saas() -> bool:
     return pathlib.Path("./SAAS_DEPLOYMENT").exists()
 
 
+def has_email_provider() -> bool:
+    match settings.EMAIL_BACKEND:
+        case "django.core.mail.backends.smtp.EmailBackend":
+            return settings.EMAIL_HOST_USER is not None
+        case "sgbackend.SendGridBackend":
+            return settings.SENDGRID_API_KEY is not None
+        case "django_ses.SESBackend":
+            return settings.AWS_SES_REGION_ENDPOINT is not None
+        case _:
+            return False
+
+
 @lru_cache
 def get_version_info() -> VersionInfo:
     """Reads the version info baked into src folder of the docker container"""
@@ -45,6 +59,7 @@ def get_version_info() -> VersionInfo:
     version_json = version_json | {
         "ci_commit_sha": _get_file_contents("./CI_COMMIT_SHA"),
         "image_tag": image_tag,
+        "has_email_provider": has_email_provider(),
         "is_enterprise": is_enterprise(),
         "is_saas": is_saas(),
     }

api/pyproject.toml

@@ -228,6 +228,7 @@ django-extensions = "^3.2.3"
 pdbpp = "^0.10.3"
 mypy-boto3-dynamodb = "^1.33.0"
 pytest-structlog = "^1.1"
+pyfakefs = "^5.7.4"
 
 [build-system]
 requires = ["poetry>=2.0.0"]

api/tests/unit/app/conftest.py

@@ -0,0 +1,11 @@
+from typing import Generator
+
+import pytest
+
+from app.utils import get_version_info
+
+
+@pytest.fixture(autouse=True)
+def clear_get_version_info_cache() -> Generator[None, None, None]:
+    yield
+    get_version_info.cache_clear()

api/tests/unit/app/test_unit_app_utils.py

@@ -1,42 +1,21 @@
 import json
-import pathlib
-from typing import Generator
-from unittest import mock
 
 import pytest
-from pytest_mock import MockerFixture
+from pyfakefs.fake_filesystem import FakeFilesystem
+from pytest_django.fixtures import SettingsWrapper
 
 from app.utils import get_version_info
 
 
-@pytest.fixture(autouse=True)
-def clear_get_version_info_cache() -> Generator[None, None, None]:
-    yield
-    get_version_info.cache_clear()
-
-
-def test_get_version_info(mocker: MockerFixture) -> None:
+def test_get_version_info(fs: FakeFilesystem) -> None:
     # Given
-    mocked_pathlib = mocker.patch("app.utils.pathlib")
-
-    def path_side_effect(file_path: str) -> mocker.MagicMock:
-        mocked_path_object = mocker.MagicMock(spec=pathlib.Path)
-
-        if file_path == "./ENTERPRISE_VERSION":
-            mocked_path_object.exists.return_value = True
-
-        if file_path == "./SAAS_DEPLOYMENT":
-            mocked_path_object.exists.return_value = False
-
-        return mocked_path_object
-
-    mocked_pathlib.Path.side_effect = path_side_effect
-
-    manifest_mocked_file = {
+    expected_manifest_contents = {
         ".": "2.66.2",
     }
-    mock_get_file_contents = mocker.patch("app.utils._get_file_contents")
-    mock_get_file_contents.side_effect = (json.dumps(manifest_mocked_file), "some_sha")
+
+    fs.create_file("./ENTERPRISE_VERSION")
+    fs.create_file(".versions.json", contents=json.dumps(expected_manifest_contents))
+    fs.create_file("./CI_COMMIT_SHA", contents="some_sha")
 
     # When
     result = get_version_info()
@@ -45,29 +24,16 @@ def path_side_effect(file_path: str) -> mocker.MagicMock:
     assert result == {
         "ci_commit_sha": "some_sha",
         "image_tag": "2.66.2",
+        "has_email_provider": False,
         "is_enterprise": True,
         "is_saas": False,
         "package_versions": {".": "2.66.2"},
     }
 
 
-def test_get_version_info_with_missing_files(mocker: MockerFixture) -> None:
+def test_get_version_info_with_missing_files(fs: FakeFilesystem) -> None:
     # Given
-    mocked_pathlib = mocker.patch("app.utils.pathlib")
-
-    def path_side_effect(file_path: str) -> mocker.MagicMock:
-        mocked_path_object = mocker.MagicMock(spec=pathlib.Path)
-
-        if file_path == "./ENTERPRISE_VERSION":
-            mocked_path_object.exists.return_value = True
-
-        if file_path == "./SAAS_DEPLOYMENT":
-            mocked_path_object.exists.return_value = False
-
-        return mocked_path_object
-
-    mocked_pathlib.Path.side_effect = path_side_effect
-    mock.mock_open.side_effect = IOError
+    fs.create_file("./ENTERPRISE_VERSION")
 
     # When
     result = get_version_info()
@@ -76,6 +42,58 @@ def path_side_effect(file_path: str) -> mocker.MagicMock:
     assert result == {
         "ci_commit_sha": "unknown",
         "image_tag": "unknown",
+        "has_email_provider": False,
         "is_enterprise": True,
         "is_saas": False,
     }
+
+
+EMAIL_BACKENDS_AND_SETTINGS = [
+    ("django.core.mail.backends.smtp.EmailBackend", "EMAIL_HOST_USER"),
+    ("django_ses.SESBackend", "AWS_SES_REGION_ENDPOINT"),
+    ("sgbackend.SendGridBackend", "SENDGRID_API_KEY"),
+]
+
+
+@pytest.mark.parametrize(
+    "email_backend,expected_setting_name",
+    EMAIL_BACKENDS_AND_SETTINGS,
+)
+def test_get_version_info__email_config_enabled__return_expected(
+    settings: SettingsWrapper,
+    email_backend: str,
+    expected_setting_name: str,
+) -> None:
+    # Given
+    settings.EMAIL_BACKEND = email_backend
+    setattr(settings, expected_setting_name, "value")
+
+    # When
+    result = get_version_info()
+
+    # Then
+    assert result["has_email_provider"] is True
+
+
+@pytest.mark.parametrize(
+    "email_backend,expected_setting_name",
+    [
+        (None, None),
+        *EMAIL_BACKENDS_AND_SETTINGS,
+    ],
+)
+def test_get_version_info__email_config_disabled__return_expected(
+    settings: SettingsWrapper,
+    email_backend: str | None,
+    expected_setting_name: str | None,
+) -> None:
+    # Given
+    settings.EMAIL_BACKEND = email_backend
+    if expected_setting_name:
+        setattr(settings, expected_setting_name, None)
+
+    # When
+    result = get_version_info()
+
+    # Then
+    assert result["has_email_provider"] is False

api/tests/unit/app/test_unit_app_views.py

@@ -15,6 +15,7 @@ def test_get_version_info(api_client: APIClient) -> None:
     assert response.json() == {
         "ci_commit_sha": "unknown",
         "image_tag": "unknown",
+        "has_email_provider": False,
         "is_enterprise": False,
         "is_saas": False,
     }

frontend/common/utils/utils.tsx

@@ -552,6 +552,8 @@ const Utils = Object.assign({}, require('./base/_utils'), {
   getViewIdentitiesPermission() {
     return 'VIEW_IDENTITIES'
   },
+  hasEmailProvider: () =>
+    global.flagsmithVersion?.backend?.has_email_provider ?? false,
   isEnterpriseImage: () => global.flagsmithVersion?.backend.is_enterprise,
   isMigrating() {
     const model = ProjectStore.model as null | ProjectType

frontend/web/components/pages/UsersAndPermissionsPage.tsx

@@ -45,6 +45,7 @@ type UsersAndPermissionsPageType = {
 }
 
 const widths = [300, 200, 80]
+const noEmailProvider = `You must configure an email provider before using email invites. Please read our documentation on how to configure an email provider.`
 
 type UsersAndPermissionsInnerType = {
   organisation: Organisation
@@ -73,6 +74,7 @@ const UsersAndPermissionsInner: FC<UsersAndPermissionsInnerType> = ({
   const verifySeatsLimit = Utils.getFlagsmithHasFeature(
     'verify_seats_limit_for_invite_links',
   )
+  const hasEmailProvider = Utils.hasEmailProvider()
   const manageUsersPermission = useHasPermission({
     id: AccountStore.getOrganisation()?.id,
     level: 'organisation',
@@ -84,6 +86,12 @@ const UsersAndPermissionsInner: FC<UsersAndPermissionsInnerType> = ({
     permission: 'MANAGE_USER_GROUPS',
   })
 
+  const hasInvitePermission =
+    hasEmailProvider && manageUsersPermission.permission
+  const tooltTipText = !hasEmailProvider
+    ? noEmailProvider
+    : Constants.organisationPermissions('Admin')
+
   const roleChanged = (id: number, { value: role }: { value: string }) => {
     AppActions.updateUserRole(id, role)
   }
@@ -229,10 +237,11 @@ const UsersAndPermissionsInner: FC<UsersAndPermissionsInnerType> = ({
                       <Row space className='mt-4'>
                         <h5 className='mb-0'>Team Members</h5>
                         {Utils.renderWithPermission(
-                          !manageUsersPermission.permission,
-                          Constants.organisationPermissions('Admin'),
+                          hasInvitePermission,
+                          tooltTipText,
                           <Button
                             disabled={
+                              !hasEmailProvider ||
                               needsUpgradeForAdditionalSeats ||
                               !manageUsersPermission.permission
                             }