Improve user creation

generic/tables.py

@@ -11,6 +11,7 @@ from generic.models import CidUser
 
 from django.contrib.auth.models import User
 
+
 class ImageColumn(tables.Column):
     def render(self, value):
         try:
@@ -28,11 +29,11 @@ class CidUserTable(tables.Table):
         "cid_scores", args=[A("cid"), A("passcode")], orderable=True
     )
 
-    #physics_exams = tables.ManyToManyColumn(verbose_name="Physics Exams")
+    # physics_exams = tables.ManyToManyColumn(verbose_name="Physics Exams")
-    #rapid_exams = tables.ManyToManyColumn(verbose_name="Rapid Exams")
+    # rapid_exams = tables.ManyToManyColumn(verbose_name="Rapid Exams")
-    #sba_exams = tables.ManyToManyColumn(verbose_name="SBA Exams")
+    # sba_exams = tables.ManyToManyColumn(verbose_name="SBA Exams")
-    #longs_exams = tables.ManyToManyColumn(verbose_name="Long Exams")
+    # longs_exams = tables.ManyToManyColumn(verbose_name="Long Exams")
-    #anatomy_exams = tables.ManyToManyColumn(verbose_name="Anatomy Exams")
+    # anatomy_exams = tables.ManyToManyColumn(verbose_name="Anatomy Exams")
     group = tables.Column(linkify=True)
 
     selection = tables.CheckBoxColumn(accessor="pk", orderable=False)
@@ -58,9 +59,7 @@ class CidUserTable(tables.Table):
 
     def __init__(self, data=None, *args, **kwargs):
         super().__init__(
-            data.prefetch_related(
+            data.prefetch_related("group"),
-                "group"
-            ),
             *args,
             **kwargs,
         )
@@ -72,6 +71,7 @@ class CidUserTable(tables.Table):
         else:
             return format_html(record.email)
 
+
 class CidUserExamTable(tables.Table):
     # edit = tables.LinkColumn(
     #    "anatomy:anatomy_question_update", text="Edit", args=[A("pk")], orderable=False
