Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions backend/app/database/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,22 @@ def db_toggle_image_favourite_status(image_id: str) -> bool:
return False
finally:
conn.close()


def db_get_total_image_count() -> int:
"""
Get the total number of images in the database.
Returns:
Total count of images
"""
conn = _connect()
cursor = conn.cursor()
try:
cursor.execute("SELECT COUNT(*) FROM images")
result = cursor.fetchone()
return result[0] if result else 0
except Exception as e:
logger.error(f"Error getting total image count: {e}")
return 0
finally:
conn.close()
9 changes: 8 additions & 1 deletion backend/app/routes/folders.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
db_get_folder_ids_by_path_prefix,
db_get_all_folder_details,
)
from app.database.images import db_get_total_image_count
from app.logging.setup_logging import get_logger
Comment on lines +14 to 15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider perf + error semantics of COUNT(*) on every /all-folders call
This correctly wires total_images into the response, but a full-table COUNT(*) can become expensive at scale, and returning 0 on error (per db_get_total_image_count) can silently mislead the UI. Consider either caching/materializing the count (or computing it alongside existing queries), and/or propagating an error (or a nullable field) instead of defaulting to 0 on DB failure.

Also applies to: 464-472

🤖 Prompt for AI Agents
In @backend/app/routes/folders.py around lines 14 - 15, The code calls
db_get_total_image_count for the /all-folders endpoint and returns 0 on DB
error, which is both a potential perf hotspot (COUNT(*) on every request) and
hides errors; change the implementation to avoid per-request full-table COUNT(*)
by either: 1) returning the total_images as a nullable field or propagating the
DB error upward instead of defaulting to 0 (so the UI can handle/error), or 2)
compute the count alongside your existing folder query or use a
cached/materialized value updated asynchronously; update the handler(s) that
reference db_get_total_image_count (including the other use at the block around
lines 464-472) to adopt one of these approaches and ensure error paths do not
silently return 0.

from app.schemas.folders import (
AddFolderRequest,
Expand Down Expand Up @@ -460,9 +461,15 @@ def get_all_folders():
taggingCompleted=tagging_completed,
)
)

total_images = db_get_total_image_count()

return GetAllFoldersResponse(
data=GetAllFoldersData(folders=folders, total_count=len(folders)),
data=GetAllFoldersData(
folders=folders,
total_count=len(folders),
total_images=total_images
),
success=True,
message=f"Successfully retrieved {len(folders)} folder(s)",
)
Expand Down
1 change: 1 addition & 0 deletions backend/app/schemas/folders.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class FolderDetails(BaseModel):
class GetAllFoldersData(BaseModel):
folders: List[FolderDetails]
total_count: int
total_images: int


class AddFolderData(BaseModel):
Expand Down
7 changes: 6 additions & 1 deletion docs/backend/backend_python/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1785,12 +1785,17 @@
"total_count": {
"type": "integer",
"title": "Total Count"
},
"total_images": {
"type": "integer",
"title": "Total Images"
}
},
"type": "object",
"required": [
"folders",
"total_count"
"total_count",
"total_images"
],
"title": "GetAllFoldersData"
},
Expand Down
7 changes: 4 additions & 3 deletions frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"@vitejs/plugin-react": "^4.2.1",
"autoprefixer": "^10.4.20",
"babel-jest": "^29.7.0",
"baseline-browser-mapping": "^2.9.11",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's check the package.json file to confirm the dependency
cat -n frontend/package.json | head -100

Repository: AOSSIE-Org/PictoPy

Length of output: 4146


🏁 Script executed:

# Search for any usage of baseline-browser-mapping in the codebase
rg "baseline-browser-mapping" --type js --type json -A 2 -B 2

Repository: AOSSIE-Org/PictoPy

Length of output: 1775


🏁 Script executed:

# Also search for any imports or references to this package
rg "baseline.*browser.*mapping" --type js --type ts --type json

Repository: AOSSIE-Org/PictoPy

Length of output: 563


🏁 Script executed:

# Check what packages depend on baseline-browser-mapping
rg "baseline-browser-mapping" frontend/package-lock.json -B 20 | grep -E '"(name|requires|dependencies)"' | head -30

Repository: AOSSIE-Org/PictoPy

Length of output: 85


🏁 Script executed:

