Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test new actions and apps #16

Merged
merged 15 commits into from
Sep 29, 2024
Merged

Test new actions and apps #16

merged 15 commits into from
Sep 29, 2024

Conversation

Azanul
Copy link
Owner

@Azanul Azanul commented Sep 28, 2024

No description provided.

Azanul and others added 9 commits September 27, 2024 15:54
Signed-off-by: Azanul <[email protected]>
Signed-off-by: Azanul <[email protected]>
Signed-off-by: Azanul <[email protected]>
Signed-off-by: Azanul <[email protected]>
refactor: replace template strings with regular string literals
Copy link

gooroo-dev bot commented Sep 28, 2024

Please double check the following review of the pull request:

Issues counts

🐞Mistake 🤪Typo 🚨Security 🚀Performance 💪Best Practices 📖Readability ❓Others
0 0 0 0 1 0 0

Changes in the diff

  • ➕ Added .deepsource.toml configuration file for DeepSource analyzers.
  • ➕ Added GitHub Actions workflow for PR checks.
  • ✅ Corrected string literal in frontend/app/page.tsx.
  • 🛠️ Renamed embedFrontend.go to embed_frontend.go.
  • ➕ Added repository interfaces in server/internal/repository/interfaces.go.
  • 💪 Implemented repository interfaces in server/internal/repository/movie_repository.go, rating_repository.go, and user_repository.go.
  • ➕ Added tests for repository methods in server/internal/repository/movie_repository_test.go, rating_repository_test.go, and user_repository_test.go.
  • 🛠️ Updated service constructors to use repository interfaces in server/internal/services/movie_service.go, rating_service.go, and user_service.go.
  • ➕ Added tests for service methods in server/internal/services/movie_service_test.go, rating_service_test.go, and user_service_test.go.
  • ✅ Corrected service instantiation in server/server.go.

Identified Issues

ID Type Details Severity Confidence
1 💪Best Practices The GetMovies method in movie_repository.go does not handle pagination correctly. 🟠Medium 🟠Medium

Issue 1: 💪Best Practices

Details: The GetMovies method in movie_repository.go does not handle pagination correctly. It fetches all movies without considering the page and pageSize parameters.

File Path: server/internal/repository/movie_repository.go

Lines of Code:

rows := sqlmock.NewRows([]string{"id", "title", "genre", "year", "wiki", "plot", "director", "cast"}).
    AddRow(uuid.New(), "Movie 1", "Action", 2021, "wiki1", "plot1", "director1", "cast1").
    AddRow(uuid.New(), "Movie 2", "Comedy", 2022, "wiki2", "plot2", "director2", "cast2")
mock.ExpectQuery("^SELECT (.+) FROM movies").WillReturnRows(rows)
mock.ExpectQuery("^SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(2))

Proposed Fix:

func (r *MovieRepository) GetMovies(ctx context.Context, searchTerm string, page, pageSize int) (*MoviePage, error) {
    offset := (page - 1) * pageSize
    query := "SELECT id, title, genre, year, wiki, plot, director, cast FROM movies"
    if searchTerm != "" {
        query += " WHERE title ILIKE '%' || $1 || '%' OR genre ILIKE '%' || $1 || '%'"
    }
    query += " LIMIT $2 OFFSET $3"
    
    rows, err := r.db.QueryContext(ctx, query, searchTerm, pageSize, offset)
    if err != nil {
        return nil, err
    }
    defer rows.Close()
    
    var movies []*models.Movie
    for rows.Next() {
        var movie models.Movie
        if err := rows.Scan(&movie.ID, &movie.Title, &movie.Genre, &movie.Year, &movie.Wiki, &movie.Plot, &movie.Director, &movie.Cast); err != nil {
            return nil, err
        }
        movies = append(movies, &movie)
    }
    
    countQuery := "SELECT COUNT(*) FROM movies"
    if searchTerm != "" {
        countQuery += " WHERE title ILIKE '%' || $1 || '%' OR genre ILIKE '%' || $1 || '%'"
    }
    
    var totalCount int
    err = r.db.QueryRowContext(ctx, countQuery, searchTerm).Scan(&totalCount)
    if err != nil {
        return nil, err
    }
    
    return &MoviePage{
        Movies:          movies,
        TotalCount:      totalCount,
        HasNextPage:     offset+pageSize < totalCount,
        HasPreviousPage: offset > 0,
    }, nil
}

Explanation of the Fix:
The fix involves modifying the GetMovies method to correctly handle pagination by using the LIMIT and OFFSET SQL clauses. The offset is calculated based on the page and pageSize parameters. The query is constructed to include the search term if provided, and the total count of movies is fetched separately to determine pagination details.

Missing Tests

The provided tests cover most of the new functionalities. However, additional tests can be added to ensure comprehensive coverage, especially for edge cases and error scenarios.

Example Test for Pagination:

func TestMovieRepository_GetMovies_Pagination(t *testing.T) {
    db, mock, err := sqlmock.New()
    if err != nil {
        t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
    }
    defer db.Close()

    repo := NewMovieRepository(db)

    tests := []struct {
        name       string
        searchTerm string
        page       int
        pageSize   int
        mockSetup  func()
        want       *MoviePage
        wantErr    bool
    }{
        {
            name:       "Success - With Pagination",
            searchTerm: "",
            page:       2,
            pageSize:   1,
            mockSetup: func() {
                rows := sqlmock.NewRows([]string{"id", "title", "genre", "year", "wiki", "plot", "director", "cast"}).
                    AddRow(uuid.New(), "Movie 2", "Comedy", 2022, "wiki2", "plot2", "director2", "cast2")
                mock.ExpectQuery("^SELECT (.+) FROM movies LIMIT 1 OFFSET 1").WillReturnRows(rows)
                mock.ExpectQuery("^SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(2))
            },
            want: &MoviePage{
                Movies:          []*models.Movie{},
                TotalCount:      2,
                HasNextPage:     false,
                HasPreviousPage: true,
            },
            wantErr: false,
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            tt.mockSetup()

            got, err := repo.GetMovies(context.Background(), tt.searchTerm, tt.page, tt.pageSize)
            if (err != nil) != tt.wantErr {
                t.Errorf("MovieRepository.GetMovies() error = %v, wantErr %v", err, tt.wantErr)
                return
            }
            if !tt.wantErr {
                assert.Equal(t, tt.want.TotalCount, got.TotalCount)
                assert.Equal(t, tt.want.HasNextPage, got.HasNextPage)
                assert.Equal(t, tt.want.HasPreviousPage, got.HasPreviousPage)
            }
        })
    }
}

Explanation:
This test case ensures that the pagination logic works correctly by setting up a scenario where the second page of results is fetched, and the total count of movies is verified to determine pagination details.

Summon me to re-review when updated! Yours, Gooroo.dev
Feel free to react or reply with your feedback!

@Azanul Azanul merged commit 9d2d8a7 into prod Sep 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant