Merge pull request #95 from browserstack/enhancement-group-flow

Enhancement group flow

Author: vedharish

Access/access_modules/base_email_access/access.py

@@ -174,6 +174,9 @@ def revoke(self, user, label):
     def get_extra_fields(self):
         return []
 
+    def can_auto_approve(self):
+        return False
+
     # return valid access label array which will be added in db or raise exception
     def validate_request(self, access_labels_data, request_user, is_group=False):
         valid_access_label_array = []

Access/accessrequest_helper.py

@@ -16,6 +16,7 @@
     User,
     GroupV2,
     AccessV2,
+    MembershipV2,
     ApprovalType,
 )
 from Access.background_task_manager import background_task, accept_request
@@ -224,7 +225,7 @@ def get_decline_access_request(request, access_type, request_id):
                     UserAccessMapping.get_pending_access_mapping(request_id=value)
                 )
                 request_ids.extend(current_ids)
-            access_type = access_type.rsplit("-", 1)[0]
+            access_type = "moduleAccess"
         elif access_type == "clubGroupAccess":
             for value in [request_id]:  # ready for bulk decline
                 return_ids.append(value)
@@ -238,6 +239,7 @@ def get_decline_access_request(request, access_type, request_id):
             access_type = "groupAccess"
         else:
             request_ids = [request_id]
+
         for current_request_id in request_ids:
             if access_type == "groupAccess":
                 response = decline_group_access(request, current_request_id, reason)
@@ -283,9 +285,11 @@ def get_pending_accesses_from_modules(access_user):
         process_group_requests(pending_accesses["group_requests"], group_requests)
 
         logger.info(
-            "Time to fetch pending requests of access module: %s - %s "
-            % access_module_tag,
-            str(time.time() - access_module_start_time),
+            "Time to fetch pending requests of access module: %s - %s " %
+            (
+                access_module_tag,
+                str(time.time() - access_module_start_time)
+            ),
         )
 
     return individual_requests, list(group_requests.values())
@@ -297,7 +301,7 @@ def process_individual_requests(
     if len(individual_pending_requests):
         clubbed_requests = {}
         for accessrequest in individual_pending_requests:
-            club_id = accessrequest["requestId"].rsplit("_", 1)[0]
+            club_id = accessrequest["requestId"].rsplit("_")[0]
             if club_id not in clubbed_requests:
                 clubbed_requests[club_id] = {
                     "club_id": club_id,
@@ -326,7 +330,7 @@ def process_group_requests(group_pending_requests, group_requests):
             club_id = (
                 accessrequest["groupName"]
                 + "-"
-                + accessrequest["requestId"].rsplit("-", 1)[-1].rsplit("_", 1)[0]
+                + accessrequest["requestId"].rsplit("-", 1)[-1].rsplit("_")[0]
             )
             needs_access_approve = GroupV2.objects.get(
                 name=accessrequest["groupName"], status="Approved"
@@ -382,7 +386,7 @@ def create_request(auth_user, access_request_form):
     for index1, access_type in enumerate(access_request["accessRequests"]):
         access_labels = validate_access_labels(
             access_labels_json=access_request["accessLabel"][index1],
-            access_type=access_type,
+            access_tag=access_type,
         )
         access_reason = access_request["accessReason"][index1]
 
@@ -399,17 +403,16 @@ def create_request(auth_user, access_request_form):
         }
 
         access_module = helper.get_available_access_modules()[access_type]
-        module_access_labels = access_module.validate_request(
-            access_labels, auth_user, is_group=False
-        )
-
         extra_field_labels = get_extra_field_labels(access_module)
-
         if extra_fields and extra_field_labels:
             for field in extra_field_labels:
-                module_access_labels[0][field] = extra_fields[0]
+                access_labels[0][field] = extra_fields[0]
                 extra_fields = extra_fields[1:]
 
+        module_access_labels = access_module.validate_request(
+            access_labels, auth_user, is_group=False
+        )
+
         for index2, access_label in enumerate(module_access_labels):
             request_id = request_id + "_" + str(index2)
             access_create_error = _create_access(
@@ -511,7 +514,10 @@ def get_extra_field_labels(access_module):
 def get_extra_fields(access_request):
     if "extraFields" in access_request:
         return access_request["extraFields"]
-    return []
+    elif "extraFields[]" in access_request:
+        return [access_request["extraFields[]"]]
+    else:
+        return []
 
 
 def _validate_access_request(access_request_form, user):
@@ -551,7 +557,6 @@ def validate_access_labels(access_labels_json, access_tag):
 
 def _get_approver_permissions(access_tag, access_label=None):
     json_response = {}
-
     access_module = helper.get_available_access_module_from_tag(access_tag)
     approver_permissions = []
     approver_permissions = access_module.fetch_approver_permissions(access_label)
@@ -583,9 +588,9 @@ def accept_user_access_requests(auth_user, request_id):
         )
         return json_response
 
-    requester = access_mapping.user_identity.user.email
-    if auth_user.username == requester:
-        json_response["error"] = USER_REQUEST_PERMISSION_DENIED_ERR_MSG
+    requester = access_mapping.user_identity.user
+    if auth_user.user == requester:
+        json_response["error"] = SELF_APPROVAL_ERROR_MSG
         return json_response
 
     access_label = access_mapping.access.access_label
@@ -674,37 +679,50 @@ def run_accept_request_task(
 
 def decline_individual_access(request, access_type, request_id, reason):
     json_response = {}
-    access_mapping = UserAccessMapping.get_access_request(request_id)
+    access_mapping = {}
+    decline_new_group = False
+    if access_type == "declineNewGroup":
+        access_mapping = GroupV2.get_pending_group(request_id)
+        decline_new_group = True
+    else:
+        access_mapping = UserAccessMapping.get_access_request(request_id)
+        access_type = access_mapping.access.access_tag
+
     if not is_request_valid(request_id, access_mapping):
         json_response["error"] = USER_REQUEST_IN_PROCESS_ERR_MSG.format(
             request_id=request_id,
         )
         return json_response
 
-    json_response = validate_approver_permissions(access_mapping, access_type, request)
-    if "error" in json_response:
-        return json_response
+    if not decline_new_group:
+        json_response = validate_approver_permissions(access_mapping, access_type, request)
+        if "error" in json_response:
+            return json_response
 
     with transaction.atomic():
         access_mapping.decline_access(reason)
         if hasattr(access_mapping, "approver_1"):
-            access_mapping.decline_reason = reason
             if access_mapping.approver_1 is not None:
                 access_mapping.approver_2 = request.user.user
             else:
                 access_mapping.approver_1 = request.user.user
         else:
-            access_mapping.reason = reason
-            access_mapping.approver = request.user.username
+            access_mapping.approver = request.user.user
 
         access_mapping.save()
 
-    access_module = helper.get_available_access_module_from_tag(access_type)
-    access_labels = [access_mapping.access.access_label]
-    description = access_module.combine_labels_desc(access_labels)
-    notifications.send_mail_for_request_decline(
-        request, description, request_id, reason, access_type
-    )
+    if not decline_new_group:
+        access_module = helper.get_available_access_module_from_tag(access_type)
+        access_labels = [access_mapping.access.access_label]
+        description = access_module.combine_labels_desc(access_labels)
+        notifications.send_mail_for_request_decline(
+            request, description, request_id, reason, access_type
+        )
+    else:
+        MembershipV2.update_membership(access_mapping, reason)
+        notifications.send_mail_for_request_decline(
+            request, "Group Creation", request_id, reason, access_type
+        )
 
     logger.debug(
         USER_REQUEST_DECLINE_MSG.format(
@@ -821,7 +839,7 @@ def decline_group_access(request, request_id, reason):
     access_type = group_mapping.access.access_tag
 
     json_response = validate_approver_permissions(
-        group_mapping, access_type, request, request_id
+        group_mapping, access_type, request
     )
     if "error" in json_response:
         return json_response

Access/background_task_manager.py

@@ -60,15 +60,15 @@ def run_access_grant(request_id):
     user_access_mapping = UserAccessMapping.get_access_request(request_id=request_id)
     access_tag = user_access_mapping.access.access_tag
     user = user_access_mapping.user_identity.user
-    approver = user_access_mapping.approver_1.user.username
+    approver = user_access_mapping.approver_1.user
     message = ""
     if not user_access_mapping.user_identity.user.is_active():
         user_access_mapping.decline_access(decline_reason="User is not active")
         logger.debug(
             {
                 "requestId": request_id,
                 "status": "Declined",
-                "by": approver,
+                "by": approver.username,
                 "response": message,
             }
         )

Access/group_helper.py

@@ -42,7 +42,7 @@
 
 LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR = {
     "error_msg": "Invalid Group Name",
-    "msg": "A group with {group_name} doesn't exist.",
+    "msg": "A group with name {group_name} doesn't exist.",
 }
 
 NON_OWNER_PERMISSION_DENIED_ERROR = {
@@ -129,6 +129,7 @@ def create_group(request):
         requester=request.user.user,
         description=reason,
         needsAccessApprove=needs_access_approve,
+        date_time=base_datetime_prefix,
     )
 
     new_group.add_member(
@@ -183,7 +184,9 @@ def get_group_access_list(auth_user, group_name):
         context = {
             "error": {
                 "error_msg": LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR["error_msg"],
-                "msg": LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR["msg"],
+                "msg": LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR["msg"].format(
+                    group_name=group_name
+                ),
             }
         }
         return context
@@ -236,7 +239,9 @@ def update_owners(request, group_name):
         context = {
             "error": {
                 "error_msg": LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR["error_msg"],
-                "msg": LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR["msg"],
+                "msg": LIST_GROUP_ACCESSES_GROUP_DONT_EXIST_ERROR["msg"].format(
+                    group_name=group_name
+                ),
             }
         }
         return context
@@ -441,8 +446,8 @@ def add_user_to_group(request):
                         reason=data["memberReason"][0],
                         date_time=base_datetime_prefix,
                     )
+                    membership_id = membership.membership_id
                     if not group.needsAccessApprove:
-                        membership_id = membership.membership_id
                         context = {}
                         context["accessStatus"] = {
                             "msg": REQUEST_PROCESSING.format(requestId=membership_id),
@@ -485,10 +490,11 @@ def add_user_to_group(request):
             }
 
         else:
+            membership = MembershipV2.get_membership(membership_id=membership_id)
             notifications.send_mulitple_membership_accepted_notification(
                 users_added,
                 data["groupName"][0],
-                data["memberReason"][0],
+                membership,
             )
             if len(selected_users) - len(users_added) == 0:
                 context = {}
@@ -609,6 +615,7 @@ def get_group_access(form_data, auth_user):
     )
     if validation_error:
         context["status"] = validation_error
+        return context
 
     access_module_list = data["accessList"]
     for module_value in access_module_list:
@@ -656,16 +663,17 @@ def save_group_access_request(form_data, auth_user):
         extra_fields = accessrequest_helper.get_extra_fields(access_request)
         extra_field_labels = accessrequest_helper.get_extra_field_labels(access_module)
 
-        module_access_labels = access_module.validate_request(
-            access_labels, auth_user, is_group=False
-        )
         if extra_fields and extra_field_labels:
             for field in extra_field_labels:
-                module_access_labels[0][field] = extra_fields[0]
+                access_labels[0][field] = extra_fields[0]
                 extra_fields = extra_fields[1:]
 
+        module_access_labels = access_module.validate_request(
+            access_labels, auth_user, is_group=False
+        )
+
         request_id = (
-            auth_user.username
+            group.name
             + "-"
             + access_tag
             + "-"
@@ -697,15 +705,15 @@ def save_group_access_request(form_data, auth_user):
                             "msg": "Access already exists" + json.dumps(access_label),
                         }
                     )
-        email_destination = access_module.get_approvers()
-        member_list = group.get_all_approved_members()
-        notifications.send_group_access_add_email(
-            destination=email_destination,
-            group_name=group_name,
-            requester=auth_user.user.email,
-            request_id=request_id,
-            member_list=member_list,
-        )
+        # email_destination = access_module.get_approvers()
+        # member_list = group.get_all_approved_members()
+        # notifications.send_group_access_add_email(
+        #     destination=email_destination,
+        #     group_name=group_name,
+        #     requester=auth_user.user.email,
+        #     request_id=request_id,
+        #     member_list=member_list,
+        # )
     return context
 
 
@@ -731,7 +739,7 @@ def validate_group_access_create_request(group, auth_user):
         logger.exception("This Group is not yet approved")
         return {"title": "Permisison Denied", "msg": "This Group is not yet approved"}
 
-    if not (group.is_owner(auth_user.user) or auth_user.is_superuser):
+    if not auth_user.user.is_allowed_admin_actions_on_group(group):
         logger.exception("Permission denied, you're not owner of this group")
         return {"title": "Permision Denied", "msg": "You're not owner of this group"}
     return None