feat(logout): add logout button and tests #37
Conversation
WalkthroughThe pull request introduces a logout option in the dashboard by adding a Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as Dashboard
participant S as Server
U->>D: Click "Log Out" button
D->>S: Send DELETE request to /logout
S-->>U: Respond with redirect to login page
sequenceDiagram
participant U as User
participant L as LoginPage
participant S as Server
U->>L: Visit login page
U->>L: Enter credentials
L->>S: POST credentials to /sessions
S-->>U: Respond with dashboard redirection on success
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 RuboCop (1.73)test/controllers/sessions_controller_test.rb[!] There was an error parsing from /Gemfile:20-------------------------------------------# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
-------------------------------------------/usr/lib/ruby/3.1.0/bundler/dsl.rb:357:in test/system/sessions_test.rb[!] There was an error parsing from /Gemfile:20-------------------------------------------# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
-------------------------------------------/usr/lib/ruby/3.1.0/bundler/dsl.rb:357:in ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
DevanB
force-pushed
the
23-add-logout-functionality
branch
from
7d8381a to
aa4b6d9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/frontend/pages/AppPage/Dashboard.tsx(1 hunks)test/controllers/sessions_controller_test.rb(1 hunks)test/system/sessions_test.rb(1 hunks)
🔇 Additional comments (8)
app/frontend/pages/AppPage/Dashboard.tsx (2)
2-2: Well structured import addition.The import of
Linkfrom@inertiajs/reactallows the Dashboard component to handle navigation with proper HTTP method support.
10-10: Correctly implemented logout button.The implementation follows best practices:
- Uses
as="button"for proper semantics and accessibility- Correctly specifies
method="delete"for the logout action- Provides clear "Log Out" text for the user
test/system/sessions_test.rb (3)
4-6: Good use of setup for test data reuse.The setup block efficiently initializes the user fixture that will be used across multiple tests, following DRY principles.
8-18: Well-structured login test with clear assertions.The test thoroughly validates the login flow by:
- Visiting the login page
- Filling in credentials
- Verifying both the success message and correct redirection path
This covers both the UI feedback and the navigation behavior expected after login.
20-28: Complete logout test covering the full user flow.This test effectively validates the logout functionality by:
- First logging in the user
- Clicking the logout button
- Verifying both the success message and redirect behavior
Good job ensuring both the UI feedback and navigation behavior are tested.
test/controllers/sessions_controller_test.rb (3)
8-11: Simple and effective login page test.The test correctly verifies that the login page is accessible and returns a successful response.
13-19: Good test for successful login.The test effectively:
- Verifies that a new session is created using
assert_difference- Confirms proper redirection to the dashboard after login
21-25: Important negative test case for invalid login.This test is crucial for security as it verifies that no session is created when invalid credentials are provided.
| test "should log out" do | ||
| post sessions_path, params: { email_address: @user.email_address, password: "password" } | ||
|
|
||
| assert_difference("Session.count", -1) do | ||
| delete delete_session_path(@user) | ||
| end | ||
|
|
||
| assert_redirected_to new_session_path | ||
| end |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify parameter usage in the logout path.
The test correctly verifies session destruction and redirection, but check if passing @user to delete_session_path is intentional.
Most logout implementations don't require a user parameter since they operate on the current session.
🏁 Script executed:
#!/bin/bash
# Check the route definition for delete_session_path
grep -r "delete_session" --include="*.rb" config/routes.rbLength of output: 119
Clarify parameter usage on logout route.
The test is asserting that the session is correctly destroyed and redirection is performed, which is great. However, the route definition for delete_session_path is:
delete "logout" => "sessions#destroy", as: :delete_sessionThis route doesn't require any parameters, so passing @user appears unnecessary. Please verify if this extra parameter is intentional. If not required, remove @user from the call to avoid potential confusion and ensure consistency with the route definition.
DevanB
force-pushed
the
23-add-logout-functionality
branch
from
aa4b6d9 to
cc230ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/controllers/sessions_controller_test.rb (1)
21-25: Consider adding response verification.While the test correctly verifies that no session is created with invalid credentials, it doesn't check what happens afterward. Consider adding assertions to verify either a redirection or an appropriate response status.
test "should not log in with invalid credentials" do assert_no_difference("Session.count") do post sessions_path, params: { email_address: @user.email_address, password: "wrong_password" } end + + # Verify the response - either a redirect back to login page or render with errors + assert_response :unprocessable_entity + # Or + # assert_redirected_to new_session_path end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/frontend/pages/AppPage/Dashboard.tsx(1 hunks)test/controllers/sessions_controller_test.rb(1 hunks)test/system/sessions_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/frontend/pages/AppPage/Dashboard.tsx
🔇 Additional comments (7)
test/system/sessions_test.rb (4)
25-25: Verify element type for logout action.There appears to be a potential mismatch between this test and the implementation. The test is looking for a "Log Out" button, but according to the PR summary, the implementation uses a
Linkcomponent from@inertiajs/react. Ensure the element type in the test matches what's actually rendered in the UI to prevent test failures.
8-18: LGTM! Well-structured login test.This test effectively verifies the complete login flow, including form submission and post-login state verification.
20-28: LGTM! Well-structured logout test.The test properly verifies the logout workflow by first logging in, then performing the logout action, and finally verifying the post-logout state.
4-6: LGTM! Good test setup.The setup method correctly initializes test data using fixtures, following Rails testing conventions.
test/controllers/sessions_controller_test.rb (3)
8-10: LGTM! Simple and effective test.This test correctly verifies that the login page is accessible.
13-19: LGTM! Great session verification.This test effectively verifies both session creation and proper redirection after successful login.
27-34: LGTM! Comprehensive logout test.This test effectively verifies session destruction and proper redirection after logout.
Summary by CodeRabbit
New Features
Tests