Adds Groups.delete/2 (#20)

- Implements delete operation for Groups module
- Fixes Planner tests to add ROPC user as both owner and member of groups (Planner requirement)
- Increases wait time to 8 seconds for Azure group membership propagation
- Handles Planner API returning 204 No Content for updates in Plans and Tasks modules
- Adds Tasks.ReadWrite scope to AuthTestHelpers default scopes
- Fixes auth test to check headers at correct location in Req.Request struct
- Updates quality task to properly detect and report test failures

These changes resolve failing integration tests and complete CRUD operations for Groups.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>

Author: johnnyt

lib/mix/tasks/quality.ex

@@ -270,20 +270,34 @@ defmodule Mix.Tasks.Quality do
   defp run_coverage_check do
     Mix.shell().info("📊 Running test coverage check...")
 
-    # Run coveralls with mix task to show output
-    try do
-      Mix.Task.run("coveralls", [])
-      Mix.shell().info("✅ Coverage check passed")
-    rescue
-      e in Mix.Error ->
-        Mix.shell().error("❌ Coverage check failed.")
-        Mix.shell().info("💡 Run 'MIX_ENV=test mix coveralls.detail' to see uncovered lines")
-
-        Mix.shell().info(
-          "💡 Add more tests to increase coverage above threshold set in coveralls.json"
-        )
-
-        Mix.raise("Coverage check failed: #{Exception.message(e)}")
+    # Run coveralls using System.cmd to capture output and exit code
+    case System.cmd("mix", ["coveralls"], stderr_to_stdout: true, env: [{"MIX_ENV", "test"}]) do
+      {output, 0} ->
+        # Show the output to user
+        Mix.shell().info(output)
+        Mix.shell().info("✅ Coverage check passed")
+
+      {output, exit_code} ->
+        # Show the output to user
+        Mix.shell().info(output)
+
+        # Check if it's a test failure vs coverage failure
+        cond do
+          String.contains?(output, "tests, ") and
+              (String.contains?(output, "failure") or String.contains?(output, "error")) ->
+            Mix.shell().error("❌ Test failures detected.")
+            Mix.raise("Tests failed - fix failing tests before proceeding")
+
+          true ->
+            Mix.shell().error("❌ Coverage check failed.")
+            Mix.shell().info("💡 Run 'MIX_ENV=test mix coveralls.detail' to see uncovered lines")
+
+            Mix.shell().info(
+              "💡 Add more tests to increase coverage above threshold set in coveralls.json"
+            )
+
+            Mix.raise("Coverage check failed with exit code #{exit_code}")
+        end
     end
   end
 

lib/msg/groups.ex

@@ -335,6 +335,52 @@ defmodule Msg.Groups do
     end
   end
 
+  @doc """
+  Deletes a Microsoft 365 Group.
+
+  **Warning:** This is a destructive operation that permanently removes the group and all
+  associated resources including the group mailbox, SharePoint site, Planner plans,
+  calendar, and Teams (if connected). Deleted groups may be recoverable from the
+  recycle bin for up to 30 days.
+
+  ## Parameters
+
+  - `client` - Authenticated Req.Request client
+  - `group_id` - ID of the group to delete
+
+  ## Returns
+
+  - `:ok` - Group deleted successfully (204 status)
+  - `{:error, :not_found}` - Group doesn't exist
+  - `{:error, :forbidden}` - Insufficient permissions
+  - `{:error, term}` - Other errors
+
+  ## Required Permissions
+
+  - `Group.ReadWrite.All` (application permission)
+
+  ## Examples
+
+      :ok = Msg.Groups.delete(client, "group-id-123")
+
+  ## See Also
+
+  - [Delete Group API](https://learn.microsoft.com/en-us/graph/api/group-delete)
+  """
+  @spec delete(Req.Request.t(), String.t()) :: :ok | {:error, term()}
+  def delete(client, group_id) do
+    case Req.delete(client, url: "/groups/#{group_id}") do
+      {:ok, %{status: 204}} ->
+        :ok
+
+      {:ok, %{status: status, body: body}} ->
+        handle_error(status, body)
+
+      {:error, reason} ->
+        {:error, reason}
+    end
+  end
+
   # Private functions
 
   defp handle_error(401, _), do: {:error, :unauthorized}

lib/msg/planner/plans.ex

@@ -218,6 +218,11 @@ defmodule Msg.Planner.Plans do
            json: updates_converted,
            headers: [{"If-Match", etag}]
          ) do
+      {:ok, %{status: 204}} ->
+        # Planner API returns 204 No Content for updates
+        # Fetch the updated plan to return it
+        get(client, plan_id)
+
       {:ok, %{status: status, body: body}} when status in 200..299 ->
         {:ok, body}
 

lib/msg/planner/tasks.ex

@@ -237,6 +237,11 @@ defmodule Msg.Planner.Tasks do
            json: updates_converted,
            headers: [{"If-Match", etag}]
          ) do
+      {:ok, %{status: 204}} ->
+        # Planner API returns 204 No Content for updates
+        # Fetch the updated task to return it
+        get(client, task_id)
+
       {:ok, %{status: status, body: body}} when status in 200..299 ->
         {:ok, body}
 

test/msg/integration/auth_test.exs

@@ -193,8 +193,8 @@ defmodule Msg.Integration.AuthTest do
         assert %Req.Request{} = delegated_client
         assert delegated_client.options.base_url == "https://graph.microsoft.com/v1.0"
 
