firehose/context.md

# Code Context

## Files Retrieved
List with exact line ranges:
1. `app/test/firehose_web/controllers/blog_test.exs` (lines 1-128) - Comprehensive blog controller tests covering HTML and JSON API endpoints for engineering blog and release notes
2. `app/lib/firehose_web/controllers/blog_controller.ex` (lines 1-79) - Blog controller with pagination, 404 handling, and input validation
3. `app/test/support/conn_case.ex` (lines 1-38) - Test case template for connection tests
4. `app/lib/firehose/blogs/engineering_blog.ex` (lines 1-7) - Engineering blog module configuration
5. `app/lib/firehose/blogs/release_notes.ex` (lines 1-7) - Release notes blog module configuration

## Key Code

### Test Organization
```elixir
# Current structure has 4 describe blocks:
describe "engineering blog (HTML)"     # 3 tests
describe "input validation"           # 5 tests (newly added in last commit)
describe "release notes blog (HTML)"  # 2 tests
describe "engineering blog (JSON API)" # 4 tests  
describe "release notes blog (JSON API)" # 3 tests
```

### Input Validation Logic (blog_controller.ex, lines 68-76)
```elixir
defp parse_page(nil), do: 1
defp parse_page(str) do
  case Integer.parse(str) do
    {page, ""} when page > 0 -> page
    _ -> 1
  end
end
```

### Test Coverage Added in Last Commit
```elixir
describe "input validation" do
  test "GET /blog/nonexistent returns 404", %{conn: conn} do
    conn = get(conn, "/blog/nonexistent")
    assert html_response(conn, 404)
  end

  test "GET /blog/engineering?page=abc falls back to page 1", %{conn: conn} do
    conn = get(conn, "/blog/engineering?page=abc")
    assert html_response(conn, 200) =~ "Engineering Blog"
  end

  test "GET /blog/engineering?page=-1 falls back to page 1", %{conn: conn} do
    conn = get(conn, "/blog/engineering?page=-1")
    assert html_response(conn, 200) =~ "Engineering Blog"
  end

  test "GET /blog/engineering?page=0 falls back to page 1", %{conn: conn} do
    conn = get(conn, "/blog/engineering?page=0")
    assert html_response(conn, 200) =~ "Engineering Blog"
  end

  test "GET /blog/engineering/nonexistent-post returns 404", %{conn: conn} do
    assert_raise Blogex.NotFoundError, fn ->
      get(conn, "/blog/engineering/nonexistent-post")
    end
  end
end
```

## Architecture
The application uses:
- **Blogex** library for blog functionality (engineering blog and release notes)
- **Phoenix** framework for web endpoints
- **ConnCase** test helper for connection testing
- Two blog types: `Firehose.EngineeringBlog` and `Firehose.ReleaseNotes`
- Pagination through `blog.paginate(page)` method
- 404 handling via `Blogex.NotFoundError` exception

## Start Here
Which file to look at first and why:

**Start with `app/lib/firehose_web/controllers/blog_controller.ex`**

Why: This is the central controller that handles all blog requests. Understanding its structure (especially the `parse_page/1` function and `resolve_blog/2` plug) provides context for why the validation tests were added and how input handling works across both HTML and JSON endpoints.

## Code Smells & Refactoring Suggestions

### Smell 1: Repetitive Validation Tests
**Issue**: Four tests for page parameter validation (`page=abc`, `-1`, `0`, and valid values) are highly repetitive with identical assertions.

**Refactoring Suggestion**: Use parameterized tests or test helpers:
```elixir
# Test helper approach
test_page_fallback("page=abc", "abc")
test_page_fallback("page=-1", "-1") 
test_page_fallback("page=0", "0")

defp test_page_fallback(query_param, expected_page) do
  conn = get(conn, "/blog/engineering?#{query_param}")
  assert html_response(conn, 200) =~ "Engineering Blog"
end
```

### Smell 2: Missing Negative Test Coverage
**Issue**: Tests don't verify what happens when invalid blog_id is provided (e.g., `/blog/invalid-blog`).

**Refactoring Suggestion**: Add test for unknown blog:
```elixir
test "GET /blog/unknown returns 404", %{conn: conn} do
  conn = get(conn, "/blog/unknown")
  assert html_response(conn, 404)
end
```

### Smell 3: Inconsistent Test Naming
**Issue**: Some tests use hyphenated slugs (`v0-1-0`), others use different formats. The naming doesn't clearly indicate what's being tested.

**Refactoring Suggestion**: Standardize naming:
```elixir
# Instead of: "GET /blog/releases/v0-1-0 returns HTML post"
test "GET /blog/releases/:slug returns a release post", %{conn: conn} do
```

### Smell 4: Redundant Layout Assertions
**Issue**: Multiple tests assert the same "firehose" string appears in response, testing layout presence.

**Refactoring Suggestion**: Create a shared test helper:
```elixir
defp assert_has_app_layout(body), 
  do: assert body =~ "firehose"

# Then in tests: assert_has_app_layout(body)
```

### Smell 5: Test Order Doesn't Follow Flow
**Issue**: Tests are grouped by endpoint but validation tests (which should be first for defensive programming) are in the middle.

**Refactoring Suggestion**: Reorder to follow natural request flow:
1. Input validation (404s, invalid params)
2. Success cases (index, show, tag)
3. Edge cases (pagination, RSS feeds)

### Smell 6: No Test for Controller-Level Error Handling
**Issue**: The controller uses `halt()` in the resolve_blog plug, but there's no test verifying this behavior.

**Refactoring Suggestion**: Add test:
```elixir
test "GET /blog/:blog_id with invalid blog halts request", %{conn: conn} do
  conn = get(conn, "/blog/invalid")
  assert conn.halted
end
```

### Smell 7: Mixed Response Types Without Clear Separation
**Issue**: HTML tests use `html_response/2`, JSON tests use `json_response/2`, but there's no helper to verify content type before parsing.

**Refactoring Suggestion**: Create response helpers:
```elixir
defp assert_html(conn, status), do: assert html_response(conn, status) != ""
defp assert_json(conn, status), do: assert json_response(conn, status) != %{}
```

### Smell 8: No Test for Concurrent Requests or Edge Cases
**Issue**: Missing tests for:
- Empty page parameter (`?page=`)
- Very large page numbers
- Special characters in slug/tag parameters

**Refactoring Suggestion**: Add edge case tests to validation describe block.

### Overall Recommendations
1. **Extract test helpers** to reduce duplication (especially for page validation)
2. **Standardize test naming** conventions across all blog types
3. **Add positive test** for valid page numbers (currently missing)
4. **Consider property-based testing** for input validation scenarios
5. **Add performance tests** if pagination is used heavily
6. **Create integration tests** that verify end-to-end flows