diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ccb55a..abd4aac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,7 @@ jobs: - name: Install working-directory: /home/runner/frappe-bench run: | + bench get-app frappe_factory_bot https://github.com/harshtandiya/frappe_factory_bot.git --branch main bench get-app forms_pro $GITHUB_WORKSPACE bench setup requirements --dev bench new-site --db-root-password root --admin-password admin test_site diff --git a/README.md b/README.md index e70aefe..d33e969 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,28 @@ The project includes automated testing via GitHub Actions: - **CI**: Installs the app and runs unit tests on pull requests and pushes to `develop` - **Linters**: Runs [Frappe Semgrep Rules](https://github.com/frappe/semgrep-rules) and [pip-audit](https://pypi.org/project/pip-audit/) on every pull request +### Running Tests Locally + +The test suite uses [frappe_factory_bot](https://github.com/harshtandiya/frappe_factory_bot) for test data factories. You must install it on your bench before running tests. + +**1. Install frappe_factory_bot:** + +```bash +cd $PATH_TO_YOUR_BENCH +bench get-app https://github.com/harshtandiya/frappe_factory_bot.git --branch main +bench --site install-app frappe_factory_bot +``` + +**2. Run the tests:** + +```bash +# Run all Forms Pro tests +bench --site run-tests --app forms_pro + +# Run a specific test module +bench --site run-tests --module forms_pro.tests.test_roles +``` + ## 📄 License This project is licensed under the **AGPL-3.0** License - see the [LICENSE](LICENSE) file for details. diff --git a/forms_pro/forms_pro/doctype/fp_team/test_fp_team.py b/forms_pro/forms_pro/doctype/fp_team/test_fp_team.py index 20caaad..b7580b5 100644 --- a/forms_pro/forms_pro/doctype/fp_team/test_fp_team.py +++ b/forms_pro/forms_pro/doctype/fp_team/test_fp_team.py @@ -5,7 +5,7 @@ from frappe.defaults import get_user_default from frappe.tests import IntegrationTestCase -from forms_pro.tests import FORMS_PRO_TEST_USER +from forms_pro.tests.factories import FPTeamFactory, UserFactory # On IntegrationTestCase, the doctype test records and all # link-field test record dependencies are recursively loaded @@ -22,7 +22,7 @@ class IntegrationTestFPTeam(IntegrationTestCase): def setUp(self): super().setUp() - self.test_user = FORMS_PRO_TEST_USER + frappe.set_user("Administrator") def tearDown(self): frappe.set_user("Administrator") @@ -33,19 +33,18 @@ def test_add_owner_to_team(self): Test that after a user creates a team, that owner user is added to the team and the team is shared with the owner user """ - frappe.set_user(self.test_user) + owner = UserFactory.create("with_forms_pro_role") + frappe.set_user(owner.name) - team = frappe.new_doc("FP Team") - team.team_name = "Test Team" - team.insert() + team = FPTeamFactory.create() team.reload() frappe.set_user("Administrator") - # Check that the user is added to the team - self.assertTrue(team.is_team_member(self.test_user)) + # Check that the owner is added to the team + self.assertTrue(team.is_team_member(owner.name)) - # Check that the user is added to the team + # Check that the FP Team Member row exists self.assertIsNotNone( frappe.db.exists( "FP Team Member", @@ -53,19 +52,19 @@ def test_add_owner_to_team(self): "parent": team.name, "parentfield": "users", "parenttype": "FP Team", - "user": self.test_user, + "user": owner.name, }, ) ) - # Check that the user is added to the team's docshare + # Check that the team is shared with the owner self.assertTrue( frappe.db.exists( "DocShare", { "share_doctype": "FP Team", "share_name": team.name, - "user": self.test_user, + "user": owner.name, "read": 1, "write": 1, "share": 1, @@ -73,5 +72,5 @@ def test_add_owner_to_team(self): ) ) - # Check that the user's current team is set to the team - self.assertEqual(get_user_default("current_team", self.test_user), team.name) + # Check that the owner's current team is set to this team + self.assertEqual(get_user_default("current_team", owner.name), team.name) diff --git a/forms_pro/hooks.py b/forms_pro/hooks.py index 420b49b..7268006 100644 --- a/forms_pro/hooks.py +++ b/forms_pro/hooks.py @@ -9,11 +9,6 @@ fixtures = [ {"dt": "Role", "filters": {"role_name": "Forms Pro User"}}, ] -# Apps -# ------------------ - -# required_apps = [] - # Each item in the list will be shown as an app in the apps page add_to_apps_screen = [ { diff --git a/forms_pro/tests/factories/__init__.py b/forms_pro/tests/factories/__init__.py new file mode 100644 index 0000000..437dac2 --- /dev/null +++ b/forms_pro/tests/factories/__init__.py @@ -0,0 +1,4 @@ +from forms_pro.tests.factories.fp_team_factory import FPTeamFactory +from forms_pro.tests.factories.user_factory import UserFactory + +__all__ = ["FPTeamFactory", "UserFactory"] diff --git a/forms_pro/tests/factories/fp_team_factory.py b/forms_pro/tests/factories/fp_team_factory.py new file mode 100644 index 0000000..5981173 --- /dev/null +++ b/forms_pro/tests/factories/fp_team_factory.py @@ -0,0 +1,14 @@ +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +_fake = Faker() + + +class FPTeamFactory(BaseFactory): + doctype = "FP Team" + + @property + def default_attributes(self) -> dict[str, Any]: + return {"team_name": f"{_fake.word().capitalize()} Team"} diff --git a/forms_pro/tests/factories/user_factory.py b/forms_pro/tests/factories/user_factory.py new file mode 100644 index 0000000..28016ef --- /dev/null +++ b/forms_pro/tests/factories/user_factory.py @@ -0,0 +1,26 @@ +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from forms_pro.roles import FORMS_PRO_ROLE + +_fake = Faker() + + +class UserFactory(BaseFactory): + doctype = "User" + + @property + def default_attributes(self) -> dict[str, Any]: + return { + "email": _fake.unique.email(), + "first_name": _fake.first_name(), + "last_name": _fake.last_name(), + } + + @property + def with_forms_pro_role(self) -> dict[str, Any]: + # Frappe accepts child table rows as dicts in the initial doc dict. + # on_update fires with the role already set, so the default team is created. + return {"roles": [{"role": FORMS_PRO_ROLE}]} diff --git a/forms_pro/tests/factories/user_invitation_factory.py b/forms_pro/tests/factories/user_invitation_factory.py new file mode 100644 index 0000000..2e74763 --- /dev/null +++ b/forms_pro/tests/factories/user_invitation_factory.py @@ -0,0 +1,29 @@ +from typing import Any + +from faker import Faker +from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory + +from forms_pro.roles import FORMS_PRO_ROLE + +_fake = Faker() + + +class UserInvitationFactory(BaseFactory): + doctype = "User Invitation" + + @property + def default_attributes(self) -> dict[str, Any]: + return { + "email": self.overrides.get("email", _fake.unique.email()), + "redirect_to_path": self.overrides.get("redirect_to_path", "/forms"), + "invited_by": self.overrides.get("invited_by") or "Administrator", + "status": self.overrides.get("status") or "Pending", + } + + @property + def to_forms_pro_app(self) -> dict[str, Any]: + return { + "app_name": "forms_pro", + "redirect_to_path": "/forms", + "roles": [{"role": FORMS_PRO_ROLE}], + } diff --git a/forms_pro/tests/test_invitations.py b/forms_pro/tests/test_invitations.py index 886a29c..a1addf0 100644 --- a/forms_pro/tests/test_invitations.py +++ b/forms_pro/tests/test_invitations.py @@ -5,11 +5,16 @@ import frappe from faker import Faker +from frappe.model.document import Document from frappe.tests import IntegrationTestCase from forms_pro.api.team import add_member_to_team_via_invitation, invite_team_members from forms_pro.overrides.invitations import after_accept, after_insert from forms_pro.roles import FORMS_PRO_ROLE +from forms_pro.tests.factories import FPTeamFactory, UserFactory +from forms_pro.tests.factories.user_invitation_factory import UserInvitationFactory + +fake = Faker() class _StubRole: @@ -37,101 +42,56 @@ def setUp(self): super().setUp() self.sendmail_patcher = patch("frappe.sendmail") mock_sendmail = self.sendmail_patcher.start() - # Ensure the return value's .message is a string so any Frappe version - # that accesses q.message (e.g. for regex/parser ops) doesn't get a MagicMock. mock_sendmail.return_value.message = "" - self.fake = Faker() - self.teams_created = [] - self.users_created = [] frappe.set_user("Administrator") def tearDown(self): self.sendmail_patcher.stop() frappe.set_user("Administrator") - for team_id in self.teams_created: - if frappe.db.exists("FP Team", team_id): - frappe.delete_doc("FP Team", team_id, force=True, ignore_permissions=True) - for email in self.users_created: - if frappe.db.exists("User", email): - frappe.delete_doc("User", email, force=True, ignore_permissions=True) - frappe.db.delete("User Invitation", {"app_name": "forms_pro"}) - frappe.db.commit() super().tearDown() - def _create_user(self, email: str | None = None, with_forms_pro_role: bool = True) -> str: - email = email or self.fake.email() - if frappe.db.exists("User", email): - return email - user = frappe.get_doc( - { - "doctype": "User", - "email": email, - "first_name": self.fake.first_name(), - "last_name": self.fake.last_name(), - } - ) - user.insert(ignore_permissions=True) - if with_forms_pro_role: - user.append_roles(FORMS_PRO_ROLE) - user.save(ignore_permissions=True) - self.users_created.append(email) - return email - - def _create_team(self, owner: str, team_name: str | None = None) -> str: - frappe.set_user(owner) - team = frappe.get_doc( - { - "doctype": "FP Team", - "team_name": team_name or f"{self.fake.word()} Team", - } - ) - team.insert() - self.teams_created.append(team.name) - frappe.set_user("Administrator") - return team.name - - def _make_accepted_invitation(self, invitee_email: str, owner: str) -> object: + def _make_accepted_invitation(self, invitee: Document, owner: Document) -> Document: inv_doc = frappe.get_doc( { "doctype": "User Invitation", - "email": invitee_email, + "email": invitee.name, "app_name": "forms_pro", "redirect_to_path": "/forms", "roles": [{"role": FORMS_PRO_ROLE}], - "invited_by": owner, + "invited_by": owner.name, } ) inv_doc.insert(ignore_permissions=True) inv_doc.db_set("status", "Accepted") - inv_doc.db_set("user", invitee_email) - frappe.db.commit() + inv_doc.db_set("user", invitee.name) return inv_doc - def _get_team_memberships(self, email: str) -> list: - return frappe.get_all("FP Team Member", filters={"user": email}) + def _get_team_memberships(self, user: Document) -> list: + return frappe.get_all("FP Team Member", filters={"user": user.name}) # --- invite_team_members --- def test_invite_requires_permission(self): - """User without read permission on team cannot invite members.""" - owner = self._create_user() - other = self._create_user(with_forms_pro_role=False) - team_id = self._create_team(owner) + """User without write permission on team cannot invite members.""" + owner = UserFactory.create("with_forms_pro_role") + other = UserFactory.create() + frappe.set_user(owner.name) + team = FPTeamFactory.create() - frappe.set_user(other) + frappe.set_user(other.name) with self.assertRaises(frappe.PermissionError) as ctx: - invite_team_members(team_id=team_id, emails=[self.fake.email()]) + invite_team_members(team_id=team.name, emails=[other.email]) self.assertIn("permission", str(ctx.exception).lower()) def test_invite_creates_invitation_with_redirect(self): """Inviting creates a User Invitation whose redirect contains team_id and invite_id.""" - owner = self._create_user() - invitee_email = self.fake.email() - team_id = self._create_team(owner) + owner = UserFactory.create("with_forms_pro_role") + invitee_email = fake.email() - frappe.set_user(owner) - invite_team_members(team_id=team_id, emails=[invitee_email]) - frappe.db.commit() + frappe.set_user(owner.name) + team = FPTeamFactory.create() + + invite_team_members(team_id=team.name, emails=[invitee_email]) invitations = frappe.get_all( "User Invitation", @@ -140,7 +100,7 @@ def test_invite_creates_invitation_with_redirect(self): ) self.assertEqual(len(invitations), 1) inv = invitations[0] - self.assertIn(team_id, inv.redirect_to_path) + self.assertIn(team.name, inv.redirect_to_path) self.assertIn("add_member_to_team_via_invitation", inv.redirect_to_path) self.assertIn(f"invite_id={inv.name}", inv.redirect_to_path) @@ -148,99 +108,118 @@ def test_invite_creates_invitation_with_redirect(self): def test_after_insert_adds_invite_id_to_redirect(self): """after_insert hook rewrites redirect_to_path to include invite_id.""" - team_id = self._create_team(self._create_user()) - doc = frappe.get_doc( - { - "doctype": "User Invitation", - "email": self.fake.email(), - "app_name": "forms_pro", - "redirect_to_path": f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team_id}", - "roles": [{"role": FORMS_PRO_ROLE}], - } + owner = UserFactory.create("with_forms_pro_role") + frappe.set_user(owner.name) + team = FPTeamFactory.create() + frappe.set_user("Administrator") + + invite_doc = UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + team=team, + email=fake.email(), + redirect_to_path=f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team.name}", ) - doc.insert(ignore_permissions=True) - doc.reload() - self.assertIn(team_id, doc.redirect_to_path) - self.assertIn(f"invite_id={doc.name}", doc.redirect_to_path) - self.assertIn("add_member_to_team_via_invitation", doc.redirect_to_path) - frappe.delete_doc("User Invitation", doc.name, force=True) + self.assertIn(team.name, invite_doc.redirect_to_path) + self.assertIn(f"invite_id={invite_doc.name}", invite_doc.redirect_to_path) + self.assertIn("add_member_to_team_via_invitation", invite_doc.redirect_to_path) def test_after_insert_skips_non_forms_pro_app(self): """after_insert does not modify redirect when app_name is not forms_pro.""" original_path = "/some/path?team_id=abc" - doc = _StubInvitationDoc( - app_name="other_app", redirect_to_path=original_path, roles=[_StubRole(FORMS_PRO_ROLE)] - ) - after_insert(doc, "after_insert") - self.assertEqual(doc.redirect_to_path, original_path) - - def test_after_insert_skips_wrong_roles(self): - """after_insert does not modify redirect when roles do not include Forms Pro User.""" - team_id = self._create_team(self._create_user()) - original_path = f"/api/v2/method/some.method?team_id={team_id}" - doc = _StubInvitationDoc( - app_name="forms_pro", redirect_to_path=original_path, roles=[_StubRole("System Manager")] + invite_doc = UserInvitationFactory.create( + app_name="frappe", + redirect_to_path=original_path, + roles=[{"role": "System Manager"}], + invited_by="Administrator", ) - after_insert(doc, "after_insert") - self.assertEqual(doc.redirect_to_path, original_path) + after_insert(invite_doc, "after_insert") + self.assertEqual(invite_doc.redirect_to_path, original_path) + + def test_after_insert_raises_when_roles_do_not_include_forms_pro_user(self): + """after_insert raises when roles do not include Forms Pro User for forms_pro app""" + owner = UserFactory.create("with_forms_pro_role") + frappe.set_user(owner.name) + + team = FPTeamFactory.create() + frappe.set_user("Administrator") + + original_path = f"/api/v2/method/some.method?team_id={team.name}" + role = "System Manager" + + with self.assertRaises(frappe.ValidationError) as ctx: + UserInvitationFactory.create( + app_name="forms_pro", + redirect_to_path=original_path, + roles=[{"role": role}], + ) + self.assertIn(f"{role} is not an allowed role for forms_pro", str(ctx.exception)) # --- add_member_to_team_via_invitation --- - def test_add_member_raises_when_invitation_not_accepted(self): + def test_add_member_to_team_via_invitation_raises_when_invitation_is_pending(self): """Raises PermissionError when invitation status is not Accepted.""" - owner = self._create_user() - team_id = self._create_team(owner) - inv_doc = frappe.get_doc( - { - "doctype": "User Invitation", - "email": self.fake.email(), - "app_name": "forms_pro", - "redirect_to_path": "/forms", - "roles": [{"role": FORMS_PRO_ROLE}], - "invited_by": owner, - "status": "Pending", - } + owner = UserFactory.create("with_forms_pro_role") + frappe.set_user(owner.name) + team = FPTeamFactory.create() + frappe.set_user("Administrator") + + inv_doc = UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + team=team, + email=fake.email(), + redirect_to_path=f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team.name}", + status="Pending", ) - inv_doc.insert(ignore_permissions=True) - frappe.db.commit() with self.assertRaises(frappe.PermissionError) as ctx: - add_member_to_team_via_invitation(team_id=team_id, invite_id=inv_doc.name) + add_member_to_team_via_invitation(team_id=team.name, invite_id=inv_doc.name) self.assertIn("Invitation not accepted", str(ctx.exception)) - def test_add_member_raises_when_user_not_found(self): + def test_add_member_to_team_via_invitation_raises_when_user_not_found(self): """Raises PermissionError when the invited email has no User record.""" - owner = self._create_user() - team_id = self._create_team(owner) - inv_doc = frappe.get_doc( - { - "doctype": "User Invitation", - "email": self.fake.email(), - "app_name": "forms_pro", - "redirect_to_path": "/forms", - "roles": [{"role": FORMS_PRO_ROLE}], - "invited_by": owner, - } + owner = UserFactory.create("with_forms_pro_role") + frappe.set_user(owner.name) + team = FPTeamFactory.create() + frappe.set_user("Administrator") + + inv_doc = UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + team=team, + email=fake.email(), + redirect_to_path=f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team.name}", ) - inv_doc.insert(ignore_permissions=True) - inv_doc.db_set("status", "Accepted") - frappe.db.commit() + inv_doc.status = "Accepted" + inv_doc.save(ignore_permissions=True) with self.assertRaises(frappe.PermissionError) as ctx: - add_member_to_team_via_invitation(team_id=team_id, invite_id=inv_doc.name) + add_member_to_team_via_invitation(team_id=team.name, invite_id=inv_doc.name) self.assertIn("User not found", str(ctx.exception)) - def test_add_member_adds_user_to_team_and_redirects(self): + def test_add_member_to_team_via_invitation_adds_user_to_team_and_redirects(self): """Successfully adds user to team and sets redirect response to /forms.""" - owner = self._create_user() - invitee_email = self._create_user(with_forms_pro_role=False) - team_id = self._create_team(owner) - inv_doc = self._make_accepted_invitation(invitee_email, owner) + owner = UserFactory.create("with_forms_pro_role") + invitee = UserFactory.create() + frappe.set_user(owner.name) + team = FPTeamFactory.create() + frappe.set_user("Administrator") - add_member_to_team_via_invitation(team_id=team_id, invite_id=inv_doc.name) + inv_doc = UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + team=team, + email=invitee.email, + redirect_to_path=f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team.name}", + ) + inv_doc.status = "Accepted" + inv_doc.save(ignore_permissions=True) + + add_member_to_team_via_invitation(team_id=team.name, invite_id=inv_doc.name) - team = frappe.get_doc("FP Team", team_id) - self.assertIn(invitee_email, [row.user for row in team.users]) + team_doc = frappe.get_doc("FP Team", team.name) + self.assertIn(invitee.name, [row.user for row in team_doc.users]) self.assertEqual(frappe.local.response.get("type"), "redirect") self.assertEqual(frappe.local.response.get("location"), "/forms") @@ -248,55 +227,46 @@ def test_add_member_adds_user_to_team_and_redirects(self): def test_after_accept_adds_user_to_inviting_team(self): """after_accept adds the invited user to the team and updates redirect to /forms.""" - owner = self._create_user() - invitee_email = self._create_user(with_forms_pro_role=False) - team_id = self._create_team(owner) + owner = UserFactory.create("with_forms_pro_role") + invitee = UserFactory.create() + frappe.set_user(owner.name) + team = FPTeamFactory.create() + frappe.set_user("Administrator") - inv_doc = frappe.get_doc( - { - "doctype": "User Invitation", - "email": invitee_email, - "app_name": "forms_pro", - "redirect_to_path": f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team_id}&invite_id=TEST-001", - "roles": [{"role": FORMS_PRO_ROLE}], - "invited_by": owner, - } + inv_doc = UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + team=team, + email=invitee.email, + redirect_to_path=f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team.name}", ) - inv_doc.insert(ignore_permissions=True) - frappe.db.commit() + inv_doc.status = "Accepted" + inv_doc.save(ignore_permissions=True) - invitee_user = frappe.get_doc("User", invitee_email) - after_accept(invitation=inv_doc, user=invitee_user, user_inserted=True) + after_accept(invitation=inv_doc, user=invitee, user_inserted=True) - team = frappe.get_doc("FP Team", team_id) - self.assertIn(invitee_email, [row.user for row in team.users]) + team_doc = frappe.get_doc("FP Team", team.name) + self.assertIn(invitee.name, [row.user for row in team_doc.users]) self.assertEqual(inv_doc.redirect_to_path, "/forms") - frappe.delete_doc("User Invitation", inv_doc.name, force=True) def test_after_accept_skips_when_no_team_id(self): """after_accept is a no-op when redirect_to_path has no team_id.""" - owner = self._create_user() - invitee_email = self._create_user(with_forms_pro_role=False) - inv_doc = frappe.get_doc( - { - "doctype": "User Invitation", - "email": invitee_email, - "app_name": "forms_pro", - "redirect_to_path": "/forms", - "roles": [{"role": FORMS_PRO_ROLE}], - "invited_by": owner, - } + owner = UserFactory.create("with_forms_pro_role") + invitee = UserFactory.create() + + inv_doc = UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + email=invitee.email, + redirect_to_path="/forms", ) - inv_doc.insert(ignore_permissions=True) - frappe.db.commit() + inv_doc.status = "Accepted" + inv_doc.save(ignore_permissions=True) - invitee_user = frappe.get_doc("User", invitee_email) - after_accept(invitation=inv_doc, user=invitee_user, user_inserted=True) + after_accept(invitation=inv_doc, user=invitee, user_inserted=True) - # redirect_to_path unchanged; no team membership created self.assertEqual(inv_doc.redirect_to_path, "/forms") - self.assertEqual(self._get_team_memberships(invitee_email), []) - frappe.delete_doc("User Invitation", inv_doc.name, force=True) + self.assertEqual(self._get_team_memberships(invitee), []) # --- default team creation guard (bug fix) --- @@ -306,36 +276,32 @@ def test_no_default_team_created_for_invited_user(self): the role-change hook must NOT create a default team — the invitation redirect will place them in the correct inviting team instead. """ - invitee_email = self._create_user(with_forms_pro_role=False) - owner = self._create_user() - team_id = self._create_team(owner) + invitee = UserFactory.create() + owner = UserFactory.create("with_forms_pro_role") + frappe.set_user(owner.name) + team = FPTeamFactory.create() + frappe.set_user("Administrator") # Simulate a pending invitation (exists before the user accepts) - frappe.get_doc( - { - "doctype": "User Invitation", - "email": invitee_email, - "app_name": "forms_pro", - "redirect_to_path": f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team_id}", - "roles": [{"role": FORMS_PRO_ROLE}], - "invited_by": owner, - "status": "Pending", - } - ).insert(ignore_permissions=True) - frappe.db.commit() - - memberships_before = self._get_team_memberships(invitee_email) + UserInvitationFactory.create( + "to_forms_pro_app", + invited_by=owner.name, + email=invitee.email, + redirect_to_path=f"/api/v2/method/forms_pro.api.team.add_member_to_team_via_invitation?team_id={team.name}", + status="Pending", + ) # Assigning the Forms Pro role is what Frappe does on invitation acceptance; # the hook must skip default team creation here. - user = frappe.get_doc("User", invitee_email) - user.append_roles(FORMS_PRO_ROLE) - user.save(ignore_permissions=True) - frappe.db.commit() + invitee.append_roles(FORMS_PRO_ROLE) + invitee.save(ignore_permissions=True) + + # There should be no team memberships for the invited user + # because the role change hook will not create a default team. + # Team will be created by the invitation redirect. - memberships_after = self._get_team_memberships(invitee_email) self.assertEqual( - len(memberships_before), - len(memberships_after), + self._get_team_memberships(invitee), + [], "A default team must not be created for an invited user", ) diff --git a/forms_pro/tests/test_roles.py b/forms_pro/tests/test_roles.py index 5e0c8b0..a1fb895 100644 --- a/forms_pro/tests/test_roles.py +++ b/forms_pro/tests/test_roles.py @@ -1,9 +1,11 @@ +# Copyright (c) 2025, harsh@buildwithhussain.com and contributors +# For license information, please see license.txt + import frappe -from faker import Faker -from frappe.core.doctype.user.user import User from frappe.tests import IntegrationTestCase from forms_pro.roles import FORMS_PRO_ROLE +from forms_pro.tests.factories import UserFactory from forms_pro.utils.teams import get_user_teams @@ -12,20 +14,13 @@ def setUp(self): super().setUp() def test_roles(self): - fake = Faker() - user: User = frappe.get_doc( - { - "doctype": "User", - "email": fake.email(), - "first_name": fake.first_name(), - "last_name": fake.last_name(), - } - ) - user.insert() + user = UserFactory.create() roles = frappe.get_roles(user.name) self.assertNotIn(FORMS_PRO_ROLE, roles) self.assertEqual(len(get_user_teams(user.name)), 0) + # Role assignment post-insert is intentional here: this test specifically + # exercises the on_update hook that fires when a role is added after creation. user.append_roles(FORMS_PRO_ROLE) user.save() roles = frappe.get_roles(user.name)