diff --git a/backend/config.py b/backend/config.py index bcb1f33346..523b49abb7 100644 --- a/backend/config.py +++ b/backend/config.py @@ -152,3 +152,4 @@ class TestEnvironmentConfig(EnvironmentConfig): + f"{POSTGRES_PORT}" + f"/test_{POSTGRES_DB}" ) + LOG_LEVEL = "DEBUG" diff --git a/backend/models/dtos/team_dto.py b/backend/models/dtos/team_dto.py index 634baf442f..9273464deb 100644 --- a/backend/models/dtos/team_dto.py +++ b/backend/models/dtos/team_dto.py @@ -41,6 +41,9 @@ class TeamMembersDTO(Model): username = StringType(required=True) function = StringType(required=True, validators=[validate_team_member_function]) active = StringType() + join_request_notifications = BooleanType( + default=False, serialized_name="joinRequestNotifications" + ) picture_url = StringType(serialized_name="pictureUrl") @@ -139,7 +142,7 @@ class UpdateTeamDTO(Model): name = StringType() logo = StringType() description = StringType() - invite_only = BooleanType(default=False, serialized_name="inviteOnly") + invite_only = BooleanType(serialized_name="inviteOnly") visibility = StringType( validators=[validate_team_visibility], serialize_when_none=False ) diff --git a/backend/models/dtos/user_dto.py b/backend/models/dtos/user_dto.py index 6ce72e8260..f0a6ff6db9 100644 --- a/backend/models/dtos/user_dto.py +++ b/backend/models/dtos/user_dto.py @@ -79,7 +79,9 @@ class UserDTO(Model): tasks_comments_notifications = BooleanType( serialized_name="taskCommentsNotifications" ) - teams_notifications = BooleanType(serialized_name="teamsNotifications") + teams_announcement_notifications = BooleanType( + serialized_name="teamsAnnouncementNotifications" + ) # these are read only gender = StringType( diff --git a/backend/models/postgis/team.py b/backend/models/postgis/team.py index b2ebdb3bc9..564c7c0074 100644 --- a/backend/models/postgis/team.py +++ b/backend/models/postgis/team.py @@ -26,7 +26,9 @@ class TeamMembers(db.Model): ) function = db.Column(db.Integer, nullable=False) # either 'editor' or 'manager' active = db.Column(db.Boolean, default=False) - + join_request_notifications = db.Column( + db.Boolean, nullable=False, default=False + ) # Managers can turn notifications on/off for team join requests member = db.relationship( User, backref=db.backref("teams", cascade="all, delete-orphan") ) @@ -131,18 +133,25 @@ def update(self, team_dto: TeamDTO): if team_dto.members != self._get_team_members() and team_dto.members: for member in self.members: - db.session.delete(member) - + member_name = User.get_by_id(member.user_id).username + if member_name not in [i["username"] for i in team_dto.members]: + member.delete() for member in team_dto.members: - user = User.get_by_username(member["userName"]) - + user = User.get_by_username(member["username"]) if user is None: raise NotFound("User not found") - - new_team_member = TeamMembers() - new_team_member.team = self - new_team_member.member = user - new_team_member.function = TeamMemberFunctions[member["function"]].value + team_member = TeamMembers.get(self.id, user.id) + if team_member: + team_member.join_request_notifications = member[ + "join_request_notifications" + ] + else: + new_team_member = TeamMembers() + new_team_member.team = self + new_team_member.member = user + new_team_member.function = TeamMemberFunctions[ + member["function"] + ].value db.session.commit() @@ -205,6 +214,7 @@ def as_dto_team_member(self, member) -> TeamMembersDTO: member_dto.function = member_function member_dto.picture_url = user.picture_url member_dto.active = member.active + member_dto.join_request_notifications = member.join_request_notifications return member_dto def as_dto_team_project(self, project) -> TeamProjectDTO: diff --git a/backend/models/postgis/user.py b/backend/models/postgis/user.py index c10a431df5..c28625aa31 100644 --- a/backend/models/postgis/user.py +++ b/backend/models/postgis/user.py @@ -61,7 +61,9 @@ class User(db.Model): projects_notifications = db.Column(db.Boolean, default=True, nullable=False) tasks_notifications = db.Column(db.Boolean, default=True, nullable=False) tasks_comments_notifications = db.Column(db.Boolean, default=False, nullable=False) - teams_notifications = db.Column(db.Boolean, default=True, nullable=False) + teams_announcement_notifications = db.Column( + db.Boolean, default=True, nullable=False + ) date_registered = db.Column(db.DateTime, default=timestamp) # Represents the date the user last had one of their tasks validated last_validation_date = db.Column(db.DateTime, default=timestamp) @@ -364,7 +366,9 @@ def as_dto(self, logged_in_username: str) -> UserDTO: user_dto.projects_comments_notifications = self.projects_comments_notifications user_dto.tasks_notifications = self.tasks_notifications user_dto.tasks_comments_notifications = self.tasks_comments_notifications - user_dto.teams_notifications = self.teams_notifications + user_dto.teams_announcement_notifications = ( + self.teams_announcement_notifications + ) if self.username == logged_in_username: # Only return email address and gender information when logged in user is looking at their own profile diff --git a/backend/services/messaging/message_service.py b/backend/services/messaging/message_service.py index c11c103740..41732bff85 100644 --- a/backend/services/messaging/message_service.py +++ b/backend/services/messaging/message_service.py @@ -171,7 +171,7 @@ def _push_messages(messages): ): continue if ( - user.teams_notifications is False + user.teams_announcement_notifications is False and obj.message_type == MessageType.TEAM_BROADCAST.value ): messages_objs.append(obj) @@ -371,8 +371,9 @@ def send_request_to_join_team( MessageService.get_user_link(from_username), MessageService.get_team_link(team_name, team_id, True), ) - message.add_message() - message.save() + MessageService._push_messages( + [dict(message=message, user=User.query.get(to_user))] + ) @staticmethod def accept_reject_request_to_join_team( diff --git a/backend/services/team_service.py b/backend/services/team_service.py index 7290d07ba9..b4470a37fe 100644 --- a/backend/services/team_service.py +++ b/backend/services/team_service.py @@ -91,13 +91,13 @@ def join_team(team_id: int, requesting_user: int, username: str, role: str = Non active = True TeamService.add_team_member(team_id, user.id, role, active) - if team.invite_only: team_managers = team.get_team_managers() - for member in team_managers: - MessageService.send_request_to_join_team( - user.id, user.username, member.user_id, team.name, team_id - ) + for manager in team_managers: + if manager.join_request_notifications: + MessageService.send_request_to_join_team( + user.id, user.username, manager.user_id, team.name, team_id + ) @staticmethod def send_invite(team_id, from_user_id, username): diff --git a/frontend/src/components/teamsAndOrgs/members.js b/frontend/src/components/teamsAndOrgs/members.js index 617bd7a6f6..7f5883f4b5 100644 --- a/frontend/src/components/teamsAndOrgs/members.js +++ b/frontend/src/components/teamsAndOrgs/members.js @@ -8,6 +8,7 @@ import messages from './messages'; import { UserAvatar } from '../user/avatar'; import { EditModeControl } from './editMode'; import { Button } from '../button'; +import { SwitchToggle } from '../formInputs'; import { fetchLocalJSONAPI, pushToLocalJSONAPI } from '../../network/genericJSONRequest'; import { Alert } from '../alert'; import { useOnClickOutside } from '../../hooks/UseOnClickOutside'; @@ -155,8 +156,31 @@ export function Members({ ); } -export function JoinRequests({ requests, teamId, addMembers, updateRequests }: Object) { +export function JoinRequests({ + requests, + teamId, + addMembers, + updateRequests, + managers, + updateTeam, + isTeamInviteOnly, +}: Object) { const token = useSelector((state) => state.auth.get('token')); + const { username: loggedInUsername } = useSelector((state) => state.auth.get('userDetails')); + + const showJoinRequestSwitch = + isTeamInviteOnly && + managers?.filter( + (manager) => manager.username === loggedInUsername && manager.function === 'MANAGER', + ).length > 0; + const [isChecked, setIsChecked] = useState(false); + + useEffect(() => { + const isJoinRequestEnabled = managers.filter( + (manager) => manager.username === loggedInUsername, + )[0]?.joinRequestNotifications; + setIsChecked(isJoinRequestEnabled); + }, [loggedInUsername, managers]); const acceptRejectRequest = useCallback( (user, action) => { @@ -176,6 +200,18 @@ export function JoinRequests({ requests, teamId, addMembers, updateRequests }: O [teamId, requests, updateRequests, addMembers, token], ); + const handleJoinRequestNotificationsChange = (e) => { + const { checked } = e.target; + setIsChecked(checked); + let member = managers.find((member) => member.username === loggedInUsername); + + Object.assign(member, { + joinRequestNotifications: checked, + active: checked.toString(), + }); + updateTeam({ members: [member] }); + }; + return (
@@ -183,6 +219,18 @@ export function JoinRequests({ requests, teamId, addMembers, updateRequests }: O
+ {showJoinRequestSwitch && ( +
+ +
+ handleJoinRequestNotificationsChange(e)} + labelPosition="right" + /> +
+
+ )}
{requests.map((user, n) => (
@@ -213,7 +261,7 @@ export function JoinRequests({ requests, teamId, addMembers, updateRequests }: O
))} {requests.length === 0 && ( -
+
)} diff --git a/frontend/src/components/teamsAndOrgs/messages.js b/frontend/src/components/teamsAndOrgs/messages.js index a86d12260b..47cde645f7 100644 --- a/frontend/src/components/teamsAndOrgs/messages.js +++ b/frontend/src/components/teamsAndOrgs/messages.js @@ -417,6 +417,11 @@ export default defineMessages({ id: 'management.teams.invite_only.description', defaultMessage: "Managers need to approve a member's request to join.", }, + newJoinRequestNotification: { + id: 'management.teams.newJoinRequestNotification', + defaultMessage: + 'Enable for team managers to receive (email) notifications each time a new join request is made', + }, waitingApproval: { id: 'teamsAndOrgs.management.teams.messages.waiting_approval', defaultMessage: 'Your request to join this team is waiting for approval.', diff --git a/frontend/src/components/teamsAndOrgs/tests/members.test.js b/frontend/src/components/teamsAndOrgs/tests/members.test.js index 30a5ab5594..7779c01129 100644 --- a/frontend/src/components/teamsAndOrgs/tests/members.test.js +++ b/frontend/src/components/teamsAndOrgs/tests/members.test.js @@ -19,7 +19,7 @@ describe('test JoinRequest list', () => { ]; const element = createComponentWithIntl( - + , ); const testInstance = element.root; @@ -53,7 +53,7 @@ describe('test JoinRequest list', () => { describe('test JoinRequest list without requests', () => { const element = createComponentWithIntl( - + , ); const testInstance = element.root; @@ -92,7 +92,7 @@ describe('test JoinRequest list without requests', () => { ); }); it('no requests message is present', () => { - expect(testInstance.findByProps({ className: 'tc' }).children[0].props.id).toBe( + expect(testInstance.findByProps({ className: 'tc mt3' }).children[0].props.id).toBe( 'management.teams.join_requests.empty', ); }); diff --git a/frontend/src/components/user/forms/notifications.js b/frontend/src/components/user/forms/notifications.js index 641d493569..318cf11d48 100644 --- a/frontend/src/components/user/forms/notifications.js +++ b/frontend/src/components/user/forms/notifications.js @@ -16,7 +16,7 @@ export function UserNotificationsForm(props) { - + diff --git a/frontend/src/views/teams.js b/frontend/src/views/teams.js index 3d9bf88e27..b8c53d5d3b 100644 --- a/frontend/src/views/teams.js +++ b/frontend/src/views/teams.js @@ -8,8 +8,8 @@ import messages from './messages'; import { useFetch } from '../hooks/UseFetch'; import { useEditTeamAllowed } from '../hooks/UsePermissions'; import { useSetTitleTag } from '../hooks/UseMetaTags'; +import useForceUpdate from '../hooks/UseForceUpdate'; import { fetchLocalJSONAPI, pushToLocalJSONAPI } from '../network/genericJSONRequest'; -import { useForceUpdate } from '../hooks/UseForceUpdate'; import { getMembersDiff, filterActiveMembers, @@ -220,7 +220,7 @@ export function EditTeam(props) { const [canUserEditTeam] = useEditTeamAllowed(team); const [memberJoinTeamError, setMemberJoinTeamError] = useState(null); const [managerJoinTeamError, setManagerJoinTeamError] = useState(null); - + useEffect(() => { if (!initManagers && team && team.members) { setManagers(filterActiveManagers(team.members)); @@ -288,6 +288,7 @@ export function EditTeam(props) { const updateTeam = (payload) => { pushToLocalJSONAPI(`teams/${props.id}/`, JSON.stringify(payload), token, 'PATCH'); + forceUpdate(); }; if (team && team.teamId && !canUserEditTeam) { @@ -351,6 +352,9 @@ export function EditTeam(props) { teamId={team.teamId} addMembers={addMembers} updateRequests={setRequests} + managers={managers} + updateTeam={updateTeam} + isTeamInviteOnly={team.inviteOnly} />
diff --git a/migrations/versions/bcb474128817_.py b/migrations/versions/bcb474128817_.py new file mode 100644 index 0000000000..815b1e5c29 --- /dev/null +++ b/migrations/versions/bcb474128817_.py @@ -0,0 +1,61 @@ +"""empty message + +Revision ID: bcb474128817 +Revises: 8b61ac59bc57 +Create Date: 2022-07-03 10:46:56.075805 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "bcb474128817" +down_revision = "8b61ac59bc57" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_index( + "idx_task_validation_validator_status_composite", + "task_invalidation_history", + ["invalidator_id", "is_closed"], + unique=False, + ) + op.add_column( + "team_members", + sa.Column("join_request_notifications", sa.Boolean(), nullable=True), + ) + op.add_column( + "users", + sa.Column("teams_announcement_notifications", sa.Boolean(), nullable=True), + ) + op.execute("UPDATE team_members SET join_request_notifications = false") + op.execute("UPDATE users SET teams_announcement_notifications = false") + op.alter_column("team_members", "join_request_notifications", nullable=False) + op.alter_column("users", "teams_announcement_notifications", nullable=False) + op.drop_column("users", "teams_notifications") + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column( + "users", + sa.Column( + "teams_notifications", + sa.BOOLEAN(), + server_default=sa.text("true"), + autoincrement=False, + nullable=False, + ), + ) + op.drop_column("users", "teams_announcement_notifications") + op.drop_column("team_members", "join_request_notifications") + op.drop_index( + "idx_task_validation_validator_status_composite", + table_name="task_invalidation_history", + ) + # ### end Alembic commands ### diff --git a/tests/backend/integration/services/messaging/test_smtp_service.py b/tests/backend/integration/services/messaging/test_smtp_service.py index 90a4621995..00a7d27df4 100644 --- a/tests/backend/integration/services/messaging/test_smtp_service.py +++ b/tests/backend/integration/services/messaging/test_smtp_service.py @@ -44,11 +44,11 @@ def test_send_alert(self): message_id=self.message_id, from_username=self.from_username, project_id=self.project_id, + project_name=self.project_name, task_id=self.task_id, subject=self.subject, content=self.content, message_type=self.message_type, - project_name=self.project_name, ) self.assertTrue(sent_alert) @@ -65,11 +65,11 @@ def test_send_alert_message_limits(self): message_id=self.message_id, from_username=self.from_username, project_id=self.project_id, + project_name=self.project_name, task_id=self.task_id, subject=self.subject, content=self.content, message_type=self.message_type, - project_name=self.project_name, ) self.assertTrue(sent_alert) @@ -84,11 +84,11 @@ def test_alert_not_sent_if_email_not_supplied(self): message_id=self.message_id, from_username=self.from_username, project_id=self.project_id, + project_name=self.project_name, task_id=self.task_id, subject=self.subject, content=self.content, message_type=self.message_type, - project_name=self.project_name, ) self.assertFalse(sent_alert) @@ -100,11 +100,11 @@ def test_does_not_send_if_user_not_verified(self): message_id=self.message_id, from_username=self.from_username, project_id=self.project_id, + project_name=self.project_name, task_id=self.task_id, subject=self.subject, content=self.content, message_type=self.message_type, - project_name=self.project_name, ) self.assertFalse(sent_alert) @@ -120,11 +120,11 @@ def test_does_send_if_user_verified(self, mock_send_message): message_id=self.message_id, from_username=self.from_username, project_id=self.project_id, + project_name=self.project_name, task_id=self.task_id, subject=self.subject, content=self.content, message_type=self.message_type, - project_name=self.project_name, ) # Assert self.assertTrue(sent_alert) diff --git a/tests/backend/integration/services/test_team_service.py b/tests/backend/integration/services/test_team_service.py index 0095be78c5..9f812f201f 100644 --- a/tests/backend/integration/services/test_team_service.py +++ b/tests/backend/integration/services/test_team_service.py @@ -1,11 +1,19 @@ +from unittest.mock import patch + from backend.models.postgis.statuses import TeamMemberFunctions, TeamRoles -from backend.services.team_service import TeamService +from backend.services.team_service import ( + TeamService, + TeamJoinNotAllowed, + MessageService, +) from tests.backend.base import BaseTestCase from tests.backend.helpers.test_helpers import ( add_user_to_team, assign_team_to_project, create_canned_project, create_canned_team, + create_canned_user, + return_canned_user, ) @@ -86,3 +94,55 @@ def test_get_project_teams_as_dto(self): # Assert self.assertIn("teams", teams_dto) self.assertNotEqual(len(teams_dto["teams"]), 0) + + @patch.object(TeamService, "is_user_team_member") + def test_join_team_raises_error_if_user_already_team_member( + self, mock_is_team_member + ): + # Arrange + test_team = create_canned_team() + test_user = create_canned_user() + add_user_to_team(test_team, test_user, TeamMemberFunctions.MEMBER.value, True) + mock_is_team_member.return_value = True + # Act/Assert + with self.assertRaises(TeamJoinNotAllowed): + TeamService.join_team( + test_team.id, test_user.id, test_user.username, "MEMBER" + ) + + @patch.object(TeamService, "is_user_team_member") + @patch.object(MessageService, "_push_messages") + def test_join_team_sends_notification_if_team_is_invite_only_and_manager_has_allowed_notification( + self, mock_send_notification, mock_is_team_member + ): + # Arrange + test_team = create_canned_team() + test_team.invite_only = True + test_user = create_canned_user() + mock_is_team_member.return_value = False + test_manager = return_canned_user("test manager", 1234) + test_manager = add_user_to_team(test_team, test_manager, 1, True) + test_manager.join_request_notifications = True + + # Act + TeamService.join_team(test_team.id, test_user.id, test_user.username, "MEMBER") + # Assert + mock_send_notification.assert_called() + + @patch.object(TeamService, "is_user_team_member") + @patch.object(MessageService, "_push_messages") + def test_join_team_doesnt_send_notification_if_manager_has_disallowed_notification( + self, mock_send_notification, mock_is_team_member + ): + # Arrange + test_team = create_canned_team() + test_team.invite_only = True + test_user = create_canned_user() + mock_is_team_member.return_value = False + test_manager = return_canned_user("test manager", 1234) + test_manager = add_user_to_team(test_team, test_manager, 1, True) + test_manager.join_request_notifications = False + # Act + TeamService.join_team(test_team.id, test_user.id, test_user.username, "MEMBER") + # Assert + mock_send_notification.assert_not_called() diff --git a/tests/backend/unit/services/test_mapping_service.py b/tests/backend/unit/services/test_mapping_service.py index 381362ee64..2f34e237a9 100644 --- a/tests/backend/unit/services/test_mapping_service.py +++ b/tests/backend/unit/services/test_mapping_service.py @@ -1,3 +1,4 @@ +from unittest.mock import patch, MagicMock from backend.services.mapping_service import ( MappingService, Task, @@ -12,7 +13,7 @@ from backend.models.postgis.project_info import ProjectInfo from backend.models.dtos.mapping_dto import MappedTaskDTO, LockTaskDTO from backend.models.postgis.task import TaskHistory, TaskAction, User -from unittest.mock import patch, MagicMock +from backend.services.messaging.message_service import MessageService from tests.backend.base import BaseTestCase @@ -129,10 +130,12 @@ def test_if_new_state_not_acceptable_raise_error(self, mock_task): @patch.object(Task, "update") @patch.object(TaskHistory, "get_last_status") @patch.object(TaskHistory, "update_task_locked_with_duration") + @patch.object(MessageService, "send_message_after_comment") @patch.object(MappingService, "get_task") def test_unlock_with_comment_sets_history( self, mock_task, + mock_send_message, mock_history, mock_update, mock_stats, @@ -150,6 +153,7 @@ def test_unlock_with_comment_sets_history( test_task = MappingService.unlock_task_after_mapping(self.mapped_task_dto) # Assert + mock_send_message.assert_called() self.assertEqual(TaskAction.COMMENT.name, test_task.task_history[0].action) self.assertEqual(test_task.task_history[0].action_text, "Test comment")