feat: add status filter to projects table#1328
Conversation
🔍 Deadcode AnalysisFound 1 unreachable functions in the backend. View detailsOnly remove deadcode that you know is 100% no longer used.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
cabaucom376
force-pushed
the
feat/inactive-project-sorting
branch
from
ce50317 to
42dbcf9
Compare
|
I'm not sure I like this current solution. The desired behavior is to just get rid of stopped projects from the view, so a filter like usage on the images table would be better. |
cabaucom376
changed the title
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
I couldnt get this to work, which then got me thinking the existsing status field should be able to work for this id think |
|
It is working for me but I'n all honesty I am now noticing it is janky starting it back up. If you stop a project externally it still shows the project under the running filter even though the status is correctly showing Stopped. I'll take another look. On a side note, not related to this issue, but selecting multiple filters does not properly work, it seems to behave as a first selected basis. Try selecting both option under images usage filter for an example |
|
Check that out is that what you meant? |
|
Docker images for this PR have been built successfully!
Built from commit 1d69b38 |
|
@cabaucom376 See the filter fix i made in #1391, this is cleaner to me , and should fix every page as well. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
|
@cabaucom376 did you wanna try this again after my fixes ? or did you want me to try it? |
|
@kmendell I'll refactor it in like an hour 😃 |
|
I just tried it and it still works just fine. Not sure if you would like to change anything else or what but I'll hand it off to you. |
| export const statusFilters = [ | ||
| { | ||
| value: 'running', | ||
| label: m.common_running(), | ||
| icon: StartIcon | ||
| }, | ||
| { | ||
| value: 'stopped', | ||
| label: m.common_stopped(), | ||
| icon: StopIcon | ||
| }, | ||
| { | ||
| value: 'partially running', | ||
| label: m.common_partially_running(), | ||
| icon: AlertIcon | ||
| } | ||
| ]; |
There was a problem hiding this comment.
The statusFilters array is missing the "unknown" status, which is a valid project status supported by the backend (see ProjectStatusUnknown in backend/internal/models/project.go). The API handler documentation on line 44 of backend/internal/huma/handlers/projects.go lists "unknown" as one of the supported filter values.
Projects can have an "unknown" status when they're first discovered from the filesystem or when their Docker status cannot be determined. Users should be able to filter for these projects.
Consider adding the unknown status to maintain feature parity with the backend:
| export const statusFilters = [ | |
| { | |
| value: 'running', | |
| label: m.common_running(), | |
| icon: StartIcon | |
| }, | |
| { | |
| value: 'stopped', | |
| label: m.common_stopped(), | |
| icon: StopIcon | |
| }, | |
| { | |
| value: 'partially running', | |
| label: m.common_partially_running(), | |
| icon: AlertIcon | |
| } | |
| ]; | |
| export const statusFilters = [ | |
| { | |
| value: 'running', | |
| label: m.common_running(), | |
| icon: StartIcon | |
| }, | |
| { | |
| value: 'stopped', | |
| label: m.common_stopped(), | |
| icon: StopIcon | |
| }, | |
| { | |
| value: 'partially running', | |
| label: m.common_partially_running(), | |
| icon: AlertIcon | |
| }, | |
| { | |
| value: 'unknown', | |
| label: m.common_unknown(), | |
| icon: AlertIcon | |
| } | |
| ]; |
Note: The common_unknown translation key already exists in all language files.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/lib/components/arcane-table/data.ts
Line: 15:31
Comment:
The statusFilters array is missing the "unknown" status, which is a valid project status supported by the backend (see `ProjectStatusUnknown` in `backend/internal/models/project.go`). The API handler documentation on line 44 of `backend/internal/huma/handlers/projects.go` lists "unknown" as one of the supported filter values.
Projects can have an "unknown" status when they're first discovered from the filesystem or when their Docker status cannot be determined. Users should be able to filter for these projects.
Consider adding the unknown status to maintain feature parity with the backend:
```suggestion
export const statusFilters = [
{
value: 'running',
label: m.common_running(),
icon: StartIcon
},
{
value: 'stopped',
label: m.common_stopped(),
icon: StopIcon
},
{
value: 'partially running',
label: m.common_partially_running(),
icon: AlertIcon
},
{
value: 'unknown',
label: m.common_unknown(),
icon: AlertIcon
}
];
```
Note: The `common_unknown` translation key already exists in all language files.
How can I resolve this? If you propose a fix, please make it concise.| func (s *ProjectService) ListProjects(ctx context.Context, params pagination.QueryParams) ([]project.Details, pagination.Response, error) { | ||
| var projectsArray []models.Project | ||
| query := s.db.WithContext(ctx).Model(&models.Project{}) | ||
|
|
||
| if term := strings.TrimSpace(params.Search); term != "" { | ||
| searchPattern := "%" + term + "%" | ||
| query = query.Where( | ||
| "name LIKE ? OR path LIKE ? OR status LIKE ? OR COALESCE(dir_name, '') LIKE ?", | ||
| searchPattern, searchPattern, searchPattern, searchPattern, | ||
| ) | ||
| // We fetch all projects from the database first, then enrich with live status, | ||
| // and finally filter and paginate in memory. This ensures the status filter | ||
| // always works against the true live status. | ||
| if err := s.db.WithContext(ctx).Find(&projectsArray).Error; err != nil { | ||
| return nil, pagination.Response{}, fmt.Errorf("failed to fetch projects: %w", err) | ||
| } |
There was a problem hiding this comment.
This change from database-level filtering to fetching all projects and filtering in-memory could cause performance issues as the number of projects grows. The previous implementation used pagination.PaginateAndSortDB which applied filters at the database level, allowing the DB to efficiently handle large datasets.
The current approach:
- Fetches ALL projects from database (
Find(&projectsArray)) - Enriches all of them with live Docker status
- Then filters and paginates in memory
This means that with 1000 projects, you're:
- Loading 1000 database records
- Making Docker API calls for all 1000 projects
- Then filtering down to potentially just 10 results
While the comment justifies this as ensuring "the status filter always works against the true live status," this could be optimized by:
- Applying non-status filters at the DB level first
- Only enriching the filtered subset with live status
- Then applying the status filter in memory
Consider implementing a hybrid approach that filters on database fields first, then enriches and filters on runtime status. Alternatively, document this performance trade-off in the code comments and consider implementing pagination limits or caching strategies for deployments with many projects.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/services/project_service.go
Line: 1226:1234
Comment:
This change from database-level filtering to fetching all projects and filtering in-memory could cause performance issues as the number of projects grows. The previous implementation used `pagination.PaginateAndSortDB` which applied filters at the database level, allowing the DB to efficiently handle large datasets.
The current approach:
1. Fetches ALL projects from database (`Find(&projectsArray)`)
2. Enriches all of them with live Docker status
3. Then filters and paginates in memory
This means that with 1000 projects, you're:
- Loading 1000 database records
- Making Docker API calls for all 1000 projects
- Then filtering down to potentially just 10 results
While the comment justifies this as ensuring "the status filter always works against the true live status," this could be optimized by:
1. Applying non-status filters at the DB level first
2. Only enriching the filtered subset with live status
3. Then applying the status filter in memory
Consider implementing a hybrid approach that filters on database fields first, then enriches and filters on runtime status. Alternatively, document this performance trade-off in the code comments and consider implementing pagination limits or caching strategies for deployments with many projects.
How can I resolve this? If you propose a fix, please make it concise.
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
|
Git screwed up locally and corrupted this branch, and would not let me pull it again, so i repushed it to a new branch feat/kyles-project-status, closign this in favor of that PR so i can finish work on it. |
3 participants
closes #1163
Disclaimer Greptiles Reviews use AI, make sure to check over its work
Greptile Overview
Greptile Summary
This PR adds a status filter to the projects table, allowing users to filter projects by their runtime status (running, stopped, partially running).
Implementation Overview
Backend Changes:
Statusquery parameter to the API endpoint (ListProjectsInput)ListProjectsservice method to fetch all projects, enrich with live Docker status, then filter and paginate in-memoryFrontend Changes:
statusFiltersarray with filter options (running, stopped, partially running)project-service.tsto transform "projectStatus" back to "status" for API callsArchitecture
The implementation follows the existing pattern used for other table filters in the codebase. The filter supports comma-separated values for multi-select filtering, which is handled by the pagination utility's
matchValuefunction.The backend now uses a different approach than database-level filtering: it fetches all projects, enriches them with live Docker container status, then filters in-memory. This ensures the status filter always reflects the true runtime state rather than stale database values.
Issues Found
Missing "unknown" status option - The filter only includes 3 of the 7 supported statuses. Projects can have "unknown" status when first discovered or when Docker status cannot be determined.
Performance consideration - The new approach fetches and enriches all projects before filtering, which could impact performance with large project counts. This is a trade-off for accuracy.
Confidence Score: 4/5
Important Files Changed
File Analysis