# Search for any configuration files that might use it
find frontend -type f \( -name "jest.config.*" -o -name "postcss.config.*" -o -name "babel.config.*" -o -name ".babelrc*" \) -exec cat {} \;

Repository: AOSSIE-Org/PictoPy

Length of output: 753


🏁 Script executed:

# Check if it's referenced in any scripts or configuration
rg "baseline" frontend --type json --type js --type ts -i

Repository: AOSSIE-Org/PictoPy

Length of output: 563


🏁 Script executed:

# Check which package in package-lock.json has baseline-browser-mapping as a dependency
python3 <<'EOF'
import json
import sys

with open('frontend/package-lock.json', 'r') as f:
    lock_data = json.load(f)

packages = lock_data.get('packages', {})
for pkg_name, pkg_info in packages.items():
    if 'dependencies' in pkg_info:
        if 'baseline-browser-mapping' in pkg_info['dependencies']:
            print(f"Package '{pkg_name}' depends on baseline-browser-mapping: {pkg_info['dependencies']['baseline-browser-mapping']}")
EOF

Repository: AOSSIE-Org/PictoPy

Length of output: 148


🏁 Script executed:

# Also check the dependencies of autoprefixer and caniuse which often pull in browser mappings
rg "\"autoprefixer\"" frontend/package-lock.json -A 15 | head -40

Repository: AOSSIE-Org/PictoPy

Length of output: 1193


Remove baseline-browser-mapping from devDependencies

This package is explicitly listed but has no usage anywhere in the codebase—not in any code files, configuration files, or npm scripts. It's a CLI tool for browser mapping that's unrelated to the PR objective of displaying total image count and appears to be an accidental inclusion.

🤖 Prompt for AI Agents
In @frontend/package.json at line 91, The devDependency
"baseline-browser-mapping" is unused and should be removed from
frontend/package.json; delete the "baseline-browser-mapping": "^2.9.11" entry
from the devDependencies block, run your package manager (npm install or yarn
install) to update the lockfile, and verify there are no references by searching
the repo for "baseline-browser-mapping" and confirming it no longer appears in
package-lock.json or yarn.lock.

"eslint": "^8.57.1",
"eslint-config-prettier": "^9.1.0",
"eslint-config-react-app": "^7.0.1",
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/features/folderSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ interface FolderState {
folders: FolderDetails[];
taggingStatus: Record<string, FolderTaggingInfo>;
lastUpdatedAt?: number;
totalImages: number;
}

const initialState: FolderState = {
folders: [],
taggingStatus: {},
totalImages: 0,
};

const folderSlice = createSlice({
Expand All @@ -22,6 +24,11 @@ const folderSlice = createSlice({
state.folders = action.payload;
},

// Set total images
setTotalImages(state, action: PayloadAction<number>) {
state.totalImages = action.payload;
},

// Add a single folder
addFolder(state, action: PayloadAction<FolderDetails>) {
const newFolder = action.payload;
Expand Down Expand Up @@ -69,6 +76,7 @@ const folderSlice = createSlice({
// Clear all folder data
clearFolders(state) {
state.folders = [];
state.totalImages = 0;
},

// Set tagging status for folders
Expand All @@ -91,6 +99,7 @@ const folderSlice = createSlice({

export const {
setFolders,
setTotalImages,
addFolder,
updateFolder,
removeFolders,
Expand Down
8 changes: 7 additions & 1 deletion frontend/src/hooks/useFolderOperations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
deleteFolders,
} from '@/api/api-functions';
import { selectAllFolders } from '@/features/folderSelectors';
import { setFolders, setTaggingStatus } from '@/features/folderSlice';
import { setFolders, setTaggingStatus, setTotalImages } from '@/features/folderSlice';
import { FolderDetails } from '@/types/Folder';
import { useMutationFeedback } from './useMutationFeedback';
import { getFoldersTaggingStatus } from '@/api/api-functions/folders';
Expand Down Expand Up @@ -61,6 +61,12 @@ export const useFolderOperations = () => {
if (foldersQuery.data?.data?.folders) {
const folders = foldersQuery.data.data.folders as FolderDetails[];
dispatch(setFolders(folders));
dispatch(
setTotalImages(
(foldersQuery.data.data as unknown as { total_images: number })
.total_images || 0,
),
);
}
}, [foldersQuery.data, dispatch]);

Expand Down
Loading