-        # Verify it has authorization header
-        assert Map.has_key?(delegated_client.options, :headers)
+        # Verify it has authorization header (in Req, headers are at top level, not in options)
+        assert Map.has_key?(delegated_client.headers, "authorization")
       else
         # Skip if no ROPC credentials or consent not granted
         assert true

test/msg/integration/groups_test.exs

@@ -109,9 +109,7 @@ defmodule Msg.Integration.GroupsTest do
     assert "Unified" in group["groupTypes"]
 
     # Clean up - delete the created group
-    # Note: Group deletion requires DELETE /groups/{id}
-    # We'll add cleanup in a separate operation
-    cleanup_group(client, group["id"])
+    assert :ok = Groups.delete(client, group["id"])
   end
 
   test "handles error when getting non-existent group", %{client: client} do
@@ -195,15 +193,34 @@ defmodule Msg.Integration.GroupsTest do
     assert :ok = Groups.add_owner(client, group["id"], other_test_user_id)
 
     # Clean up
-    cleanup_group(client, group["id"])
+    assert :ok = Groups.delete(client, group["id"])
   end
 
-  # Helper function to clean up test groups
-  defp cleanup_group(client, group_id) do
+  test "deletes a group successfully", %{client: client} do
+    timestamp = System.system_time(:second)
+
+    attrs = %{
+      display_name: "Test Group to Delete #{timestamp}",
+      mail_enabled: true,
+      mail_nickname: "test-delete-#{timestamp}",
+      security_enabled: false,
+      group_types: ["Unified"],
+      description: "Temporary test group for delete operation"
+    }
+
+    {:ok, group} = Groups.create(client, attrs)
+    group_id = group["id"]
+
     # Delete the group
-    case Req.delete(client, url: "/groups/#{group_id}") do
-      {:ok, %{status: 204}} -> :ok
-      _ -> :ok
-    end
+    assert :ok = Groups.delete(client, group_id)
+
+    # Verify the group is deleted
+    assert {:error, :not_found} = Groups.get(client, group_id)
+  end
+
+  test "handles error when deleting non-existent group", %{client: client} do
+    fake_id = "00000000-0000-0000-0000-000000000000"
+
+    assert {:error, :not_found} = Groups.delete(client, fake_id)
   end
 end

test/msg/integration/planner/plans_test.exs

@@ -43,8 +43,16 @@ defmodule Msg.Integration.Planner.PlansTest do
         {:ok, created_group} = Groups.create(app_client, group)
         group_id = created_group["id"]
 
-        # Give Azure some time to provision the group
-        Process.sleep(2000)
+        # Add the ROPC user as both owner and member (Planner requirement)
+        system_user_id = System.get_env("MICROSOFT_TEST_USER_ID")
+
+        if system_user_id do
+          :ok = Groups.add_owner(app_client, group_id, system_user_id)
+          :ok = Groups.add_member(app_client, group_id, system_user_id)
+        end
+
+        # Give Azure time to propagate group membership (needs 8+ seconds)
+        Process.sleep(8000)
 
         # Create plan (requires delegated permissions)
         {:ok, created_plan} =
@@ -113,7 +121,15 @@ defmodule Msg.Integration.Planner.PlansTest do
         {:ok, created_group} = Groups.create(app_client, group)
         group_id = created_group["id"]
 
-        Process.sleep(2000)
+        # Add the ROPC user as both owner and member (Planner requirement)
+        system_user_id = System.get_env("MICROSOFT_TEST_USER_ID")
+
+        if system_user_id do
+          :ok = Groups.add_owner(app_client, group_id, system_user_id)
+          :ok = Groups.add_member(app_client, group_id, system_user_id)
+        end
+
+        Process.sleep(8000)
 
         # Create a plan
         {:ok, plan} =

test/msg/integration/planner/tasks_test.exs

@@ -36,7 +36,16 @@ defmodule Msg.Integration.Planner.TasksTest do
         {:ok, created_group} = Groups.create(app_client, group)
         group_id = created_group["id"]
 
-        Process.sleep(2000)
+        # Add the ROPC user as both owner and member (Planner requirement)
+        system_user_id = System.get_env("MICROSOFT_TEST_USER_ID")
+
+        if system_user_id do
+          :ok = Groups.add_owner(app_client, group_id, system_user_id)
+          :ok = Groups.add_member(app_client, group_id, system_user_id)
+        end
+
+        # Give Azure time to propagate group membership (needs 8+ seconds)
+        Process.sleep(8000)
 
         {:ok, plan} =
           Plans.create(delegated_client, %{
@@ -219,7 +228,16 @@ defmodule Msg.Integration.Planner.TasksTest do
         {:ok, created_group} = Groups.create(app_client, group)
         group_id = created_group["id"]
 
-        Process.sleep(2000)
+        # Add the ROPC user as both owner and member (Planner requirement)
+        system_user_id = System.get_env("MICROSOFT_TEST_USER_ID")
+
+        if system_user_id do
+          :ok = Groups.add_owner(app_client, group_id, system_user_id)
+          :ok = Groups.add_member(app_client, group_id, system_user_id)
+        end
+
+        # Give Azure time to propagate group membership (needs 8+ seconds)
+        Process.sleep(8000)
 
         {:ok, plan} =
           Plans.create(delegated_client, %{

test/support/auth_test_helpers.ex

@@ -81,6 +81,7 @@ defmodule Msg.AuthTestHelpers do
           Keyword.get(opts, :scopes, [
             "Calendars.ReadWrite.Shared",
             "Group.ReadWrite.All",
+            "Tasks.ReadWrite",
             "offline_access"
           ])