@@ -110,7 +110,12 @@ class CidUserExamTable(tables.Table):
     def __init__(self, data=None, *args, **kwargs):
         super().__init__(
             data.prefetch_related(
-                "physics_exams", "rapid_exams", "sba_exams", "anatomy_exams", "longs_exams", "group"
+                "physics_exams",
+                "rapid_exams",
+                "sba_exams",
+                "anatomy_exams",
+                "longs_exams",
+                "group",
             ),
             *args,
             **kwargs,
@@ -122,42 +127,66 @@ class CidUserExamTable(tables.Table):
         else:
             return format_html(record.email)
 
+
 #    def render_cid(self, value, record):
 #        return format_html("""<a href="#" onclick="return window.create_popup_window('/longs/series/{}', 'Series')" >Popup</a>""", record.pk)
 
+
 class UserUserTable(tables.Table):
     # edit = tables.LinkColumn(
     #    "anatomy:anatomy_question_update", text="Edit", args=[A("pk")], orderable=False
     # )
 
     user = tables.Column(
-        linkify={"viewname": "account_profile", "args":[A("username")]}, orderable=True, verbose_name="User", empty_values=()
+        linkify={"viewname": "account_profile", "args": [A("username")]},
+        orderable=True,
+        verbose_name="User",
+        empty_values=(),
     )
 
     supervisor = tables.Column(
-        linkify={"viewname": "generic:supervisor_detail", "args":[A("userprofile__supervisor__pk")]}, orderable=True, verbose_name="Supervisor", accessor=A("userprofile__supervisor")
+        linkify={
+            "viewname": "generic:supervisor_detail",
+            "args": [A("userprofile__supervisor__pk")],
+        },
+        orderable=True,
+        verbose_name="Supervisor",
+        accessor=A("userprofile__supervisor"),
     )
 
     selection = tables.CheckBoxColumn(accessor="pk", orderable=False)
 
     edit = tables.Column(
-         empty_values=(), linkify={"viewname": "account_update", "args":[A("username")]}, orderable=True, verbose_name="Edit",
+        empty_values=(),
+        linkify={"viewname": "account_update", "args": [A("username")]},
+        orderable=True,
+        verbose_name="Edit",
     )
 
     user_groups = tables.ManyToManyColumn(verbose_name="Groups", linkify_item=True)
 
+    date_joined = tables.DateTimeColumn(format="Y/m/d")
+
     class Meta:
         model = User
         template_name = "django_tables2/bootstrap4.html"
         fields = (
             # "cid",
-            #"userprofile",
+            # "userprofile",
             "userprofile__grade",
-            #"user_groups",
+            "date_joined",
-            #"userprofile.supervisor",
+            # "user_groups",
+            # "userprofile.supervisor",
             # "email",
         )
-        sequence = ("user", "userprofile__grade", "supervisor", "user_groups", "edit")
+        sequence = (
+            "user",
+            "userprofile__grade",
+            "supervisor",
+            "user_groups",
+            "date_joined",
+            "edit",
+        )
 
     def __init__(self, data=None, *args, **kwargs):
         super().__init__(
@@ -176,4 +205,10 @@ class UserUserTable(tables.Table):
         return "Edit"
 
     def render_user(self, value, record):
-        return format_html("{} {}<br/>{}<br/>{}", record.first_name, record.last_name, record.username, record.email)
+        return format_html(
+            "{} {}<br/>{}<br/>{}",
+            record.first_name,
+            record.last_name,
+            record.username,
+            record.email,
+        )

generic/urls.py

@@ -31,7 +31,9 @@ urlpatterns = [
         "cids/manage/<int:pk>/update", views.CidUserUpdate.as_view(), name="update_cid"
     ),
     path(
-        "cids/manage/<int:pk>/update/exams", views.CidUserExamUpdate.as_view(), name="update_cid_exams"
+        "cids/manage/<int:pk>/update/exams",
+        views.CidUserExamUpdate.as_view(),
+        name="update_cid_exams",
     ),
     path(
         "cids/manage/<int:cid>/email_results",
@@ -47,7 +49,11 @@ urlpatterns = [
     path("cids/manage/exams", views.CidUserExamView.as_view(), name="manage_cid_exams"),
     path("cids/group/", views.cid_group_view, name="cid_group_view"),
     path("cids/group/all", views.cid_group_view_all, name="cid_group_view_all"),
-    path("cids/group/<int:group_id>/", views.cid_group_view_detail, name="cid_group_detail"),
+    path(
+        "cids/group/<int:group_id>/",
+        views.cid_group_view_detail,
+        name="cid_group_detail",
+    ),
     path("cids/group/<int:pk>/email", views.group_email, name="group_email"),
     path(
         "cids/group/<int:pk>/email_results",
@@ -77,11 +83,23 @@ urlpatterns = [
     path(
         "cids/group/create", views.CidUserGroupCreate.as_view(), name="cid_group_create"
     ),
-    path("user/bulk_delete_supervisors/", views.users_bulk_delete_supervisors, name="users_bulk_delete_supervisors"),
+    path(
-    path("user/bulk_edit_grade/", views.users_bulk_edit_grade, name="users_bulk_edit_grade"),
+        "user/bulk_delete_supervisors/",
+        views.users_bulk_delete_supervisors,
+        name="users_bulk_delete_supervisors",
+    ),
+    path(
+        "user/bulk_edit_grade/",
+        views.users_bulk_edit_grade,
+        name="users_bulk_edit_grade",
+    ),
     path("user/group/", views.user_group_view, name="user_group_view"),
     path("user/group/all", views.user_group_view_all, name="user_group_view_all"),
-    path("user/group/<int:group_id>/", views.user_group_view_detail, name="user_group_detail"),
+    path(
+        "user/group/<int:group_id>/",
+        views.user_group_view_detail,
+        name="user_group_detail",
+    ),
     path(
         "user/group/<int:pk>/update",
         views.UserUserGroupUpdate.as_view(),
@@ -100,10 +118,24 @@ urlpatterns = [
     path("cids/create", views.manage_cid_users, name="manage_cid_users"),
     path("cids/create/email", views.create_cid_email, name="create_cid_email"),
     path("supervisor", views.SupervisorList.as_view(), name="supervisor"),
-    path("supervisor/<int:pk>", views.SupervisorDetail.as_view(), name="supervisor_detail"),
+    path(
-    path("supervisor/<int:pk>/edit", views.SupervisorUpdate.as_view(), name="supervisor_edit"),
+        "supervisor/<int:pk>",
-    path("supervisor/create", views.SupervisorCreate.as_view(), name="supervisor_create"),
+        views.SupervisorDetail.as_view(),
-    path("supervisor/<int:pk>/delete", views.SupervisorDelete.as_view(), name="supervisor_delete"),
+        name="supervisor_detail",
+    ),
+    path(
+        "supervisor/<int:pk>/edit",
+        views.SupervisorUpdate.as_view(),
+        name="supervisor_edit",
+    ),
+    path(
+        "supervisor/create", views.SupervisorCreate.as_view(), name="supervisor_create"
+    ),
+    path(
+        "supervisor/<int:pk>/delete",
+        views.SupervisorDelete.as_view(),
+        name="supervisor_delete",
+    ),
 ]
 
 
@@ -237,7 +269,11 @@ def generic_exam_urls(generic_exam_view: GenericExamViews):
         ),
         path("exam/json/", generic_exam_view.active_exams, name="active_exams"),
         path("exam/json/<int:pk>", generic_exam_view.exam_json, name="exam_json"),
-        path("exam/json/<int:pk>/<int:cid>/<str:passcode>", generic_exam_view.exam_json_cid, name="exam_json_cid"),
+        path(
+            "exam/json/<int:pk>/<int:cid>/<str:passcode>",
+            generic_exam_view.exam_json_cid,
+            name="exam_json_cid",
+        ),
         path(
             "exam/json/<int:pk>/unbased",
             generic_exam_view.exam_json_unbased,

rad/urls.py

@@ -74,6 +74,11 @@ urlpatterns = [
         views.UpdateUserView.as_view(),
         name="account_update",
     ),
+    path(
+        "accounts/delete/<int:pk>/",
+        views.DeleteUserView.as_view(),
+        name="account_delete",
+    ),
     path(
         "accounts/update_profile/<str:slug>/",
         views.UpdateUserProfileView.as_view(),

rad/views.py

@@ -125,17 +125,18 @@ def cid_results(request, cid):
         },
     )
 
+
 @login_required
 def user_scores(request):
     # exam = get_object_or_404(Exam, pk=pk)
 
-    #cid = pk
+    # cid = pk
 
-    #cid_user = CidUser.objects.filter(cid=cid).first()
+    # cid_user = CidUser.objects.filter(cid=cid).first()
-    #print(cid_user)
+    # print(cid_user)
-    #if not cid_user or cid_user.passcode != passcode:
+    # if not cid_user or cid_user.passcode != passcode:
     #    raise Http404("CID / Passcode combination not found")
-    #print(cid_user.passcode)
+    # print(cid_user.passcode)
 
     # questions = exam.exam_questions.all()
     EXAM_ANSWER_MAP = {
@@ -156,8 +157,7 @@ def user_scores(request):
         if exams_to_add:
             exams.append((exam_type, exams_to_add))
 
-    #case_collections = request.user.casecollection_exams.all()
+    # case_collections = request.user.casecollection_exams.all()
-
 
     USER_EXAM_TYPES = (
         ("Physics", "user_physics_exams"),
@@ -165,7 +165,7 @@ def user_scores(request):
         ("SBAs", "user_sba_exams"),
         ("Anatomy", "user_anatomy_exams"),
         ("Longs", "user_longs_exams"),
-        #("CaseCollection", "user_casecollection_exams"),
+        # ("CaseCollection", "user_casecollection_exams"),
     )
     available_exams = []
 
@@ -175,7 +175,7 @@ def user_scores(request):
             temp_exams = exam_rel.filter(exam_mode=True, archive=False).order_by("name")
             available_exams.append((n, temp_exams))
 
-    #available_exams = request.user.get_cid_exams()
+    # available_exams = request.user.get_cid_exams()
 
     return render(
         request,
@@ -187,13 +187,14 @@ def user_scores(request):
             # "longs_exams": longs_exams,
             # "sba_exams": sba_exams,
             "all_exams": exams,
-            #"passcode": passcode,
+            # "passcode": passcode,
             "cid_user": request.user,
             "available_exams": available_exams,
-            "case_collections": [],#case_collections,
+            "case_collections": [],  # case_collections,
         },
     )
 
+
 def cid_scores(request, cid, passcode):
     # exam = get_object_or_404(Exam, pk=pk)
 
@@ -345,7 +346,6 @@ class AddQuestionNote(CreateView):
         return context
 
     def form_valid(self, form):
-
         model = form.save(commit=False)
 
         if self.request.user.is_authenticated:
@@ -482,6 +482,7 @@ def view_feedback(request):
         },
     )
 
+
 def privacy_view(request):
     return render(request, "privacy.html")
 
@@ -517,6 +518,7 @@ class UserListView(CidManagerRequiredMixin, FilterView):
 
     filterset_class = UserListFilter
 
+
 class UserListTableView(CidManagerRequiredMixin, SingleTableMixin, FilterView):
     model = User
     table_class = UserUserTable
@@ -528,32 +530,36 @@ class UserListTableView(CidManagerRequiredMixin, SingleTableMixin, FilterView):
         context = super().get_context_data(**kwargs)
         # user = self.request.user
 
-        #filters = {"archive": False, "exam_mode": True}
+        # filters = {"archive": False, "exam_mode": True}
 
-        #physics_exams = [(i.name, i.pk) for i in PhysicsExam.objects.filter(**filters)]
+        # physics_exams = [(i.name, i.pk) for i in PhysicsExam.objects.filter(**filters)]
-        #rapid_exams = [(i.name, i.pk) for i in RapidExam.objects.filter(**filters)]
+        # rapid_exams = [(i.name, i.pk) for i in RapidExam.objects.filter(**filters)]
-        #sba_exams = [(i.name, i.pk) for i in SbasExam.objects.filter(**filters)]
+        # sba_exams = [(i.name, i.pk) for i in SbasExam.objects.filter(**filters)]
-        #longs_exams = [(i.name, i.pk) for i in LongExam.objects.filter(**filters)]
+        # longs_exams = [(i.name, i.pk) for i in LongExam.objects.filter(**filters)]
-        #anatomy_exams = [(i.name, i.pk) for i in AnatomyExam.objects.filter(**filters)]
+        # anatomy_exams = [(i.name, i.pk) for i in AnatomyExam.objects.filter(**filters)]
-        #casecollection_exams = [
+        # casecollection_exams = [
         #    (i.name, i.pk)
         #    for i in CaseCollection.objects.filter(archive=False, collection_type__gt=0)
-        #]
+        # ]
 
-        #context["physics_exams"] = physics_exams
+        # context["physics_exams"] = physics_exams
-        #context["rapid_exams"] = rapid_exams
+        # context["rapid_exams"] = rapid_exams
-        #context["sba_exams"] = sba_exams
+        # context["sba_exams"] = sba_exams
-        #context["longs_exams"] = longs_exams
+        # context["longs_exams"] = longs_exams
-        #context["anatomy_exams"] = anatomy_exams
+        # context["anatomy_exams"] = anatomy_exams
-        #context["casecollection_exams"] = casecollection_exams
+        # context["casecollection_exams"] = casecollection_exams
 
-        #cid_user_groups = [
+        # cid_user_groups = [
         #    (i.name, i.pk) for i in CidUserGroup.objects.filter(archive=False)
-        #]
+        # ]
 
-        #context["cid_user_groups"] = cid_user_groups
+        # context["cid_user_groups"] = cid_user_groups
         return context
 
+class DeleteUserView(CidManagerRequiredMixin, DeleteView):
+    model = User
+    template_name: str = "confirm_delete.html"
+    success_url = reverse_lazy("accounts_list")
 
 class UpdateUserView(CidManagerRequiredMixin, UpdateView):
     model = User
@@ -564,9 +570,10 @@ class UpdateUserView(CidManagerRequiredMixin, UpdateView):
     slug_url_kwarg = "slug"
 
     def get_success_url(self):
-        view_name = 'account_profile'
+        view_name = "account_profile"
         # No need for reverse_lazy here, because it's called inside the method
-        return reverse(view_name, kwargs={'slug': self.object.username})
+        return reverse(view_name, kwargs={"slug": self.object.username})
+
 
 class UpdateUserProfileView(CidManagerRequiredMixin, UpdateView):
     model = UserProfile
@@ -581,9 +588,9 @@ class UpdateUserProfileView(CidManagerRequiredMixin, UpdateView):
     slug_url_kwarg = "slug"
 
     def get_success_url(self):
-        view_name = 'account_profile'
+        view_name = "account_profile"
         # No need for reverse_lazy here, because it's called inside the method
-        return reverse(view_name, kwargs={'slug': self.object.username})
+        return reverse(view_name, kwargs={"slug": self.object.username})
 
 
 # class UpdateUser(TemplateView):
@@ -617,6 +624,7 @@ class UpdateUserProfileView(CidManagerRequiredMixin, UpdateView):
 #    def get(self, request, *args, **kwargs):
 #        return self.post(request, *args, **kwargs)
 
+
 @user_is_cid_user_manager
 def accounts_check_users(request):
     if request.method == "POST":
@@ -651,14 +659,25 @@ def accounts_bulk_create(request):
 
             for user in users:
                 if not User.objects.filter(username=user["email"]).exists():
+                    # Start by checking the email
+                    try:
+                        validate_email(user["email"])
+                    except ValidationError:
+                        error_text = (
+                            error_text
+                            + f"<p>{user['email']} does not appear to be a valid email (account for {user['first_name']} {user['last_name']} not created).</p>"
+                        )
+                        continue
+
+                    # Try to create the user
                     try:
                         print(user)
                         user_dict = {
                             "username": user["email"],
-                            "first_name": user["first_name"],
+                            "first_name": user["email"],
                             "last_name": user["last_name"],
                             "email": user["email"],
-                            "password": secrets.token_hex(nbytes=16)
+                            "password": secrets.token_hex(nbytes=16),
                         }
                         new_user = User.objects.create_user(**user_dict)
                     except Exception as error:
@@ -666,25 +685,27 @@ def accounts_bulk_create(request):
                             error_text
                             + f"<p>Error creating user: { user['email'] }<br/>{error}</p>"
                         )
+                        # No need to try and create a profile if we can't create a user
+                        continue
 
                     try:
-
                         user_profile = UserProfile.objects.get(user=new_user)
                         user_profile.peninsula_trainee = True
                         if "supervisor_email" in user and "supervisor_name" in user:
-                            s, created = Supervisor.objects.get_or_create(email=user["supervisor_email"], name=user["supervisor_name"])
+                            s, created = Supervisor.objects.get_or_create(
+                                email=user["supervisor_email"],
+                                name=user["supervisor_name"],
+                            )
 
                             if created:
                                 s.save()
 
                             user_profile.supervisor = s
 
-
                         if "grade" in user:
                             if not user["grade"].startswith("ST"):
                                 user["grade"] = f"ST{user['grade']}"
 
-
                             grade = UserGrades.objects.get(name=user["grade"])
                             user_profile.grade = grade
 
@@ -692,9 +713,11 @@ def accounts_bulk_create(request):
 
                         created_users.append(user["email"])
                     except Exception as error:
+                        # If we fail to create the profile delete the user
+                        new_user.delete()
                         error_text = (
                             error_text
-                            + f"<p>Error creating user profile: { user['email'] }<br/>{error}</p>"
+                            + f"<p>Error creating user profile: { user['email'] }<br/>{error} (user not created)</p>"
                         )
                 else:
                     existing_users.append(user["email"])
@@ -709,22 +732,7 @@ def accounts_bulk_create(request):
             context["message"] = "The following users already exist: " + ", ".join(
                 existing_users
             )
-        # if not self.check_user_edit_access(request.user):
-        #    data = {"status": "error, insufficient permission"}
-        #    return JsonResponse(data, status=400)
 
-        # exam = get_object_or_404(self.Exam, pk=pk)
-
-        # exam.publish_results = (
-        #    True if request.POST.get("publish_results") == "true" else False
-        # )
-        # exam.save()
-        # data = {
-        #    "status": "success",
-        #    "publish_results": exam.publish_results,
-        #    "name": exam.name,
-        #    "id": exam.id,
-        # }
         return render(request, "accounts_bulk_create.html", context)
     else:
         return render(request, "accounts_bulk_create.html", {})
@@ -743,5 +751,6 @@ def request_cid_details(request):
         cid_user.email_details(resend=True)
         cid_user.email_details()
 
-
+    return HttpResponse(
-    return HttpResponse(f"Candidate details will be sent to the requested email (if it is found in the system). If you have not received an email after a few minutes you may need to contact <a href='mailto:{settings.CONTACT_EMAIL}'>{settings.CONTACT_EMAIL}</a>")
+        f"Candidate details will be sent to the requested email (if it is found in the system). If you have not received an email after a few minutes you may need to contact <a href='mailto:{settings.CONTACT_EMAIL}'>{settings.CONTACT_EMAIL}</a>"
+    )

rapids/tests/test_urls.py

@@ -27,6 +27,12 @@ EXAM_VIEWS = (
     AnatomyExamViews,
 )
 
+JSON_VIEWS = (
+    RapidExamViews,
+    LongExamViews,
+    AnatomyExamViews,
+)
+
 
 @pytest.fixture
 def create_superuser(db, django_user_model):
@@ -46,7 +52,8 @@ def set_up_exams(request, django_user_model, create_superuser):
     return request.param
 
 
-@pytest.fixture(params=EXAM_VIEWS)
+# We only test JSON functionality with this so limit the views
+@pytest.fixture(params=JSON_VIEWS)
 def set_up_packets(request):
     e = request.param.Exam.objects.create(name="test packet", exam_mode=False, active=True)
     return request.param

templates/accounts_bulk_create.html

@@ -77,7 +77,7 @@
             var users = [];
 
             $("#users-list").empty();
-            $("#users-list").append(`The following users will be created. "undefined" fields will be left blank. Emails will be used as usernames. Supervisors will be created automatically if required.`);
+            $("#users-list").append(`The following users will be created. "undefined" fields will be left blank. Emails will be used as usernames. Supervisors will be created automatically if required. If an incorrect grade is used the user will not be created.`);
 
             var emails = [];
             for (i=0; i<excelRow.length - 1; i++) {

templates/user_update.html

@@ -1,10 +1,14 @@
 {% extends 'generic/base.html' %}
 {% load crispy_forms_tags %}
 {% block content %}
-<h2>Editing user: {{object.username}}</h2>
+    <h2>Editing user: {{object.username}}</h2>
-This form allows you to edit the users name and email address. More details (such as grade / supervisor / etc...) can be changed <a href="{% url 'account_profile_update' object.username %}">here</a>
+    This form allows you to edit the users name and email address. More details (such as grade / supervisor / etc...) can be changed <a href="{% url 'account_profile_update' object.username %}">here</a>
-<form method="post">{% csrf_token %}
+    <form method="post">{% csrf_token %}
-    {{ form|crispy }}
+        {{ form|crispy }}
-    <input type="submit" value="Update">
+        <input type="submit" value="Update">
-</form>
+    </form>
+
+    <details><summary>Delete user </summary>
+        <a href="{% url 'account_delete' object.pk %}">Click to delete user</a>
+    </details>
 {% endblock %}