Skip to content

Commit 4aa9721

Browse files
authored
fix update group idempotence (#899)
1 parent d4ebc98 commit 4aa9721

File tree

2 files changed

+22
-6
lines changed

2 files changed

+22
-6
lines changed

lib/cadet/courses/courses.ex

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,12 +242,16 @@ defmodule Cadet.Courses do
242242

243243
# If admin or staff, remove their previous group assignment and set them as group leader
244244
_ ->
245-
remove_staff_from_group(course_id, course_reg.id)
246-
update_course_reg_group(course_reg, group.id)
247-
248-
group
249-
|> Group.changeset(%{leader_id: course_reg.id})
250-
|> Repo.update()
245+
if group.leader_id != course_reg.id do
246+
update_course_reg_group(course_reg, group.id)
247+
remove_staff_from_group(course_id, course_reg.id)
248+
249+
group
250+
|> Group.changeset(%{leader_id: course_reg.id})
251+
|> Repo.update()
252+
else
253+
{:ok, nil}
254+
end
251255
end
252256
end
253257
end

test/cadet/courses/courses_test.exs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,18 @@ defmodule Cadet.CoursesTest do
524524

525525
leader = CourseRegistration |> Repo.get(new_group_leader.id)
526526
assert leader |> Map.fetch!(:group_id) == group2.id
527+
528+
# test on update idempotence
529+
usernames_and_groups2 = [
530+
%{username: new_group_leader.user.username, group: "Existing Group"}
531+
]
532+
533+
assert :ok == Courses.upsert_groups_in_course(usernames_and_groups2, course2.id, "test")
534+
535+
group2 =
536+
Group |> where(course_id: ^course2.id) |> where(name: "Existing Group") |> Repo.one()
537+
538+
assert group2 |> Map.fetch!(:leader_id) == new_group_leader.id
527539
end
528540
end
529541

0 commit comments

Comments
 (0)