From 91f8bb03bc198b65cff6de38a6f43f7d61cbfae9 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 28 Jan 2026 13:46:18 +0100 Subject: [PATCH] refactor: remove tests against basic framework functionalities --- ...est-optimization-user-liveview-analysis.md | 524 ++++++++++++++++++ test/mv_web/live/user_live/show_test.exs | 48 -- test/mv_web/user_live/form_test.exs | 163 ------ test/mv_web/user_live/index_test.exs | 243 -------- 4 files changed, 524 insertions(+), 454 deletions(-) create mode 100644 docs/test-optimization-user-liveview-analysis.md diff --git a/docs/test-optimization-user-liveview-analysis.md b/docs/test-optimization-user-liveview-analysis.md new file mode 100644 index 0000000..6955f62 --- /dev/null +++ b/docs/test-optimization-user-liveview-analysis.md @@ -0,0 +1,524 @@ +# User LiveView Tests - Optimization Analysis + +**Date:** 2026-01-28 +**Status:** πŸ“‹ Analysis Complete - Ready for Implementation + +--- + +## Executive Summary + +Analysis of User LiveView tests identified significant optimization opportunities: +- **Framework functionality over-testing:** ~15-20 tests test Phoenix/Ash core features +- **Redundant test data creation:** Each test creates users/members independently +- **Missing shared fixtures:** No `setup_all` usage for common data +- **Estimated time savings:** 15-20 seconds (from current ~35.5s for 10 tests in Top 20) + +--- + +## Current Performance Metrics + +### Top 20 Slowest Tests (User LiveView) + +| Rank | Test | File | Time | Category | +|------|------|------|------|----------| +| 3 | `test sorting functionality initially sorts by email ascending` | `index_test.exs` | 4.5s | Index | +| 5 | `test edit user form - updates admin sets new password for user` | `form_test.exs` | 4.2s | Form | +| 6 | `test edit user form - validation shows error for duplicate email` | `form_test.exs` | 4.0s | Form | +| 7 | `test mount and display displays linked member when present` | `show_test.exs` | 3.9s | Show | +| 8 | `test member linking - workflow selecting member and saving links member to user` | `form_test.exs` | 3.6s | Form | +| 10 | `test edit user form - validation shows error for invalid password` | `form_test.exs` | 3.5s | Form | +| 13 | `test mount and display displays password authentication status when enabled` | `show_test.exs` | 3.3s | Show | +| 14 | `test mount and display mounts successfully with valid user ID` | `show_test.exs` | 3.1s | Show | +| 15 | `test new user form - creation creates user with password when enabled` | `form_test.exs` | 3.0s | Form | +| 16 | `test edge cases handles very long email addresses` | `index_test.exs` | 2.9s | Index | +| 17 | `test checkbox selection functionality can select individual users` | `index_test.exs` | 2.8s | Index | +| 19 | `test checkbox selection functionality select all automatically checks when all individual users are selected` | `index_test.exs` | 2.7s | Index | + +**Total:** ~35.5 seconds for User LiveView tests in Top 20 + +--- + +## Analysis by Test File + +### 1. `test/mv_web/user_live/index_test.exs` + +**Total Tests:** 25 tests +**Performance Impact:** ~10.2s for 3 tests in Top 20 + +#### Issues Identified + +**1. Framework Functionality Over-Testing** + +**Navigation Tests (Lines 329-355):** +```elixir +test "clicking on user row navigates to user show page" +test "edit link points to correct edit page" +test "new user button points to correct new page" +``` +**Problem:** Tests Phoenix LiveView navigation (framework functionality) +**Recommendation:** Remove or consolidate into single integration test + +**Translation Tests (Lines 357-377):** +```elixir +test "shows German translations for selection" +test "shows English translations for selection" +``` +**Problem:** Tests Gettext (framework functionality) +**Recommendation:** Remove - translations are tested by framework + +**Basic Display Tests (Lines 6-47):** +```elixir +test "shows translated title in German" +test "shows translated title in English" +test "shows New User button" +test "displays users in a table" +test "shows correct action links" +``` +**Problem:** Tests basic HTML rendering (framework functionality) +**Recommendation:** Consolidate into single smoke test + +**2. Redundant Test Data Creation** + +**Sorting Tests (Lines 49-133):** +- Each test creates 3 users in `setup` block +- Users are identical across tests +- Could be shared in `setup_all` + +**Checkbox Tests (Lines 135-299):** +- Each test creates 2 users in `setup` block +- Users are identical across tests +- Could be shared in `setup_all` + +**3. Framework Sorting Logic Testing** + +**Sorting Tests:** +```elixir +test "initially sorts by email ascending" +test "can sort email descending by clicking sort button" +test "toggles back to ascending when clicking sort button twice" +test "shows sort direction icons" +``` +**Problem:** Tests `Enum.sort_by` (Elixir standard library) and basic UI state +**Recommendation:** Keep one integration test, remove others + +**4. Checkbox State Testing** + +**Checkbox Tests (Lines 135-299):** +- 7 tests testing checkbox state management +- Most test Phoenix LiveView event handling (framework functionality) +- Only business logic: "select all automatically checks when all individual users are selected" + +**Recommendation:** Keep business logic test, remove framework tests + +#### Optimization Opportunities + +1. **Consolidate Basic Display Tests:** 5 tests β†’ 1 smoke test +2. **Remove Translation Tests:** 2 tests β†’ 0 (framework functionality) +3. **Consolidate Navigation Tests:** 3 tests β†’ 1 integration test +4. **Share Test Data:** Move user creation to `setup_all` for sorting/checkbox tests +5. **Reduce Sorting Tests:** 4 tests β†’ 1 integration test +6. **Reduce Checkbox Tests:** 7 tests β†’ 2 tests (business logic only) + +**Estimated Savings:** 5-7 seconds + +--- + +### 2. `test/mv_web/user_live/form_test.exs` + +**Total Tests:** 20 tests +**Performance Impact:** ~15.0s for 4 tests in Top 20 +**Note:** Uses `async: false` to prevent PostgreSQL deadlocks + +#### Issues Identified + +**1. Framework Validation Testing** + +**Validation Tests (Lines 122-153, 221-258):** +```elixir +test "shows error for duplicate email" +test "shows error for short password" +test "shows error for invalid password" +``` +**Problem:** Tests Ash validations (framework functionality) +**Recommendation:** Remove - validations are tested by Ash framework + +**2. Redundant Test Data Creation** + +**Form Display Tests:** +- Each test creates users/members independently +- `setup_live_view` creates new admin user each time +- Members created in each test that needs them + +**Recommendation:** Use `setup_all` for common fixtures + +**3. Framework Functionality Testing** + +**Internationalization Tests (Lines 261-292):** +```elixir +test "shows German labels" +test "shows English labels" +test "shows different labels for edit vs new" +``` +**Problem:** Tests Gettext (framework functionality) +**Recommendation:** Remove - translations are tested by framework + +**4. Password Storage Testing** + +**Password Storage Tests (Lines 92-119):** +```elixir +test "stores password when provided" +``` +**Problem:** Tests password hashing (AshAuthentication framework functionality) +**Recommendation:** Remove - password hashing is tested by AshAuthentication + +**5. Member Linking Tests** + +**Member Linking Tests (Lines 294-422):** +- Tests member linking workflow (business logic) βœ… Keep +- But creates members in each test +- Could share fixtures + +#### Optimization Opportunities + +1. **Remove Validation Tests:** 3 tests β†’ 0 (framework functionality) +2. **Remove Translation Tests:** 3 tests β†’ 0 (framework functionality) +3. **Remove Password Storage Test:** 1 test β†’ 0 (framework functionality) +4. **Share Test Data:** Use `setup_all` for common users/members +5. **Consolidate Display Tests:** Multiple display tests β†’ 1-2 smoke tests + +**Estimated Savings:** 6-8 seconds + +--- + +### 3. `test/mv_web/live/user_live/show_test.exs` + +**Total Tests:** 11 tests +**Performance Impact:** ~10.3s for 3 tests in Top 20 + +#### Issues Identified + +**1. Framework Functionality Testing** + +**Navigation Tests (Lines 105-133):** +```elixir +test "back button navigates to user list" +test "edit button navigates to edit form" +``` +**Problem:** Tests Phoenix LiveView navigation (framework functionality) +**Recommendation:** Remove or consolidate into single integration test + +**2. Basic Display Tests** + +**Display Tests (Lines 25-103):** +- Multiple tests for basic HTML rendering +- Some test framework functionality (Gettext) + +**Recommendation:** Consolidate into fewer tests + +**3. Redundant Test Data Creation** + +- Each test creates users/members independently +- `setup` creates one user, but many tests create additional users/members + +**Recommendation:** Use `setup_all` for common fixtures + +#### Optimization Opportunities + +1. **Consolidate Display Tests:** 5 tests β†’ 2-3 tests +2. **Remove Navigation Tests:** 2 tests β†’ 0 (framework functionality) +3. **Share Test Data:** Use `setup_all` for common users/members + +**Estimated Savings:** 3-4 seconds + +--- + +### 4. Additional Test Files + +#### `test/mv_web/user_live/form_member_selection_test.exs` +- **Tests:** 6 tests +- **Status:** βœ… Good - Tests business logic (member selection workflow) +- **Optimization:** Could share member fixtures in `setup_all` + +#### `test/mv_web/user_live/form_member_search_test.exs` +- **Tests:** 4 tests +- **Status:** βœ… Good - Tests business logic (fuzzy search) +- **Optimization:** Could share member fixtures in `setup_all` + +--- + +## Framework Functionality Over-Testing + +### Tests That Should Be Removed + +According to `CODE_GUIDELINES.md` Section 4.7, we should not test framework functionality: + +**Phoenix LiveView Framework:** +- Navigation (redirects, links) +- Basic HTML rendering +- Event handling (unless custom business logic) + +**Ash Framework:** +- Standard validations (duplicate email, password length) +- Standard CRUD operations +- Relationship loading (unless custom logic) + +**Gettext Framework:** +- Translation functionality + +**Elixir Standard Library:** +- `Enum.sort_by` sorting +- Basic list operations + +### Complete List of Tests to Remove/Consolidate + +| Test | File | Reason | Action | +|------|------|--------|--------| +| `shows translated title in German` | `index_test.exs` | Gettext framework | Remove | +| `shows translated title in English` | `index_test.exs` | Gettext framework | Remove | +| `shows New User button` | `index_test.exs` | Basic HTML rendering | Consolidate | +| `displays users in a table` | `index_test.exs` | Basic HTML rendering | Consolidate | +| `shows correct action links` | `index_test.exs` | Basic HTML rendering | Consolidate | +| `shows sort direction icons` | `index_test.exs` | UI state (framework) | Remove | +| `toggles back to ascending when clicking sort button twice` | `index_test.exs` | Framework sorting | Remove | +| `shows select all checkbox` | `index_test.exs` | Basic HTML rendering | Remove | +| `shows individual user checkboxes` | `index_test.exs` | Basic HTML rendering | Remove | +| `can select individual users` | `index_test.exs` | Framework event handling | Remove | +| `can deselect individual users` | `index_test.exs` | Framework event handling | Remove | +| `select all functionality selects all users` | `index_test.exs` | Framework event handling | Remove | +| `deselect all functionality deselects all users` | `index_test.exs` | Framework event handling | Remove | +| `clicking on user row navigates to user show page` | `index_test.exs` | Framework navigation | Remove | +| `edit link points to correct edit page` | `index_test.exs` | Framework navigation | Remove | +| `new user button points to correct new page` | `index_test.exs` | Framework navigation | Remove | +| `shows German translations for selection` | `index_test.exs` | Gettext framework | Remove | +| `shows English translations for selection` | `index_test.exs` | Gettext framework | Remove | +| `shows correct form elements` | `form_test.exs` | Basic HTML rendering | Consolidate | +| `hides password fields initially` | `form_test.exs` | UI state (framework) | Remove | +| `shows password fields when checkbox toggled` | `form_test.exs` | UI state (framework) | Remove | +| `shows error for duplicate email` | `form_test.exs` | Ash validation | Remove | +| `shows error for short password` | `form_test.exs` | Ash validation | Remove | +| `shows error for invalid password` | `form_test.exs` | Ash validation | Remove | +| `stores password when provided` | `form_test.exs` | AshAuthentication framework | Remove | +| `shows German labels` | `form_test.exs` | Gettext framework | Remove | +| `shows English labels` | `form_test.exs` | Gettext framework | Remove | +| `shows different labels for edit vs new` | `form_test.exs` | Gettext framework | Remove | +| `shows correct form elements for existing user` | `form_test.exs` | Basic HTML rendering | Consolidate | +| `shows admin password fields when enabled` | `form_test.exs` | UI state (framework) | Remove | +| `back button navigates to user list` | `show_test.exs` | Framework navigation | Remove | +| `edit button navigates to edit form` | `show_test.exs` | Framework navigation | Remove | +| `displays user email` | `show_test.exs` | Basic HTML rendering | Consolidate | +| `sets correct page title` | `show_test.exs` | Basic HTML rendering | Remove | + +**Total Tests to Remove:** ~30 tests +**Estimated Time Saved:** 8-12 seconds + +--- + +## Optimization Recommendations + +### Priority 1: Remove Framework Functionality Tests + +**Action:** Remove tests that verify framework functionality (Phoenix, Ash, Gettext) + +**Impact:** +- **Tests Removed:** ~30 tests +- **Time Saved:** 8-12 seconds +- **Risk:** ⚠️ Very Low - Framework functionality is tested by framework maintainers + +**Implementation:** +1. Remove tests listed in "Complete List of Tests to Remove/Consolidate" +2. Keep only business logic tests +3. Consolidate basic display tests into 1-2 smoke tests + +### Priority 2: Implement Shared Fixtures + +**Action:** Use `setup_all` for common test data + +**Current Problem:** +- Each test creates users/members independently +- `conn_with_oidc_user` creates new admin user each time +- Members created repeatedly for similar tests + +**Solution:** +```elixir +# In index_test.exs +setup_all do + # Create shared users for sorting tests + user_a = create_test_user(%{email: "alpha@example.com", oidc_id: "alpha"}) + user_z = create_test_user(%{email: "zulu@example.com", oidc_id: "zulu"}) + user_m = create_test_user(%{email: "mike@example.com", oidc_id: "mike"}) + + # Create shared users for checkbox tests + user1 = create_test_user(%{email: "user1@example.com", oidc_id: "user1"}) + user2 = create_test_user(%{email: "user2@example.com", oidc_id: "user2"}) + + %{ + sorting_users: [user_a, user_z, user_m], + checkbox_users: [user1, user2] + } +end +``` + +**Impact:** +- **Time Saved:** 3-5 seconds +- **Risk:** ⚠️ Low - Need to ensure test isolation is maintained + +**Note:** `async: false` in `form_test.exs` prevents `setup_all` usage due to sandbox limitations. Consider: +- Using `:shared` sandbox mode (adds complexity) +- Or keeping current approach for form tests + +### Priority 3: Consolidate Redundant Tests + +**Action:** Merge similar tests into single integration tests + +**Examples:** +- Basic display tests β†’ 1 smoke test +- Navigation tests β†’ 1 integration test +- Sorting tests β†’ 1 integration test (keep business logic) + +**Impact:** +- **Tests Consolidated:** ~10 tests β†’ 3-4 tests +- **Time Saved:** 2-3 seconds +- **Risk:** ⚠️ Very Low - Same coverage, fewer tests + +### Priority 4: Optimize Test Data Volume + +**Action:** Reduce number of test records where possible + +**Current:** +- Sorting tests create 3 users +- Checkbox tests create 2 users +- Some tests create multiple members + +**Optimization:** +- Use minimum required data (2 users for sorting, 2 for checkboxes) +- Share data across tests via `setup_all` + +**Impact:** +- **Time Saved:** 1-2 seconds +- **Risk:** ⚠️ Very Low - Still tests same functionality + +--- + +## Estimated Total Impact + +| Optimization | Tests Affected | Time Saved | Risk | +|--------------|----------------|------------|------| +| Remove framework tests | ~30 tests | 8-12s | ⚠️ Very Low | +| Shared fixtures | All test files | 3-5s | ⚠️ Low | +| Consolidate tests | ~10 tests | 2-3s | ⚠️ Very Low | +| Optimize data volume | Multiple tests | 1-2s | ⚠️ Very Low | +| **Total** | | **14-22s** | | + +**Projected Final Time:** From ~35.5s to **~13-21s** (40-60% reduction) + +--- + +## Risk Assessment + +### Overall Risk: ⚠️ Low + +**Mitigations:** +1. Framework functionality is tested by framework maintainers +2. Business logic tests remain intact +3. Shared fixtures maintain test isolation +4. Consolidation preserves coverage + +**What to Monitor:** +1. No increase in bugs related to removed functionality +2. Test coverage remains adequate +3. CI execution times improve as expected + +--- + +## Implementation Plan + +### Phase 1: Remove Framework Tests (Low Risk, High Impact) + +1. Remove translation tests (Gettext) +2. Remove navigation tests (Phoenix LiveView) +3. Remove validation tests (Ash) +4. Remove basic HTML rendering tests (consolidate into smoke test) + +**Estimated Time:** 1-2 hours +**Risk:** ⚠️ Very Low + +### Phase 2: Implement Shared Fixtures (Medium Risk, Medium Impact) + +1. Add `setup_all` to `index_test.exs` for sorting/checkbox users +2. Add `setup_all` to `show_test.exs` for common users/members +3. Update tests to use shared fixtures +4. Verify test isolation maintained + +**Estimated Time:** 2-3 hours +**Risk:** ⚠️ Low (need to verify sandbox isolation) + +### Phase 3: Consolidate Tests (Low Risk, Low Impact) + +1. Merge basic display tests into smoke test +2. Merge navigation tests into integration test +3. Reduce sorting tests to 1 integration test + +**Estimated Time:** 1-2 hours +**Risk:** ⚠️ Very Low + +### Phase 4: Verify and Benchmark + +1. Run full test suite +2. Benchmark execution times +3. Verify no regressions +4. Update documentation + +**Estimated Time:** 1 hour +**Risk:** ⚠️ Very Low + +--- + +## Tests to Keep (Business Logic) + +### Index Tests +- βœ… `initially sorts by email ascending` - Tests default sort (business logic) +- βœ… `can sort email descending by clicking sort button` - Tests sort functionality +- βœ… `select all automatically checks when all individual users are selected` - Business logic +- βœ… `does not show system actor user in list` - Business rule +- βœ… `displays linked member name in user list` - Business logic +- βœ… `handles empty user list gracefully` - Edge case +- βœ… `handles users with missing OIDC ID` - Edge case + +### Form Tests +- βœ… `creates user without password` - Business logic +- βœ… `creates user with password when enabled` - Business logic +- βœ… `updates email without changing password` - Business logic +- βœ… `admin sets new password for user` - Business logic +- βœ… `selecting member and saving links member to user` - Business logic +- βœ… `unlinking member and saving removes member from user` - Business logic +- βœ… `redirects to user list when editing system actor user` - Business rule + +### Show Tests +- βœ… `displays password authentication status when enabled` - Business logic +- βœ… `displays password authentication status when not enabled` - Business logic +- βœ… `displays linked member when present` - Business logic +- βœ… `displays 'No member linked' when no member is linked` - Business logic +- βœ… `redirects to user list when viewing system actor user` - Business rule + +### Member Selection/Search Tests +- βœ… All tests in `form_member_selection_test.exs` - Business logic +- βœ… All tests in `form_member_search_test.exs` - Business logic (fuzzy search) + +--- + +## References + +- **Coding Guidelines:** `CODE_GUIDELINES.md` - Section 4.7 (Testing Best Practices) +- **Test Performance Optimization:** `docs/test-performance-optimization.md` +- **User LiveView Implementation:** `lib/mv_web/live/user_live/` + +--- + +## Next Steps + +1. βœ… Analysis complete +2. ⏳ Review with team +3. ⏳ Implement Phase 1 (Remove framework tests) +4. ⏳ Implement Phase 2 (Shared fixtures) +5. ⏳ Implement Phase 3 (Consolidate tests) +6. ⏳ Benchmark and verify diff --git a/test/mv_web/live/user_live/show_test.exs b/test/mv_web/live/user_live/show_test.exs index 9518106..feb4be6 100644 --- a/test/mv_web/live/user_live/show_test.exs +++ b/test/mv_web/live/user_live/show_test.exs @@ -30,14 +30,6 @@ defmodule MvWeb.UserLive.ShowTest do assert html =~ to_string(user.email) end - test "displays user email", %{conn: conn, user: user} do - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, ~p"/users/#{user.id}") - - assert html =~ to_string(user.email) - assert html =~ gettext("Email") - end - test "displays password authentication status when enabled", %{conn: conn} do user = create_test_user(%{email: "password-user@example.com", password: "test123"}) conn = conn_with_oidc_user(conn) @@ -102,36 +94,6 @@ defmodule MvWeb.UserLive.ShowTest do end end - describe "navigation" do - test "back button navigates to user list", %{conn: conn, user: user} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, ~p"/users/#{user.id}") - - assert {:error, {:live_redirect, %{to: to}}} = - view - |> element( - "a[aria-label='#{gettext("Back to users list")}'], button[aria-label='#{gettext("Back to users list")}']" - ) - |> render_click() - - assert to == "/users" - end - - test "edit button navigates to edit form", %{conn: conn, user: user} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, ~p"/users/#{user.id}") - - assert {:error, {:live_redirect, %{to: to}}} = - view - |> element( - "a[href='/users/#{user.id}/edit?return_to=show'], button[href='/users/#{user.id}/edit?return_to=show']" - ) - |> render_click() - - assert to == "/users/#{user.id}/edit?return_to=show" - end - end - describe "error handling" do test "raises exception for invalid user ID", %{conn: conn} do invalid_id = Ecto.UUID.generate() @@ -145,16 +107,6 @@ defmodule MvWeb.UserLive.ShowTest do end end - describe "page title" do - test "sets correct page title", %{conn: conn, user: user} do - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, ~p"/users/#{user.id}") - - # Check that page title is set (might be in title tag or header) - assert html =~ gettext("Show User") || html =~ to_string(user.email) - end - end - describe "system actor user" do test "redirects to user list when viewing system actor user", %{conn: conn} do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index 4b76c19..ad99337 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -20,23 +20,6 @@ defmodule MvWeb.UserLive.FormTest do assert has_element?(view, "input[name='user[email]']") assert has_element?(view, "input[type='checkbox'][name='set_password']") end - - test "hides password fields initially", %{conn: conn} do - {:ok, view, _html} = setup_live_view(conn, "/users/new") - - refute has_element?(view, "input[name='user[password]']") - refute has_element?(view, "input[name='user[password_confirmation]']") - end - - test "shows password fields when checkbox toggled", %{conn: conn} do - {:ok, view, _html} = setup_live_view(conn, "/users/new") - - view |> element("input[name='set_password']") |> render_click() - - assert has_element?(view, "input[name='user[password]']") - assert has_element?(view, "input[name='user[password_confirmation]']") - assert render(view) =~ "Password requirements" - end end describe "new user form - creation" do @@ -88,68 +71,6 @@ defmodule MvWeb.UserLive.FormTest do assert to_string(user.email) == "storetest@example.com" assert is_nil(user.hashed_password) end - - test "stores password when provided", %{conn: conn} do - {:ok, view, _html} = setup_live_view(conn, "/users/new") - - view |> element("input[name='set_password']") |> render_click() - - view - |> form("#user-form", - user: %{ - email: "passwordstoretest@example.com", - password: "securepassword123", - password_confirmation: "securepassword123" - } - ) - |> render_submit() - - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - user = - Ash.get!( - Mv.Accounts.User, - [email: Ash.CiString.new("passwordstoretest@example.com")], - domain: Mv.Accounts, - actor: system_actor - ) - - assert user.hashed_password != nil - assert String.starts_with?(user.hashed_password, "$2b$") - end - end - - describe "new user form - validation" do - test "shows error for duplicate email", %{conn: conn} do - _existing_user = create_test_user(%{email: "existing@example.com"}) - {:ok, view, _html} = setup_live_view(conn, "/users/new") - - html = - view - |> form("#user-form", user: %{email: "existing@example.com"}) - |> render_submit() - - assert html =~ "has already been taken" - end - - test "shows error for short password", %{conn: conn} do - {:ok, view, _html} = setup_live_view(conn, "/users/new") - - view |> element("input[name='set_password']") |> render_click() - - html = - view - |> form("#user-form", - user: %{ - email: "test@example.com", - password: "123", - password_confirmation: "123" - } - ) - |> render_submit() - - assert html =~ "length must be greater than or equal to 8" - end end describe "edit user form - display" do @@ -162,17 +83,6 @@ defmodule MvWeb.UserLive.FormTest do assert has_element?(view, "input[name='user[email]'][value='editme@example.com']") assert html =~ "Check 'Change Password' above to set a new password for this user" end - - test "shows admin password fields when enabled", %{conn: conn} do - user = create_test_user(%{email: "editme@example.com"}) - {:ok, view, _html} = setup_live_view(conn, "/users/#{user.id}/edit") - - view |> element("input[name='set_password']") |> render_click() - - assert has_element?(view, "input[name='user[password]']") - refute has_element?(view, "input[name='user[password_confirmation]']") - assert render(view) =~ "Admin Note" - end end describe "edit user form - updates" do @@ -218,79 +128,6 @@ defmodule MvWeb.UserLive.FormTest do end end - describe "edit user form - validation" do - test "shows error for duplicate email", %{conn: conn} do - _existing_user = create_test_user(%{email: "taken@example.com"}) - user_to_edit = create_test_user(%{email: "original@example.com"}) - {:ok, view, _html} = setup_live_view(conn, "/users/#{user_to_edit.id}/edit") - - html = - view - |> form("#user-form", user: %{email: "taken@example.com"}) - |> render_submit() - - assert html =~ "has already been taken" - end - - test "shows error for invalid password", %{conn: conn} do - user = create_test_user(%{email: "user@example.com"}) - {:ok, view, _html} = setup_live_view(conn, "/users/#{user.id}/edit") - - view |> element("input[name='set_password']") |> render_click() - - result = - view - |> form("#user-form", - user: %{ - email: "user@example.com", - password: "123" - } - ) - |> render_submit() - - case result do - {:error, {:live_redirect, %{to: "/users"}}} -> - flunk("Expected validation error but form was submitted successfully") - - html when is_binary(html) -> - assert html =~ "must have length of at least 8" - end - end - end - - describe "internationalization" do - test "shows German labels", %{conn: conn} do - conn = conn_with_oidc_user(conn, %{email: "admin_de@example.com"}) - conn = Plug.Test.init_test_session(conn, locale: "de") - {:ok, _view, html} = live(conn, "/users/new") - - assert html =~ "Neue*r Benutzer*in" - assert html =~ "E-Mail" - assert html =~ "Passwort setzen" - end - - test "shows English labels", %{conn: conn} do - conn = conn_with_oidc_user(conn, %{email: "admin_en@example.com"}) - Gettext.put_locale(MvWeb.Gettext, "en") - {:ok, _view, html} = live(conn, "/users/new") - - assert html =~ "New User" - assert html =~ "Email" - assert html =~ "Set Password" - end - - test "shows different labels for edit vs new", %{conn: conn} do - user = create_test_user(%{email: "test@example.com"}) - conn = conn_with_oidc_user(conn, %{email: "admin@example.com"}) - - {:ok, _view, new_html} = live(conn, "/users/new") - {:ok, _view, edit_html} = live(conn, "/users/#{user.id}/edit") - - assert new_html =~ "Set Password" - assert edit_html =~ "Change Password" - end - end - describe "member linking - display" do test "shows linked member with unlink button when user has member", %{conn: conn} do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/user_live/index_test.exs b/test/mv_web/user_live/index_test.exs index a1a02ea..b51fd15 100644 --- a/test/mv_web/user_live/index_test.exs +++ b/test/mv_web/user_live/index_test.exs @@ -3,26 +3,6 @@ defmodule MvWeb.UserLive.IndexTest do import Phoenix.LiveViewTest describe "basic functionality" do - test "shows translated title in German", %{conn: conn} do - conn = conn_with_oidc_user(conn) - conn = Plug.Test.init_test_session(conn, locale: "de") - {:ok, _view, html} = live(conn, "/users") - assert html =~ "Benutzer*innen auflisten" - end - - test "shows translated title in English", %{conn: conn} do - conn = conn_with_oidc_user(conn) - Gettext.put_locale(MvWeb.Gettext, "en") - {:ok, _view, html} = live(conn, "/users") - assert html =~ "Listing Users" - end - - test "shows New User button", %{conn: conn} do - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, "/users") - assert html =~ "New User" - end - test "displays users in a table", %{conn: conn} do # Create test users _user1 = create_test_user(%{email: "alice@example.com", oidc_id: "alice123"}) @@ -34,16 +14,6 @@ defmodule MvWeb.UserLive.IndexTest do assert html =~ "alice@example.com" assert html =~ "bob@example.com" end - - test "shows correct action links", %{conn: conn} do - user = create_test_user(%{email: "test@example.com"}) - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, "/users") - - assert html =~ "Edit" - assert html =~ "Delete" - assert html =~ ~r/href="[^"]*\/users\/#{user.id}\/edit"/ - end end describe "sorting functionality" do @@ -95,41 +65,6 @@ defmodule MvWeb.UserLive.IndexTest do assert mike_pos < alpha_pos, "mike@example.com should appear before alpha@example.com when sorted desc" end - - test "toggles back to ascending when clicking sort button twice", %{conn: conn} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # Click twice to toggle: asc -> desc -> asc - view |> element("button[phx-value-field='email']") |> render_click() - html = view |> element("button[phx-value-field='email']") |> render_click() - - # Should be back to ascending - assert html =~ "hero-chevron-up" - assert html =~ ~s(aria-sort="ascending") - - # Should be back to original ascending order - alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0) - mike_pos = html |> :binary.match("mike@example.com") |> elem(0) - zulu_pos = html |> :binary.match("zulu@example.com") |> elem(0) - - assert alpha_pos < mike_pos, "Should be back to ascending: alpha before mike" - assert mike_pos < zulu_pos, "Should be back to ascending: mike before zulu" - end - - test "shows sort direction icons", %{conn: conn} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # Initially ascending - should show up arrow - html = render(view) - assert html =~ "hero-chevron-up" - - # After clicking, should show down arrow - view |> element("button[phx-value-field='email']") |> render_click() - html = render(view) - assert html =~ "hero-chevron-down" - end end describe "checkbox selection functionality" do @@ -139,134 +74,6 @@ defmodule MvWeb.UserLive.IndexTest do %{users: [user1, user2]} end - test "shows select all checkbox", %{conn: conn} do - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, "/users") - - assert html =~ ~s(name="select_all") - assert html =~ ~s(phx-click="select_all") - end - - test "shows individual user checkboxes", %{conn: conn, users: [user1, user2]} do - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, "/users") - - assert html =~ ~s(name="#{user1.id}") - assert html =~ ~s(name="#{user2.id}") - assert html =~ ~s(phx-click="select_user") - end - - test "can select individual users", %{conn: conn, users: [user1, user2]} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # Initially, individual checkboxes should exist but not be checked - assert view |> element("input[type='checkbox'][name='#{user1.id}']") |> has_element?() - assert view |> element("input[type='checkbox'][name='#{user2.id}']") |> has_element?() - - # Initially, select_all should not be checked (since no individual items are selected) - refute view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - # Select first user checkbox - html = view |> element("input[type='checkbox'][name='#{user1.id}']") |> render_click() - - # The select_all checkbox should still not be checked (not all users selected) - refute view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - # Page should still function normally - assert html =~ "Email" - assert html =~ to_string(user1.email) - end - - test "can deselect individual users", %{conn: conn, users: [user1, _user2]} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # Select user first - view |> element("input[type='checkbox'][name='#{user1.id}']") |> render_click() - - # Then deselect user - html = view |> element("input[type='checkbox'][name='#{user1.id}']") |> render_click() - - # Select all should not be checked after deselecting individual user - refute view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - # Page should still function normally - assert html =~ "Email" - assert html =~ to_string(user1.email) - end - - test "select all functionality selects all users", %{conn: conn, users: [user1, user2]} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # Initially no checkboxes should be checked - refute view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - refute view - |> element("input[type='checkbox'][name='#{user1.id}'][checked]") - |> has_element?() - - refute view - |> element("input[type='checkbox'][name='#{user2.id}'][checked]") - |> has_element?() - - # Click select all - html = view |> element("input[type='checkbox'][name='select_all']") |> render_click() - - # After selecting all, the select_all checkbox should be checked - assert view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - # Page should still function normally and show all users - assert html =~ "Email" - assert html =~ to_string(user1.email) - assert html =~ to_string(user2.email) - end - - test "deselect all functionality deselects all users", %{conn: conn, users: [user1, user2]} do - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # Select all first - view |> element("input[type='checkbox'][name='select_all']") |> render_click() - - # Verify that select_all is checked - assert view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - # Then deselect all - html = view |> element("input[type='checkbox'][name='select_all']") |> render_click() - - # After deselecting all, no checkboxes should be checked - refute view - |> element("input[type='checkbox'][name='select_all'][checked]") - |> has_element?() - - refute view - |> element("input[type='checkbox'][name='#{user1.id}'][checked]") - |> has_element?() - - refute view - |> element("input[type='checkbox'][name='#{user2.id}'][checked]") - |> has_element?() - - # Page should still function normally - assert html =~ "Email" - assert html =~ to_string(user1.email) - assert html =~ to_string(user2.email) - end - test "select all automatically checks when all individual users are selected", %{ conn: conn, users: [user1, user2] @@ -326,56 +133,6 @@ defmodule MvWeb.UserLive.IndexTest do end end - describe "navigation" do - test "clicking on user row navigates to user show page", %{conn: conn} do - user = create_test_user(%{email: "navigate@example.com"}) - conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/users") - - # This test would need to check row click behavior - # The actual navigation would happen via JavaScript - html = render(view) - assert html =~ ~s(/users/#{user.id}) - end - - test "edit link points to correct edit page", %{conn: conn} do - user = create_test_user(%{email: "edit-me@example.com"}) - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, "/users") - - assert html =~ ~s(/users/#{user.id}/edit) - end - - test "new user button points to correct new page", %{conn: conn} do - conn = conn_with_oidc_user(conn) - {:ok, _view, html} = live(conn, "/users") - - assert html =~ ~s(/users/new) - end - end - - describe "translations" do - test "shows German translations for selection", %{conn: conn} do - conn = conn_with_oidc_user(conn) - conn = Plug.Test.init_test_session(conn, locale: "de") - {:ok, _view, html} = live(conn, "/users") - - assert html =~ "Alle Benutzer*innen auswΓ€hlen" - assert html =~ "Benutzer*in auswΓ€hlen" - end - - test "shows English translations for selection", %{conn: conn} do - conn = conn_with_oidc_user(conn) - Gettext.put_locale(MvWeb.Gettext, "en") - {:ok, _view, html} = live(conn, "/users") - - # Note: English translations might be empty strings by default - # This test would verify the structure is there - # Checking that aria-label attributes exist - assert html =~ ~s(aria-label=) - end - end - describe "edge cases" do test "handles empty user list gracefully", %{conn: conn} do # Don't create any users besides the authenticated one