Fix - Update staff group (#898)

* drop group name unqiue index, add per_course_group_name unique index

* update staff cr when updating group

* update staff cr when updating group

Author: YaleChen299

lib/cadet/courses/courses.ex

@@ -238,13 +238,12 @@ defmodule Cadet.Courses do
       case role do
         # If student, update his course registration
         :student ->
-          course_reg
-          |> CourseRegistration.changeset(%{group_id: group.id})
-          |> Repo.update()
+          update_course_reg_group(course_reg, group.id)
 
         # If admin or staff, remove their previous group assignment and set them as group leader
         _ ->
           remove_staff_from_group(course_id, course_reg.id)
+          update_course_reg_group(course_reg, group.id)
 
           group
           |> Group.changeset(%{leader_id: course_reg.id})
@@ -268,12 +267,11 @@ defmodule Cadet.Courses do
             |> Repo.one()} do
       case role do
         :student ->
-          course_reg
-          |> CourseRegistration.changeset(%{group_id: nil})
-          |> Repo.update()
+          update_course_reg_group(course_reg, nil)
 
         _ ->
           remove_staff_from_group(course_id, course_reg.id)
+          update_course_reg_group(course_reg, nil)
           {:ok, nil}
       end
     end
@@ -295,6 +293,12 @@ defmodule Cadet.Courses do
     end
   end
 
+  defp update_course_reg_group(course_reg, group_id) do
+    course_reg
+    |> CourseRegistration.changeset(%{group_id: group_id})
+    |> Repo.update()
+  end
+
   @doc """
   Get a group based on the group name and course id or create one if it doesn't exist
   """

test/cadet/courses/courses_test.exs

@@ -488,18 +488,42 @@ defmodule Cadet.CoursesTest do
              )
     end
 
-    test "succeeds when upsert same group name to another course" do
+    test "succeeds when upsert same group name to another course", %{
+      course: course,
+      existing_group_leader: existing_group_leader
+    } do
       course2 = insert(:course)
 
       new_group_leader = insert(:course_registration, %{course: course2, role: :staff})
       new_group_student = insert(:course_registration, %{course: course2, role: :student})
 
+      assert is_nil(
+               Group
+               |> where(course_id: ^course2.id)
+               |> where(name: "Existing Group")
+               |> Repo.one()
+             )
+
       usernames_and_groups = [
         %{username: new_group_student.user.username, group: "Existing Group"},
         %{username: new_group_leader.user.username, group: "Existing Group"}
       ]
 
       assert :ok == Courses.upsert_groups_in_course(usernames_and_groups, course2.id, "test")
+
+      group = Group |> where(course_id: ^course.id) |> where(name: "Existing Group") |> Repo.one()
+      assert group |> Map.fetch!(:leader_id) == existing_group_leader.id
+
+      group2 =
+        Group |> where(course_id: ^course2.id) |> where(name: "Existing Group") |> Repo.one()
+
+      assert group2 |> Map.fetch!(:leader_id) == new_group_leader.id
+
+      student = CourseRegistration |> Repo.get(new_group_student.id)
+      assert student |> Map.fetch!(:group_id) == group2.id
+
+      leader = CourseRegistration |> Repo.get(new_group_leader.id)
+      assert leader |> Map.fetch!(:group_id) == group2.id
     end
   end